Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved performance of max(), min(), argMin(), argMax() for DateTime64 #8199

Merged
merged 1 commit into from
Dec 13, 2019

Conversation

Enmk
Copy link
Contributor

@Enmk Enmk commented Dec 13, 2019

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

  • Performance Improvement

Changelog entry (up to few sentences, required except for Non-significant/Documentation categories):
Improved performance of max(), min(), argMin(), argMax() for DateTime64
...

Detailed description (optional):
This boost performance of min/max on my machine by factor of 2:
pre-fix query performance on hits_v1 dataset with perftest:

Query Execution time (seconds)
SELECT max(x) FROM dt [0.006]
SELECT max(x) FROM dt64 [0.046]

max on DateTime64 is 0.046 / 0.006 = 7.666666666666666 slower (A)

post-fix performance:

Query Execution time (seconds)
SELECT max(x) FROM dt [0.005]
SELECT max(x) FROM dt64 [0.018]

max on DateTime64 is 0.018 / 0.005 = 3.5999999999999996 slower (B)

Gain is:
(A - B) / A * 100 % = 53%

@alexey-milovidov
Copy link
Member

Is this code identical to what we already have in master?

@Enmk
Copy link
Contributor Author

Enmk commented Dec 13, 2019

Is this code identical to what we already have in master?

almost, I haven't noticed that something like that was already done. However, there are missing specializations in master:

if (which.idx == TypeIndex::DateTime64)
   // ...

Let me fixup the PR to include only that.

@Enmk
Copy link
Contributor Author

Enmk commented Dec 13, 2019

Let me fixup the PR to include only that.

Done

@alexey-milovidov alexey-milovidov merged commit 86e1cbf into ClickHouse:master Dec 13, 2019
@KochetovNicolai KochetovNicolai added the pr-performance Pull request with some performance improvements label Dec 16, 2019
@Enmk Enmk deleted the DateTime64_performance branch December 8, 2020 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-performance Pull request with some performance improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants