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

[Igor Munkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

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>


-- 
Best regards,
IM