Re: [Tarantool-patches] [PATCH v6 4/5] datetime: perf test for datetime parser

[Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

19.08.2021 13:29, Safin Timur пишет:
> On 19.08.2021 13:19, Serge Petrenko wrote:
>>
>>
>> 19.08.2021 05:56, Timur Safin пишет:
>>> It was told that if field `datetime.secs` would be `double` we 
>>> should get
>>> better performance in LuaJIT instead of `uint64_t` type, which is 
>>> used at the
>>> moment.
>>>
>>> So we have created benchmark, which was comparing implementations of 
>>> functions
>>> from `datetime.c` if we would use `double` or `int64_t` for 
>>> `datetime.secs` field.
>>>
>>> Despite expectations, based on prior experience with floaing-point 
>>> on x86
>>> processors, comparison shows that `double` provides similar or
>>> sometimes better timings. And picture stays consistent be it SSE2, 
>>> AVX1 or
>>> AVX2 code.
>>>
>>> Part of #5941
>>> ---
>>
>> I agree with Vladimir here.
>> Looks like this perf test doesn't belong to Tarantool repository.
>> Would you mind dropping it?
>
> Here is the case (we both aware of) I want to avoid here - today we do 
> not have saved _that_ decimal perf test, basing on which we have 
> preferred LuaC and dropped FFI implementation. We could not rerun it 
> today, within a newer LuaJIT implementation, to verify that situation 
> didn't change. This is similar case - we have made a decision basing 
> on some evaluations using this code, in a future we may decide to 
> further optimize data structure (like Vova suggested elsewhere to 
> split int64 into 2 fields), and it would be better if at that moment 
> we would still have performance test around for adaptations and rerun.
>
> Yes, it's another test of performance test we used to see in perf 
> directory (hehe, there is only single test at the moment), kind of one 
> time shot in a history, important for design decision, but from longer 
> prospective I assume it should be still around.
>
> Does my reasoning make some sense?

Yes, I see your point.

But you've saved the test in some github discussion, AFAIR. I feel like 
it's an ok place to save it.

If you really want to save the test in this repo, maybe introduce a new 
folder?
Since perf/ is really intended to store perf tests. Not the ones that 
have influenced this or that
decision, but the ones that should be run every now and then to find 
regressions.

>
> Thanks,
> Timur

-- 
Serge Petrenko