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

Shortcut to avoid fmod #66909

Merged
merged 1 commit into from Jan 20, 2021
Merged

Shortcut to avoid fmod #66909

merged 1 commit into from Jan 20, 2021

Conversation

gf2121
Copy link
Contributor

@gf2121 gf2121 commented Jan 3, 2021

Description

When benchmarking terms query (10000+ terms) on long type, I was supperised to find that the part costing most CPU is to judge whether terms have a decimal part because we execute a double mod for each long. So doing some check before mod may make sense.

image

@cla-checker-service
Copy link

cla-checker-service bot commented Jan 3, 2021

💚 CLA has been signed

@gf2121 gf2121 closed this Jan 3, 2021
@gf2121 gf2121 reopened this Jan 3, 2021
@matriv matriv added the :Search/Mapping Index mappings, including merging and defining field types label Jan 4, 2021
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Jan 4, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

@gf2121
Copy link
Contributor Author

gf2121 commented Jan 19, 2021

@matriv Hi, sorry to disturb. Could you please help take a look at this small enhancement when you are free? Thanks!

@matriv
Copy link
Contributor

matriv commented Jan 20, 2021

@elasticmachine test this please

@gf2121
Copy link
Contributor Author

gf2121 commented Jan 20, 2021

@matriv Some checks have failed for the reason that seems not related to this PR. I've rebased this commit on the newest master branch. Could you please help trigger tests again?

@matriv
Copy link
Contributor

matriv commented Jan 20, 2021

@elasticmachine test this please

@gf2121
Copy link
Contributor Author

gf2121 commented Jan 20, 2021

@matriv Hi, checks have passed :)

@matriv
Copy link
Contributor

matriv commented Jan 20, 2021

I can see @gf2121 :)

Thanks a lot for your contribution.

@matriv matriv merged commit c32f8b1 into elastic:master Jan 20, 2021
matriv added a commit that referenced this pull request Jan 20, 2021
…67752)

Benchmarking terms query (10000+ terms) on long type,
shows that the code part costing most CPU is when trying to judge
whether terms have a decimal part because a double mod is executed
for each number, regardless of if it has a decimal part.

Adding a check before the mod take place to exclude non-decimal
numbers.

(cherry picked from commit c32f8b1)

Co-authored-by: gf2121 <52390227+gf2121@users.noreply.github.com>
alyokaz pushed a commit to alyokaz/elasticsearch that referenced this pull request Mar 10, 2021
…6909)

Benchmarking terms query (10000+ terms) on long type, 
shows that the code part costing most CPU is when trying to judge
whether terms have a decimal part because a double mod is executed
for each number, regardless of if it has a decimal part. 

Adding a check before the mod take place to exclude non-decimal
numbers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search/Mapping Index mappings, including merging and defining field types Team:Search Meta label for search team v7.12.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants