Improve performance of encoding composite keys in multi-term aggregations #9412

ketanv3 · 2023-08-17T08:01:19Z

Description

Composite keys of arbitrary Java objects are encoded using the StreamOutput::writeGenericValue method (here) which is expensive as it involves a series of "instanceof" checks and map lookups to identify the registered writer. This bottleneck adds up as the number of composite keys is proportional to the number of hits, number of multi-term fields, and the number of field values.

This PR improves the encoding performance by:

Removing unnecessary writer lookups.
Reusing partial encodings of shared prefixes to avoid wasteful work.

Improvement

Improvement is proportional to the number of hits, fields, and field values. The improvement is significant!

Test	p50 before	p90 before	p50 after	p90 after
PR #2687 / mix	5500 ms	5605 ms	4023 ms	4185 ms
PR #2687 / numeric	1688 ms	1756 ms	538 ms	591 ms
OSB / http_logs / multi_term_agg	3204 ms	3243 ms	2241 ms	2282 ms

Related Issues

#8710

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff
Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

…ions Signed-off-by: Ketan Verma <[email protected]>

opensearch-trigger-bot · 2023-08-17T08:28:19Z

Compatibility status:

Checks if related components are compatible with change e1c40b4

Incompatible components

Incompatible components: [https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/security-analytics.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git]

github-actions · 2023-08-17T08:40:34Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/22966/
CommitID: 81d928a

codecov · 2023-08-17T08:41:31Z

Codecov Report

Merging #9412 (3178f37) into main (6a5b464) will decrease coverage by 0.04%.
The diff coverage is 94.64%.

@@             Coverage Diff              @@
##               main    #9412      +/-   ##
============================================
- Coverage     71.12%   71.08%   -0.04%     
  Complexity    57417    57417              
============================================
  Files          4776     4776              
  Lines        270742   270759      +17     
  Branches      39578    39577       -1     
============================================
- Hits         192558   192478      -80     
- Misses        62044    62120      +76     
- Partials      16140    16161      +21

Files Changed	Coverage Δ
...ggregations/bucket/terms/MultiTermsAggregator.java	`82.07% <93.75%> (+2.88%)`	⬆️
...opensearch/core/common/io/stream/StreamOutput.java	`94.88% <100.00%> (-0.79%)`	⬇️

... and 428 files with indirect coverage changes

nknize

Big heart here for this change! Nice clean up not having to deal with all of this introspection.

I left one minor nitpick purely out of preference but I won't block the PR for it.

Thx for this clean change!

libs/core/src/main/java/org/opensearch/core/common/io/stream/StreamOutput.java

…Class Signed-off-by: Ketan Verma <[email protected]>

github-actions · 2023-08-17T14:07:35Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/22974/
CommitID: 951e813

libs/core/src/main/java/org/opensearch/core/common/io/stream/StreamOutput.java

backslasht

Nice improvement 🥇
Looks good to me. Thanks @ketanv3

server/src/main/java/org/opensearch/search/aggregations/bucket/terms/MultiTermsAggregator.java

…m-agg

…move unused code Signed-off-by: Ketan Verma <[email protected]>

github-actions · 2023-08-17T18:27:12Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/22988/
CommitID: 3178f37

opensearch-trigger-bot · 2023-08-17T21:31:23Z

Compatibility status:

Checks if related components are compatible with change 6a5b464

Incompatible components

Incompatible components: [https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/asynchronous-search.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/reporting.git]

