Tarantool development patches archive
 help / color / mirror / Atom feed
From: Timur Safin via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: v.shpilevoy@tarantool.org, imun@tarantool.org,
	imeevma@tarantool.org, tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH v5 0/8] Initial datetime implementation
Date: Mon, 16 Aug 2021 02:59:34 +0300	[thread overview]
Message-ID: <cover.1629071531.git.tsafin@tarantool.org> (raw)

* Version #5 changes:

  - After performance evaluations it was decided to switch data type used
    by `datetime.secs` from `int64_t` (which is boxed in LuaJIT) to
    `double` (which is unboxed number).
  - please see perf/datetime*.cc perf test as a benchmark we used for
    comparison of 2 veriants of C implementation. Results are here - 
    https://gist.github.com/tsafin/618fbf847d258f6e7f5a75fdf9ea945b

* Version #4 changes:

A lot.

  - Got rid of closures usage here and there. Benchmarks revealed there is 
    20x speed difference of hash with closure handlers to the case when we 
    check keys using sequence of simple ifs.
    And despite the fact that it looks ugler, we have updated all places which
    used to use closures - so much performance impact does not worth it;

  - c-dt library now is accompanied with corresponding unit-tests;

  - due to a new (old) way of exporting public symbols, which is now used by 
    Tarantool build, we have modified c-dt sources to use approach similar 
    to that used by xxhash (or ICU) - "namespaces" for public symbols.
    If there is DT_NAMESPACE macro (e.g. DT_NAMESPACE=tnt_) defined at the 
    moment of compilation we rename all relevant public symbols to 
    use that "namespace" as prefix
 
	#define dt_from_rdn DT_NAME(DT_NAMESPACE, dt_from_rdn)

    C sources continue to use original name (now as a macro), but in Lua ffi
    we have to use renamed symbol - `tnt_dt_from_rdn`.

  - We have separated code which implement MessagePack related funcionality
    from generic (i.e. stringization) support for datetime types.
    Stringization is in `src/core/datetime.c`, but MessagePack support is in 
    `src/core/mp_datetime.c`.

  - `ctime` and `asctime` now use reentrant versions (i.e. `ctime_r` instead of 
    `ctime`, and `asctime_r` instead of `asctime`). This allows us to avoid 
    nasty problem with GC where it was possible to corrupt returned 
    static buffer regardless the fact that it was immediately passed
    to `ffi.string`. 
    `test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua` updated to check our 
    new api `datetime.ctime`, and `datetime.asctime`. 

  - Introduced MessagePack unit-test which check C implementation (mp 
    encode/decode, mp_snprint, mp_fprint) 

  - Some special efforts have been taken to make sure we deal correctly 
    with datetime stamps which were before Unix Epoch (1970-01-01), i.e. 
    for negative stamps or timezones against epoch.

  - And last, but not the least: in addition to usual getters like 
    `timestamp`, `nanoseconds`, `epoch`, we have introduced function 
    modifiers `to_utc()` or `to_tz(offset)` which allow to manipulate with 
    datetime timezone `.offset` field. Actually it does not do much now 
    (only reattributing object with different offset, but not changed 
    actual timestamp, which is UTC normalized). But different timestamp 
    modifies textual representation, which simplify debugging.

* Version #3 changes:

  - renamed `struct datetime_t` to `struct datetime`, and `struct 
    datetime_interval_t` to `struct datetime_interval`;
  - significantly reworked arguments checks in module api entries;
  - fixed datetime comparisons;
  - changed hints calculation to take into account fractional part;
  - provided more comments here and there;

NB! There are MacOSX problems due to GLIBC specific code used (as Vlad
    has already pointed out) - so additional patch, making it more 
    cross-compatible coming here soon...

* Version #2 changes:

  - fixed problem with overloaded '-' and '+' operations for datetime 
    arguments;
  - fixed messagepack serialization problems;
  - heavily documented MessagePack serialization schema in the code;
  - introduced working implementation of datetime hints for storage engines;
  - made interval related names be more consistent, renamed durations and period 
    to intervals, i.e. t_datetime_duration to datetime_interval_t, 
    duration_* to interval_*, period to interval;
  - properly implemented all reasonable cases of datetime+interval 
    arithmetic;
  - moved all initialization code to utils.c;
  - renamed core/mp_datetime.c to core/datetime.c because it makes more
    sense now;

* Version #1 - initial RFC

In brief
--------

This patchset implements datetime lua support in box, with serialization 
to messagepack, yaml, json and lua mode. Also it contains storage 
engines' indices implementation for datetime type introduced.

* Current implementation is heavily influenced by Sci-Lua lua-time module
  https://github.com/stepelu/lua-time 
  e.g. you could find very similar approach for handling of operations
  with year or month long intervals (which should be handled differently than
  usual intervals of seconds, or days).

* But internally we actually use Christian Hanson' c-dt module
  https://github.com/chansen/c-dt 
  (though it has been modified slightly for cleaner integration
  into our cmake-based build process).


Datetime Module API
-------------------

Small draft of documentation for our `datetime` module API has been
extracted to the discussion topic there - 
https://github.com/tarantool/tarantool/discussions/6244#discussioncomment-1043988
We try to update documentation there once any relevant change introdcued 
to the implementaton.

Messagepack serialization schema
--------------------------------

In short it looks like:
- now we introduce new MP_EXT extension type #4;
- we may save 1 required and 2 optional fields for datetime field;

In all gory details it's explained in MessagePack serialization schema depicted here:
https://github.com/tarantool/tarantool/discussions/6244#discussioncomment-1043990


https://github.com/tarantool/tarantool/issues/5941
https://github.com/tarantool/tarantool/issues/5946

https://github.com/tarantool/tarantool/tree/tsafin/gh-5941-datetime-v4

Timur Safin (8):
  build: add Christian Hansen c-dt to the build
  lua: built-in module datetime
  lua, datetime: display datetime
  box, datetime: messagepack support for datetime
  box, datetime: datetime comparison for indices
  lua, datetime: time intervals support
  datetime: perf test for datetime parser
  datetime: changelog for datetime module

 .gitmodules                                   |   3 +
 CMakeLists.txt                                |   8 +
 .../gh-5941-datetime-type-support.md          |   4 +
 cmake/BuildCDT.cmake                          |  10 +
 extra/exports                                 |  33 +
 perf/CMakeLists.txt                           |   3 +
 perf/datetime-common.h                        | 105 +++
 perf/datetime-compare.cc                      | 213 +++++
 perf/datetime-parser.cc                       | 105 +++
 src/CMakeLists.txt                            |   5 +-
 src/box/field_def.c                           |  53 +-
 src/box/field_def.h                           |   4 +
 src/box/lua/serialize_lua.c                   |   7 +-
 src/box/memtx_space.c                         |   3 +-
 src/box/msgpack.c                             |   7 +-
 src/box/tuple_compare.cc                      |  77 ++
 src/box/vinyl.c                               |   3 +-
 src/lib/core/CMakeLists.txt                   |   5 +-
 src/lib/core/datetime.c                       | 176 ++++
 src/lib/core/datetime.h                       | 115 +++
 src/lib/core/mp_datetime.c                    | 189 ++++
 src/lib/core/mp_datetime.h                    |  89 ++
 src/lib/core/mp_extension_types.h             |   1 +
 src/lib/mpstream/mpstream.c                   |  11 +
 src/lib/mpstream/mpstream.h                   |   4 +
 src/lua/datetime.lua                          | 880 ++++++++++++++++++
 src/lua/init.c                                |   4 +-
 src/lua/msgpack.c                             |  12 +
 src/lua/msgpackffi.lua                        |  18 +
 src/lua/serializer.c                          |   4 +
 src/lua/serializer.h                          |   2 +
 src/lua/utils.c                               |  28 +-
 src/lua/utils.h                               |  12 +
 test/app-tap/datetime.test.lua                | 370 ++++++++
 .../gh-5632-6050-6259-gc-buf-reuse.test.lua   |  74 +-
 test/engine/datetime.result                   |  77 ++
 test/engine/datetime.test.lua                 |  35 +
 test/unit/CMakeLists.txt                      |   3 +-
 test/unit/datetime.c                          | 381 ++++++++
 test/unit/datetime.result                     | 471 ++++++++++
 third_party/c-dt                              |   1 +
 third_party/lua-cjson/lua_cjson.c             |   8 +
 third_party/lua-yaml/lyaml.cc                 |   6 +-
 43 files changed, 3591 insertions(+), 28 deletions(-)
 create mode 100644 changelogs/unreleased/gh-5941-datetime-type-support.md
 create mode 100644 cmake/BuildCDT.cmake
 create mode 100644 perf/datetime-common.h
 create mode 100644 perf/datetime-compare.cc
 create mode 100644 perf/datetime-parser.cc
 create mode 100644 src/lib/core/datetime.c
 create mode 100644 src/lib/core/datetime.h
 create mode 100644 src/lib/core/mp_datetime.c
 create mode 100644 src/lib/core/mp_datetime.h
 create mode 100644 src/lua/datetime.lua
 create mode 100755 test/app-tap/datetime.test.lua
 create mode 100644 test/engine/datetime.result
 create mode 100644 test/engine/datetime.test.lua
 create mode 100644 test/unit/datetime.c
 create mode 100644 test/unit/datetime.result
 create mode 160000 third_party/c-dt

