[tarantool-patches] Re: [PATCH 1/2] Move mp_compare_double_uint64() to trivia.h

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

LGTM.

On 14/08/2019 12:36, n.pettik wrote:
> 
> 
>> On 9 Aug 2019, at 00:37, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
>>
>> Hi! Thanks for the patch!
>>
>>> diff --git a/src/trivia/util.h b/src/trivia/util.h
>>> index 1e01013db..ebfe0c392 100644
>>> --- a/src/trivia/util.h
>>> +++ b/src/trivia/util.h
>>> @@ -500,6 +500,20 @@ json_escape(char *buf, int size, const char *data);
>>> 	}									\
>>> } while(0)
>>>
>>> +#define COMPARE_RESULT(a, b) (a < b ? -1 : a > b)
>>> +
>>> +/**
>>> + * Compare LHS with RHS, return a value <0, 0 or >0 depending on the
>>> + * comparison result (strcmp-style).
>>> + * Normally, K==1. If K==-1, the result is inverted (as if LHS and RHS
>>> + * were swapped).
>>> + * K is needed to enable tail call optimization in Release build.
>>> + * NOINLINE attribute was added to avoid aggressive inlining which
>>
>> I guess, NOINLINE does not make sense anymore. But you can keep a
>> comment, that inlining should not be added in future for the sake
>> of tail call optimization.
> 
> Thx, fixed:
> 
> diff --git a/src/trivia/util.h b/src/trivia/util.h
> index ebfe0c392..8a3d22b38 100644
> --- a/src/trivia/util.h
> +++ b/src/trivia/util.h
> @@ -508,8 +508,6 @@ json_escape(char *buf, int size, const char *data);
>   * Normally, K==1. If K==-1, the result is inverted (as if LHS and RHS
>   * were swapped).
>   * K is needed to enable tail call optimization in Release build.
> - * NOINLINE attribute was added to avoid aggressive inlining which
> - * resulted in over 2Kb code size for mp_compare_number.
>   */
>  int
>  double_compare_uint64(double lhs, uint64_t rhs, int k);
> 
> Also fixed commit subject according to your nit:
> 
> Move mp_compare_double_uint64() to trivia/util.h
>