…ions (#9412) * Improve performance of encoding composite keys in multi-term aggregations Signed-off-by: Ketan Verma <[email protected]> * Rename StreamOutput::getGenericValueWriter to getGenericValueWriterByClass Signed-off-by: Ketan Verma <[email protected]> * Rename StreamOutput::getGenericValueWriterByClass to getWriter and remove unused code Signed-off-by: Ketan Verma <[email protected]> --------- Signed-off-by: Ketan Verma <[email protected]> (cherry picked from commit 46f2bd0) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…ions (#9412) (#9434) * Improve performance of encoding composite keys in multi-term aggregations * Rename StreamOutput::getGenericValueWriter to getGenericValueWriterByClass * Rename StreamOutput::getGenericValueWriterByClass to getWriter and remove unused code --------- (cherry picked from commit 46f2bd0) Signed-off-by: Ketan Verma <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…ions (opensearch-project#9412) * Improve performance of encoding composite keys in multi-term aggregations Signed-off-by: Ketan Verma <[email protected]> * Rename StreamOutput::getGenericValueWriter to getGenericValueWriterByClass Signed-off-by: Ketan Verma <[email protected]> * Rename StreamOutput::getGenericValueWriterByClass to getWriter and remove unused code Signed-off-by: Ketan Verma <[email protected]> --------- Signed-off-by: Ketan Verma <[email protected]>

…ions (opensearch-project#9412) * Improve performance of encoding composite keys in multi-term aggregations Signed-off-by: Ketan Verma <[email protected]> * Rename StreamOutput::getGenericValueWriter to getGenericValueWriterByClass Signed-off-by: Ketan Verma <[email protected]> * Rename StreamOutput::getGenericValueWriterByClass to getWriter and remove unused code Signed-off-by: Ketan Verma <[email protected]> --------- Signed-off-by: Ketan Verma <[email protected]> Signed-off-by: Gagan Juneja <[email protected]>

…ions (opensearch-project#9412) * Improve performance of encoding composite keys in multi-term aggregations Signed-off-by: Ketan Verma <[email protected]> * Rename StreamOutput::getGenericValueWriter to getGenericValueWriterByClass Signed-off-by: Ketan Verma <[email protected]> * Rename StreamOutput::getGenericValueWriterByClass to getWriter and remove unused code Signed-off-by: Ketan Verma <[email protected]> --------- Signed-off-by: Ketan Verma <[email protected]> Signed-off-by: Kiran Reddy <[email protected]>

…ions (opensearch-project#9412) * Improve performance of encoding composite keys in multi-term aggregations Signed-off-by: Ketan Verma <[email protected]> * Rename StreamOutput::getGenericValueWriter to getGenericValueWriterByClass Signed-off-by: Ketan Verma <[email protected]> * Rename StreamOutput::getGenericValueWriterByClass to getWriter and remove unused code Signed-off-by: Ketan Verma <[email protected]> --------- Signed-off-by: Ketan Verma <[email protected]> Signed-off-by: Kaushal Kumar <[email protected]>

…ions (opensearch-project#9412) * Improve performance of encoding composite keys in multi-term aggregations Signed-off-by: Ketan Verma <[email protected]> * Rename StreamOutput::getGenericValueWriter to getGenericValueWriterByClass Signed-off-by: Ketan Verma <[email protected]> * Rename StreamOutput::getGenericValueWriterByClass to getWriter and remove unused code Signed-off-by: Ketan Verma <[email protected]> --------- Signed-off-by: Ketan Verma <[email protected]> Signed-off-by: Ivan Brusic <[email protected]>

…ions (opensearch-project#9412) * Improve performance of encoding composite keys in multi-term aggregations Signed-off-by: Ketan Verma <[email protected]> * Rename StreamOutput::getGenericValueWriter to getGenericValueWriterByClass Signed-off-by: Ketan Verma <[email protected]> * Rename StreamOutput::getGenericValueWriterByClass to getWriter and remove unused code Signed-off-by: Ketan Verma <[email protected]> --------- Signed-off-by: Ketan Verma <[email protected]> Signed-off-by: Shivansh Arora <[email protected]>

Improve performance of encoding composite keys in multi-term aggregat…

81d928a

…ions Signed-off-by: Ketan Verma <[email protected]>

ketanv3 mentioned this pull request Aug 17, 2023

[Meta] Improve performance of multi-term aggregations #8710

Closed

ketanv3 marked this pull request as ready for review August 17, 2023 08:59

nknize approved these changes Aug 17, 2023

View reviewed changes

libs/core/src/main/java/org/opensearch/core/common/io/stream/StreamOutput.java Outdated Show resolved Hide resolved

Rename StreamOutput::getGenericValueWriter to getGenericValueWriterBy…

951e813

…Class Signed-off-by: Ketan Verma <[email protected]>

reta reviewed Aug 17, 2023

View reviewed changes

libs/core/src/main/java/org/opensearch/core/common/io/stream/StreamOutput.java Outdated Show resolved Hide resolved

backslasht approved these changes Aug 17, 2023

View reviewed changes

reta reviewed Aug 17, 2023

View reviewed changes

server/src/main/java/org/opensearch/search/aggregations/bucket/terms/MultiTermsAggregator.java Show resolved Hide resolved

ketanv3 added 2 commits August 17, 2023 23:14

Merge remote-tracking branch 'origin/main' into performance/multi-ter…

c3f0171

…m-agg

Rename StreamOutput::getGenericValueWriterByClass to getWriter and re…

3178f37

…move unused code Signed-off-by: Ketan Verma <[email protected]>

reta approved these changes Aug 17, 2023

View reviewed changes

reta merged commit 46f2bd0 into opensearch-project:main Aug 18, 2023
11 checks passed

reta added the backport 2.x Backport to 2.x branch label Aug 18, 2023

opensearch-trigger-bot bot mentioned this pull request Aug 18, 2023

[Backport 2.x] Improve performance of encoding composite keys in multi-term aggregations #9434

Merged

ketanv3 mentioned this pull request Aug 20, 2023

Performance improvements for BytesRefHash #8788

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve performance of encoding composite keys in multi-term aggregations #9412

Improve performance of encoding composite keys in multi-term aggregations #9412

ketanv3 commented Aug 17, 2023 •

edited

Loading

opensearch-trigger-bot bot commented Aug 17, 2023

github-actions bot commented Aug 17, 2023

codecov bot commented Aug 17, 2023 •

edited

Loading

nknize left a comment

github-actions bot commented Aug 17, 2023

backslasht left a comment

github-actions bot commented Aug 17, 2023

opensearch-trigger-bot bot commented Aug 17, 2023

Improve performance of encoding composite keys in multi-term aggregations #9412

Improve performance of encoding composite keys in multi-term aggregations #9412

Conversation

ketanv3 commented Aug 17, 2023 • edited Loading

Description

Improvement

Related Issues

Check List

opensearch-trigger-bot bot commented Aug 17, 2023

Compatibility status:

Incompatible components

Skipped components

Compatible components

github-actions bot commented Aug 17, 2023

Gradle Check (Jenkins) Run Completed with:

codecov bot commented Aug 17, 2023 • edited Loading

Codecov Report

nknize left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 17, 2023

Gradle Check (Jenkins) Run Completed with:

backslasht left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 17, 2023

Gradle Check (Jenkins) Run Completed with:

opensearch-trigger-bot bot commented Aug 17, 2023

Compatibility status:

Incompatible components

Skipped components

Compatible components

ketanv3 commented Aug 17, 2023 •

edited

Loading

codecov bot commented Aug 17, 2023 •

edited

Loading