Add singleton optimization for DateHistogramAggregator #17643
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
❌ Gradle check result for 9c88ccd: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
Hello! |
if you are expecting tangible performance improvements we recommend you to run a performance benchmark on your pull request, refer the README mentioned in the comment. Recommend you to use |
|
❌ Gradle check result for a1cade1: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
{"run-benchmark-test": "id_4"} |
|
{"run-benchmark-test": "id_4"} |
|
The Jenkins job url is https://build.ci.opensearch.org/job/benchmark-pull-request/2683/ . Final results will be published once the job is completed. |
|
{"run-benchmark-test": "id_3"} |
|
The Jenkins job url is https://build.ci.opensearch.org/job/benchmark-pull-request/2685/ . Final results will be published once the job is completed. |
Benchmark ResultsBenchmark Results for Job: https://build.ci.opensearch.org/job/benchmark-pull-request/2683/
|
Benchmark ResultsBenchmark Results for Job: https://build.ci.opensearch.org/job/benchmark-pull-request/2685/
|
Benchmark Baseline Comparison ResultsBenchmark Results for Job: https://build.ci.opensearch.org/job/benchmark-compare/45/
|
Benchmark Baseline Comparison ResultsBenchmark Results for Job: https://build.ci.opensearch.org/job/benchmark-compare/46/
|
shreyah963
force-pushed
the
changesindatehistogram
branch
from
a1cade1 to
589bca3
Compare
|
❌ Gradle check result for 688a24d: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: shreyah963 <shreyab963@gmail.com>
Signed-off-by: shreyah963 <shreyab963@gmail.com>
Signed-off-by: shreyah963 <shreyab963@gmail.com>
Signed-off-by: shreyah963 <shreyab963@gmail.com>
Signed-off-by: shreyah963 <shreyab963@gmail.com>
shreyah963
force-pushed
the
changesindatehistogram
branch
from
688a24d to
c0aca71
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17643 +/- ##
============================================
- Coverage 72.46% 72.39% -0.07%
+ Complexity 66502 66407 -95
============================================
Files 5408 5408
Lines 308080 308164 +84
Branches 44720 44737 +17
============================================
- Hits 223239 223094 -145
- Misses 66536 66782 +246
+ Partials 18305 18288 -17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
66 tasks
shreyah963
requested review from
Rishikesh1159,
VachaShah,
bugmakerrrrrr,
linuxpi,
owaiskazi19,
reta,
sachinpkale,
saratvemulapalli,
shwetathareja and
sohami
as code owners
...c/main/java/org/opensearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java
Outdated
Show resolved
Hide resolved
Co-authored-by: bowenlan-amzn <bowenlan23@gmail.com> Signed-off-by: shreyah963 <shreyab963@gmail.com>
calculation in multi-valued case Signed-off-by: shreyah963 <shreyab963@gmail.com>
bowenlan-amzn
approved these changes
Apr 8, 2025
jainankitk
approved these changes
Apr 8, 2025
|
The backport to To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-17643-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 5c7cced105b5fe3d50bfab32afe5aa242863274b
# Push it to GitHub
git push --set-upstream origin backport/backport-17643-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.xThen, create a pull request where the |
This was referenced Apr 14, 2025
Open
Harsh-87
pushed a commit
to Harsh-87/OpenSearch
that referenced
this pull request
May 7, 2025
…oject#17643) * Added Singleton DocValue Functionality Signed-off-by: shreyah963 <shreyab963@gmail.com> * sync jvm.options Signed-off-by: shreyah963 <shreyab963@gmail.com> * added override in getLeafCollector method Signed-off-by: shreyah963 <shreyab963@gmail.com> * disabled jvm debug port used for testing Signed-off-by: shreyah963 <shreyab963@gmail.com> * Fix code formatting issues Signed-off-by: shreyah963 <shreyab963@gmail.com> * Update CHANGELOG.md Signed-off-by: shreyah963 <shreyab963@gmail.com> * Restore Debug_OpenSearch.xml Signed-off-by: shreyah963 <shreyab963@gmail.com> * restore Debug_OpenSearch.xml Signed-off-by: shreyah963 <shreyab963@gmail.com> * Update CHANGELOG.md Co-authored-by: bowenlan-amzn <bowenlan23@gmail.com> Signed-off-by: shreyah963 <shreyab963@gmail.com> * Move rounding calculation out of collectValue method to avoid redundant calculation in multi-valued case Signed-off-by: shreyah963 <shreyab963@gmail.com> --------- Signed-off-by: shreyah963 <shreyab963@gmail.com> Co-authored-by: bowenlan-amzn <bowenlan23@gmail.com> Signed-off-by: Harsh Kothari <techarsh@amazon.com>
Harsh-87
pushed a commit
to Harsh-87/OpenSearch
that referenced
this pull request
May 7, 2025
…oject#17643) * Added Singleton DocValue Functionality Signed-off-by: shreyah963 <shreyab963@gmail.com> * sync jvm.options Signed-off-by: shreyah963 <shreyab963@gmail.com> * added override in getLeafCollector method Signed-off-by: shreyah963 <shreyab963@gmail.com> * disabled jvm debug port used for testing Signed-off-by: shreyah963 <shreyab963@gmail.com> * Fix code formatting issues Signed-off-by: shreyah963 <shreyab963@gmail.com> * Update CHANGELOG.md Signed-off-by: shreyah963 <shreyab963@gmail.com> * Restore Debug_OpenSearch.xml Signed-off-by: shreyah963 <shreyab963@gmail.com> * restore Debug_OpenSearch.xml Signed-off-by: shreyah963 <shreyab963@gmail.com> * Update CHANGELOG.md Co-authored-by: bowenlan-amzn <bowenlan23@gmail.com> Signed-off-by: shreyah963 <shreyab963@gmail.com> * Move rounding calculation out of collectValue method to avoid redundant calculation in multi-valued case Signed-off-by: shreyah963 <shreyab963@gmail.com> --------- Signed-off-by: shreyah963 <shreyab963@gmail.com> Co-authored-by: bowenlan-amzn <bowenlan23@gmail.com> Signed-off-by: Harsh Kothari <techarsh@amazon.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Optimized the 'DateHistogramAggregator' to provide a specialized fast path for single-valued date fields. This optimization reduces method call overhead and eliminates unnecessary iteration logic when processing single values.
Key Changes
Single-Value Optimization
DocValues.unwrapSingleton()Before vs After Comparison
Before (Original Implementation):
After (Optimized Implementation):
Technical Details
Method Call Reduction:
advanceExact → docValueCount → nextValue → roundadvanceExact → longValueMemory Access Pattern:
Code Path Optimization:
Performance Impact
90th percentile service time (ms)
Before Optimization
The flame graph shows a nextValue() call, followed by a visible gap, and then a longValue operation.
After Optimization
The flame graph shows a direct path to longValue without any gaps or intermediate nextValue() calls, demonstrating more efficient execution with eliminated overhead.
Testing Performance Improvement
Infrastructure Details
Test Environment
Filter rewrite optimization disabled:
{ "persistent": { "search.max_aggregation_rewrite_filters": 0 } }Benchmark command:
Run 1
Before change
After change
Run 2
Before change
After change
Run 3
Before change
After change
Run 4
Before change
After change
Run 5
Before change
After change
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.