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

Count directly into the values array in FastTaxonomyFacetCounts#countAl [LUCENE-10379] #11415

Closed
asfimport opened this issue Jan 13, 2022 · 6 comments

Comments

@asfimport
Copy link

asfimport commented Jan 13, 2022

This breaks out one of the two optimizations proposed in #11386. As part of trying to track down the regressions caused by #11386, I propose we push just this one optimization independently (see LUCENE-10374).


Migrated from LUCENE-10379 by Greg Miller (@gsmiller), resolved Jan 18 2022
Pull requests: #605, #610

@asfimport
Copy link
Author

ASF subversion and git services (migrated from JIRA)

Commit 2f5e3c3 in lucene's branch refs/heads/main from Greg Miller
https://gitbox.apache.org/repos/asf?p=lucene.git;h=2f5e3c3

LUCENE-10379: Count directly into the dense values array in FastTaxonomyFacetCounts#countAll (#605)

Co-authored-by: guofeng.my <guofeng.my@bytedance.com>

@asfimport
Copy link
Author

Greg Miller (@gsmiller) (migrated from JIRA)

Merged this and will watch the next nightly bench runs. Going to wait to put anything on 9.x until we sort out the regressions.

@asfimport
Copy link
Author

asfimport commented Jan 14, 2022

Greg Miller (@gsmiller) (migrated from JIRA)

Looks like this change worked! The performance improvement carried over into the nightly benchmarks this time with no regressions (e.g., https://home.apache.org/\~mikemccand/lucenebench/BrowseMonthTaxoFacets.html). So I strongly suspect the issue before (in LUCENE-10350) was changing the for-loop to a while-loop. I suspect the compiler is able to optimize the for-loop construct on the nightly benchmark hardware in a way that it can't with the while-loop (???). I'm going to give this a try (created #11416 to track).

@asfimport
Copy link
Author

ASF subversion and git services (migrated from JIRA)

Commit c5c082e in lucene's branch refs/heads/branch_9x from Greg Miller
https://gitbox.apache.org/repos/asf?p=lucene.git;h=c5c082e

LUCENE-10379: Count directly into the dense values array in FastTaxonomyFacetCounts#countAll

Co-authored-by: guofeng.my <guofeng.my@bytedance.com>

@asfimport
Copy link
Author

Greg Miller (@gsmiller) (migrated from JIRA)

This has now been backported to 9.x. Resolving.

@asfimport
Copy link
Author

Adrien Grand (@jpountz) (migrated from JIRA)

Closing after the 9.1.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment