From: Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Safin Timur <tsafin@tarantool.org>, vdavydov@tarantool.org, tarantool-patches@dev.tarantool.org Cc: v.shpilevoy@tarantool.org Subject: Re: [Tarantool-patches] [PATCH v6 4/5] datetime: perf test for datetime parser Date: Thu, 19 Aug 2021 14:11:35 +0300 [thread overview] Message-ID: <27cf13a8-5ef8-536e-8e17-fb5e78078a41@tarantool.org> (raw) In-Reply-To: <a4f22464-5279-1b45-75aa-d990fe1f86a3@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
next prev parent reply other threads:[~2021-08-19 11:11 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-08-19 2:56 [Tarantool-patches] [PATCH v6 0/5] Initial datetime implementation Timur Safin via Tarantool-patches 2021-08-19 2:56 ` [Tarantool-patches] [PATCH v6 1/5] build, lua: built-in module datetime Timur Safin via Tarantool-patches 2021-08-19 9:43 ` Serge Petrenko via Tarantool-patches 2021-08-19 9:47 ` Safin Timur via Tarantool-patches 2021-08-19 15:26 ` Vladimir Davydov via Tarantool-patches 2021-08-24 21:13 ` Vladislav Shpilevoy via Tarantool-patches 2021-08-19 2:56 ` [Tarantool-patches] [PATCH v6 2/5] box, datetime: messagepack support for datetime Timur Safin via Tarantool-patches 2021-08-19 9:58 ` Serge Petrenko via Tarantool-patches 2021-08-19 2:56 ` [Tarantool-patches] [PATCH v6 3/5] box, datetime: datetime comparison for indices Timur Safin via Tarantool-patches 2021-08-19 10:16 ` Serge Petrenko via Tarantool-patches 2021-08-19 11:18 ` UNera via Tarantool-patches 2021-08-19 11:53 ` Safin Timur via Tarantool-patches 2021-08-19 14:47 ` Dmitry E. Oboukhov via Tarantool-patches 2021-08-19 2:56 ` [Tarantool-patches] [PATCH v6 4/5] datetime: perf test for datetime parser Timur Safin via Tarantool-patches 2021-08-19 10:19 ` Serge Petrenko via Tarantool-patches 2021-08-19 10:29 ` Safin Timur via Tarantool-patches 2021-08-19 11:11 ` Serge Petrenko via Tarantool-patches [this message] 2021-08-19 15:58 ` Vladimir Davydov via Tarantool-patches 2021-08-19 2:56 ` [Tarantool-patches] [PATCH v6 5/5] datetime: changelog for datetime module Timur Safin via Tarantool-patches 2021-08-19 10:20 ` Serge Petrenko 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=27cf13a8-5ef8-536e-8e17-fb5e78078a41@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=sergepetrenko@tarantool.org \ --cc=tsafin@tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --cc=vdavydov@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v6 4/5] datetime: perf test for datetime parser' \ /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