-- 
2.29.2


             reply	other threads:[~2021-08-15 23:59 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-15 23:59 Timur Safin via Tarantool-patches [this message]
2021-08-15 23:59 ` [Tarantool-patches] [PATCH v5 1/8] build: add Christian Hansen c-dt to the build Timur Safin via Tarantool-patches
2021-08-17 12:15   ` Serge Petrenko via Tarantool-patches
2021-08-17 23:24     ` Safin Timur via Tarantool-patches
2021-08-18  8:56       ` Serge Petrenko via Tarantool-patches
2021-08-17 15:50   ` Vladimir Davydov via Tarantool-patches
2021-08-18 10:04     ` Safin Timur via Tarantool-patches
2021-08-15 23:59 ` [Tarantool-patches] [PATCH v5 2/8] lua: built-in module datetime Timur Safin via Tarantool-patches
2021-08-17 12:15   ` Serge Petrenko via Tarantool-patches
2021-08-17 23:30     ` Safin Timur via Tarantool-patches
2021-08-18  8:56       ` Serge Petrenko via Tarantool-patches
2021-08-17 16:52   ` Vladimir Davydov via Tarantool-patches
2021-08-17 19:16     ` Vladimir Davydov via Tarantool-patches
2021-08-18 13:38       ` Safin Timur via Tarantool-patches
2021-08-18 10:03     ` Safin Timur via Tarantool-patches
2021-08-18 10:06       ` Safin Timur via Tarantool-patches
2021-08-18 11:45       ` Vladimir Davydov via Tarantool-patches
2021-08-15 23:59 ` [Tarantool-patches] [PATCH v5 3/8] lua, datetime: display datetime Timur Safin via Tarantool-patches
2021-08-17 12:15   ` Serge Petrenko via Tarantool-patches
2021-08-17 23:32     ` Safin Timur via Tarantool-patches
2021-08-17 17:06   ` Vladimir Davydov via Tarantool-patches
2021-08-18 14:10     ` Safin Timur via Tarantool-patches
2021-08-15 23:59 ` [Tarantool-patches] [PATCH v5 4/8] box, datetime: messagepack support for datetime Timur Safin via Tarantool-patches
2021-08-16  0:20   ` Safin Timur via Tarantool-patches
2021-08-17 12:15     ` Serge Petrenko via Tarantool-patches
2021-08-17 12:16   ` Serge Petrenko via Tarantool-patches
2021-08-17 23:42     ` Safin Timur via Tarantool-patches
2021-08-18  9:01       ` Serge Petrenko via Tarantool-patches
2021-08-17 18:36   ` Vladimir Davydov via Tarantool-patches
2021-08-18 14:27     ` Safin Timur via Tarantool-patches
2021-08-15 23:59 ` [Tarantool-patches] [PATCH v5 5/8] box, datetime: datetime comparison for indices Timur Safin via Tarantool-patches
2021-08-17 12:16   ` Serge Petrenko via Tarantool-patches
2021-08-17 23:43     ` Safin Timur via Tarantool-patches
2021-08-18  9:03       ` Serge Petrenko via Tarantool-patches
2021-08-17 19:05   ` Vladimir Davydov via Tarantool-patches
2021-08-18 17:18     ` Safin Timur via Tarantool-patches
2021-08-15 23:59 ` [Tarantool-patches] [PATCH v5 6/8] lua, datetime: time intervals support Timur Safin via Tarantool-patches
2021-08-17 12:16   ` Serge Petrenko via Tarantool-patches
2021-08-17 23:44     ` Safin Timur via Tarantool-patches
2021-08-17 18:52   ` Vladimir Davydov via Tarantool-patches
2021-08-15 23:59 ` [Tarantool-patches] [PATCH v5 7/8] datetime: perf test for datetime parser Timur Safin via Tarantool-patches
2021-08-17 19:13   ` Vladimir Davydov via Tarantool-patches
2021-08-15 23:59 ` [Tarantool-patches] [PATCH v5 8/8] datetime: changelog for datetime module Timur Safin via Tarantool-patches
2021-08-17 12:16   ` Serge Petrenko via Tarantool-patches
2021-08-17 23:44     ` Safin Timur via Tarantool-patches
2021-08-18  9:04       ` Serge Petrenko via Tarantool-patches
2021-08-17 12:15 ` [Tarantool-patches] [PATCH v5 0/8] Initial datetime implementation Serge Petrenko via Tarantool-patches
     [not found] ` <20210818082222.mofgheciutpipelz@esperanza>
2021-08-18  8:25   ` Vladimir Davydov via Tarantool-patches
2021-08-18 13:24     ` Safin Timur via Tarantool-patches
2021-08-18 14:22       ` Vladimir Davydov via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1629071531.git.tsafin@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imeevma@tarantool.org \
    --cc=imun@tarantool.org \
    --cc=tsafin@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v5 0/8] Initial datetime implementation' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox