Tarantool development patches archive
 help / color / mirror / Atom feed
From: Safin Timur via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Serge Petrenko <sergepetrenko@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 13:29:06 +0300	[thread overview]
Message-ID: <a4f22464-5279-1b45-75aa-d990fe1f86a3@tarantool.org> (raw)
In-Reply-To: <82319534-df35-ceb6-ee22-cb4cbaff5e41@tarantool.org>

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?

Thanks,
Timur

  reply	other threads:[~2021-08-19 10:29 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 [this message]
2021-08-19 11:11       ` Serge Petrenko via Tarantool-patches
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=a4f22464-5279-1b45-75aa-d990fe1f86a3@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