Re: [Tarantool-patches] [PATCH v3 2/9] lua: built-in module datetime

[Safin Timur via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

On 10.08.2021 15:21, Igor Munkin wrote:
> Vlad,
> 
> On 10.08.21, Vladislav Shpilevoy wrote:
>> Hi! Thanks for the fixes!
>>
>> On 08.08.2021 19:35, Safin Timur wrote:
>>> Much respect that you've found some time for review, even while being on vacation! Thanks!
>>>
>>> On 08.08.2021 14:26, Vladislav Shpilevoy wrote:
>>>>>>> +local datetime_index_handlers = {
>>>>>>> +    unixtime = function(self)
>>>>>>> +        return self.secs
>>>>>>> +    end,
>>>>>>> +
>>>>>>> +    timestamp = function(self)
>>>>>>> +        return tonumber(self.secs) + self.nsec / 1e9
>>>>>>
>>>>>> 11. If you are saying the Lua number value range is enough,
>>>>>> why do you store them as 64bit integers in 'self' and
>>>>>> convert to a number only for the external API?
>>>>>
>>>>> May be I misunderstood your sentence, but let me elaborate here.
>>>>> I need seconds and nanoseconds kept separately for their efficient and precise handling, and for passing to c-dt.
>>>>>
>>>>> If we would keep 32-bit integers in seconds then we would be able to handle only dates upto 2038 year, thus we need 64-bit seconds for extended range.
>>>>>
>>>>> OTOH, not whole 64-bit range is practical, and required for expected in real-life datetime range values. It's not a problem that Lua number and int64_t have very different range for precise integer values. Lua number could handle integer values upto 9e15, which is corresponding to ...
>>>>
>>>> I know all that. The question is why do you keep storing cdata 64 bits numbers
>>>> inside of the datetime objects, if you convert them all to Lua numbers before
>>>> return? You could store self.secs as just number. And the other fields too. Lua
>>>> number is double, it does not loose precision for integers < 2^53, which should
>>>> be enough for the ranges you want to support. Correct?
>>>
>>> I keep using 64-bit because the primary code operating with fields is on C-side, and we need Lua number only on rare cases when user asked for composed attribute date.timestamp. Whenever we deal with seconds within arthmetic operations or transfer to c-dt, we need integer C type, larger than 32-bit. It's good fit for int64_t.
>>
>> But it is slower. Notably slower last time I benched, when I also
>> thought integers should be faster than doubles. But cdata 64 bit integers
>> are slower than plain Lua numbers. Perhaps because they involve too much
>> work with metatables for everything. Besides, doubles are larger than 32
>> bits - they are 53 bits until they start loosing int precision. And it is
>> just enough for you, isn't it?
> 
> Sorry for breaking into the party, but reasoning is much simpler: Lua
> numbers are just double values stored on the guest stack (or other
> TValue storage); cdata 64-bit integers are GCcdata objects, so like all
> others GC objects it has its lifetime, has to be allocated and freed and
> traversed by GC. You can consider them as GCstr except they are not
> interned. Hence, if the precision is fine (and it is AFAICS), then there
> is not need to use GCcdata instead of Lua native numbers. In other
> words, I totally agree with you.
> 
>>
> 
> <snipped>
> 
> 


Either I do something totally wrong, or may be I'm absolutely clueless. 
Oh rather both...

Here is experiment I've proceed 
https://gist.github.com/tsafin/f7f21aad53f23801839b3b278cfac380

I try to compare speed of accessing datetime.secs field:
- when it is current int64_t redirected via ffi to the `struct datetime`;
- when it is wrapped as (calculated) attribute of type double which we 
access via FFI datetime_secs() function accessor;
- or when it's declared as `double` in the similar `struct datetime_double`.

No surpise that calculated attribute is 3x orders of magnitude slower 
than direct ffi access to either int64_t or double field.

OTOH, differences for timings to access to int64_t (which should be 
boxed) and double (which should be unboxed) are negligible, and 
essentially the same:

```
✔ ~/datetime/tarantoolt/build [tsafin/gh-5941-datetime-v4 ↑·4|✚ 3…4⚑ 3]
23:40 $ ./src/tarantool ../../bench-datetime-secs.lua
ctype<struct datetime>
date.secs       0.00035929679870605
ctype<struct datetime>
date.secsf      0.49544525146484
ctype<struct datetime_double>
date_double.secs        0.00042939186096191
✔ ~/datetime/tarantoolt/build [tsafin/gh-5941-datetime-v4 ↑·4|✚ 3…4⚑ 3]
23:40 $ ./src/tarantool ../../bench-datetime-secs.lua
ctype<struct datetime>
date.secs       0.00034856796264648
ctype<struct datetime>
date.secsf      0.40926361083984
ctype<struct datetime_double>
date_double.secs        0.00043344497680664
✔ ~/datetime/tarantoolt/build [tsafin/gh-5941-datetime-v4 ↑·4|✚ 3…4⚑ 3]
23:40 $ ./src/tarantool ../../bench-datetime-secs.lua
ctype<struct datetime>
date.secs       0.00034213066101074
ctype<struct datetime>
date.secsf      0.46818256378174
ctype<struct datetime_double>
date_double.secs        0.00037813186645508
✔ ~/datetime/tarantoolt/build [tsafin/gh-5941-datetime-v4 ↑·4|✚ 3…4⚑ 3]
23:40 $ ./src/tarantool ../../bench-datetime-secs.lua
ctype<struct datetime>
date.secs       0.00051259994506836
ctype<struct datetime>
date.secsf      0.6695671081543
ctype<struct datetime_double>
date_double.secs        0.00048208236694336
```

What did I do wrong?

Thanks,
Timur