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

Min-Max heap performance improvement #3430

Merged
merged 9 commits into from
Mar 2, 2023
Merged

Conversation

GuyAv46
Copy link
Collaborator

@GuyAv46 GuyAv46 commented Feb 26, 2023

simplifying and improving internal computations of the min-max heap, for better readability and performance

benchmark comparing changes to master code here

@filipecosta90
Copy link
Collaborator

filipecosta90 commented Feb 26, 2023

Automated performance analysis summary

This comment was automatically generated given there is performance data available.

In summary:

  • Detected a total of 13 stable tests between versions.
  • Detected a total of 3 highly unstable benchmarks.
  • Detected a total of 3 regressions bellow the regression water line 5.0.

You can check a comparison in detail via the grafana link

Comparison between master and guyav-minmax_heap_perf.

Time Period from 30 days ago. (environment used: oss-standalone)

Test Case Baseline master (median obs. +- std.dev) Comparison guyav-minmax_heap_perf (median obs. +- std.dev) % change (higher-better) Note
ftsb-10K-enwiki_abstract-hashes-term-prefix 8047 +- 2.6% (7 datapoints) 7820 +- nan% (1 datapoints) -2.8% -- no change --
ftsb-10K-enwiki_abstract-hashes-term-suffix 2060 +- 4.4% (7 datapoints) 2025 +- nan% (1 datapoints) -1.7% -- no change --
ftsb-10K-enwiki_abstract-hashes-term-suffix-withsuffixtrie 82611 +- 4.5% (7 datapoints) 75365 +- nan% (1 datapoints) -8.8% REGRESSION
ftsb-10K-enwiki_abstract-hashes-term-wildcard 13311 +- 4.1% (7 datapoints) 12775 +- nan% (1 datapoints) -4.0% potential REGRESSION
ftsb-10K-multivalue-numeric-json 840 +- 1.4% (7 datapoints) 831 +- nan% (1 datapoints) -1.0% -- no change --
ftsb-10K-singlevalue-numeric-json 369 +- 0.5% (7 datapoints) 365 +- nan% (1 datapoints) -1.1% -- no change --
ftsb-1K-enwiki_abstract-hashes-term-contains 1767 +- 3.8% (7 datapoints) 1735 +- nan% (1 datapoints) -1.8% -- no change --
ftsb-1M-enwiki_abstract-hashes-fulltext-2word-intersection-query 390 +- 5.3% (7 datapoints) 357 +- nan% (1 datapoints) -8.5% waterline=5.3%. REGRESSION
ftsb-1M-enwiki_abstract-hashes-fulltext-2word-union-query 2920 +- 3.2% (7 datapoints) 2778 +- nan% (1 datapoints) -4.9% potential REGRESSION
ftsb-1M-enwiki_abstract-hashes-fulltext-simple-1word-query 1086 +- 5.2% (7 datapoints) 1020 +- nan% (1 datapoints) -6.1% waterline=5.2%. REGRESSION
ftsb-1M-enwiki_abstract-hashes-load 23312 +- 2.8% (7 datapoints) 23131 +- nan% (1 datapoints) -0.8% -- no change --
ftsb-1M-nyc_taxis-ftadd-load 21641 +- 2.0% (7 datapoints) 20847 +- nan% (1 datapoints) -3.7% potential REGRESSION
ftsb-1M-nyc_taxis-hashes-load 22540 +- 1.9% (7 datapoints) 22033 +- nan% (1 datapoints) -2.2% -- no change --
search-ftsb-10K-enwiki_abstract-hashes-term-withoutsuffix-trie 36148 +- 1.3% (7 datapoints) 36129 +- nan% (1 datapoints) -0.1% -- no change --
search-ftsb-10K-enwiki_abstract-hashes-term-withsuffix-trie 36360 +- 2.2% (7 datapoints) 35785 +- nan% (1 datapoints) -1.6% -- no change --
search-geo 211 +- 2.5% (7 datapoints) 205 +- nan% (1 datapoints) -3.1% potential REGRESSION
search-numeric 3128 +- 35.5% UNSTABLE (7 datapoints) 3062 +- nan% (1 datapoints) -2.1% UNSTABLE (very high variance)
search-numeric-sortby 3332 +- 24.4% UNSTABLE (7 datapoints) 4974 +- nan% (1 datapoints) 49.3% UNSTABLE (very high variance)
search-numeric-sortby-desc 3280 +- 43.3% UNSTABLE (7 datapoints) 3601 +- nan% (1 datapoints) 9.8% UNSTABLE (very high variance)

@codecov
Copy link

codecov bot commented Feb 26, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 🎉

Comparison is base (68804f5) 82.71% compared to head (61b0522) 82.72%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3430      +/-   ##
==========================================
+ Coverage   82.71%   82.72%   +0.01%     
==========================================
  Files         175      175              
  Lines       29940    29945       +5     
==========================================
+ Hits        24765    24773       +8     
+ Misses       5175     5172       -3     
Impacted Files Coverage Δ
src/util/minmax_heap.c 90.19% <100.00%> (+0.33%) ⬆️
coord/src/rmr/conn.c 56.67% <0.00%> (+0.36%) ⬆️
coord/src/rmr/redis_cluster.c 82.81% <0.00%> (+3.12%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

src/util/minmax_heap.c Outdated Show resolved Hide resolved
src/util/minmax_heap.c Outdated Show resolved Hide resolved
src/util/minmax_heap.c Outdated Show resolved Hide resolved
src/util/minmax_heap.c Show resolved Hide resolved
@GuyAv46 GuyAv46 force-pushed the guyav-big_sort_bench_noci branch 2 times, most recently from a1f4c31 to c3752e7 Compare March 1, 2023 12:50
@GuyAv46 GuyAv46 changed the base branch from guyav-big_sort_bench_noci to master March 1, 2023 13:15
src/util/minmax_heap.c Show resolved Hide resolved
Copy link
Collaborator

@ephraimfeldblum ephraimfeldblum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@GuyAv46 GuyAv46 merged commit a349596 into master Mar 2, 2023
@GuyAv46 GuyAv46 deleted the guyav-minmax_heap_perf branch March 2, 2023 13:41
GuyAv46 added a commit that referenced this pull request Mar 2, 2023
* attempt to improve min-max heap

* some more optimization

* minor changes

* review fixes

* extended comment

* added a benchmark for big heap sorting

* changed BM

* improved comment

* added no content to benchmark
GuyAv46 added a commit that referenced this pull request Mar 2, 2023
* attempt to improve min-max heap

* some more optimization

* minor changes

* review fixes

* extended comment

* added a benchmark for big heap sorting

* changed BM

* improved comment

* added no content to benchmark
GuyAv46 added a commit that referenced this pull request Mar 2, 2023
* attempt to improve min-max heap

* some more optimization

* minor changes

* review fixes

* extended comment

* added a benchmark for big heap sorting

* changed BM

* improved comment

* added no content to benchmark
GuyAv46 added a commit that referenced this pull request Mar 2, 2023
* attempt to improve min-max heap

* some more optimization

* minor changes

* review fixes

* extended comment

* added a benchmark for big heap sorting

* changed BM

* improved comment

* added no content to benchmark
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants