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

Specialize intDiv/module #21307

Merged
merged 1 commit into from
Mar 3, 2021
Merged

Conversation

amosbird
Copy link
Collaborator

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 (a user-readable short description of the changes that goes to CHANGELOG.md):

Add back intDiv/module vectorConstant specializations for better performance. This fixes #21293 . The regression was introduced in #18145 .

Detailed description / Documentation draft:

.

@robot-clickhouse robot-clickhouse added the pr-performance Pull request with some performance improvements label Feb 28, 2021
@kitaisreal kitaisreal self-assigned this Feb 28, 2021
@kitaisreal
Copy link
Contributor

@amosbird could you please add perf test from linked issue ?

@amosbird
Copy link
Collaborator Author

amosbird commented Mar 2, 2021

@kitaisreal There is already a test https://github.com/ClickHouse/ClickHouse/blob/master/tests/performance/modulo.xml for this. Maybe the difference is not evident.

@akuzm
Copy link
Contributor

akuzm commented Mar 2, 2021

@kitaisreal There is already a test https://github.com/ClickHouse/ClickHouse/blob/master/tests/performance/modulo.xml for this. Maybe the difference is not evident.

There was an obvious difference but somehow the test was not marked as failed. I'll investigate.

@alexey-milovidov alexey-milovidov merged commit bd7b540 into ClickHouse:master Mar 3, 2021
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.

Modulo of division is not fast.
5 participants