Re: [Tarantool-patches] [PATCH resend v2 08/11] lua, datetime: calculated attributes for datetimes

[Oleg Babin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Thanks for your reply. See my comment below.

On 31.07.2021 01:30, Timur Safin wrote:
> Hello Oleg again!
>
> :
> :
> : On 28.07.2021 13:34, Timur Safin via Tarantool-patches wrote:
> : > * introduced a set of calculated attributes to data object, e.g.:
> : >    - timestamp, seconds, microseconds, minute, or hours
> : >
> : > Part of #5941
> : > ---
> : >   src/lua/datetime.lua           | 51 ++++++++++++++++++++++------------
> : >   test/app-tap/datetime.test.lua | 17 +++++++++++-
> : >   2 files changed, 50 insertions(+), 18 deletions(-)
> : >
> : > diff --git a/src/lua/datetime.lua b/src/lua/datetime.lua
> : > index 9ec06d8d8..7a208cef9 100644
> : > --- a/src/lua/datetime.lua
> : > +++ b/src/lua/datetime.lua
> : > @@ -224,6 +224,36 @@ local function interval_serialize(self)
> : >       return { secs = self.secs, nsec = self.nsec }
> : >   end
> : >
> : > +local datetime_index = function(self, key)
> : > +    local attributes = {
> :
> : This change looks like pessimization because currently we need to
> : recreate "attributes" table
> :
> : for each function call. I suggest to use `attributes` as `__index` but
> : not such function.
>
> What would you say if I simply precache this attributes table in local,
> and then proceed this way?
> -----------------------------------------
> -local datetime_index = function(self, key)
> -    local attributes = {
> +local datetime_index_handlers = {
>           unixtime = function(self)
>               return self.secs
>           end,
> +
>           timestamp = function(self)
>               return tonumber(self.secs) + self.nsec / 1e9
>           end,
> +
>           nanoseconds = function(self)
>               return self.secs * 1e9 + self.nsec
>           end,
> +
>           microseconds = function(self)
>               return self.secs * 1e6 + self.nsec / 1e3
>           end,
> +
>           milliseconds = function(self)
>               return self.secs * 1e3 + self.nsec / 1e6
>           end,
> +
>           seconds = function(self)
>               return tonumber(self.secs) + self.nsec / 1e9
>           end,
> +
>           minutes = function(self)
>               return (tonumber(self.secs) + self.nsec / 1e9) / 60
>           end,
> +
>           hours = function(self)
>               return (tonumber(self.secs) + self.nsec / 1e9) / (60 * 60)
>           end,
> +
>           days = function(self)
>               return (tonumber(self.secs) + self.nsec / 1e9) / (60 * 60) / 24
>           end,
> +
>           add = function(self)
>               return function(self, o)
>                   return interval_increment(self, o, 1)
>               end
>           end,
> +
>           sub = function(self)
>               return function(self, o)
>                   return interval_increment(self, o, -1)
>               end
>           end,
>       }
> -    return attributes[key] ~= nil and attributes[key](self) or nil
> +
> +local datetime_index = function(self, key)
> +    return datetime_index_handlers[key] ~= nil and
> +           datetime_index_handlers[key](self) or nil
>   end
> -----------------------------------------
>
> I mean we still carefully check existence of an asked key, and return nil if
> there is no such key at all. Is it much different than what you've meant?
>
It's again about temporary tables. I gave a benchmark in one of the 
previous emails.

After changes it shoud be better. Thanks.