-
Notifications
You must be signed in to change notification settings - Fork 454
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
[dbnode] Use bits.LeadingZeros64 to improve encoder performance #2857
Conversation
Nice. This seems like a good win, have looked at bits.TrailingZeros64 for the new roaring bitmap query side rewrite (from PR #2791):
How representative do you think the benchmark is? Can we expect most encoder performance gains to be able to be measured by this micro benchmark? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Codecov Report
@@ Coverage Diff @@
## master #2857 +/- ##
======================================
Coverage 72.1% 72.1%
======================================
Files 1099 1099
Lines 99811 99811
======================================
Hits 71982 71982
Misses 22892 22892
Partials 4937 4937
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
I've picked somewhat randomly 10 series from some real-world data file that I had (this is the same data that I used in #2827). |
* master: (28 commits) [dbnode] Add claims for index segments volume index (#2846) [dbnode] Remove namespaces from example config and integration tests (#2866) [dbnode] Resurrect flaky test skip (#2868) [aggregator] Fix checkCampaignStateLoop (#2867) [dbnode] implement deletion method in namespace kvadmin service (#2861) Replace closer with resource package (#2864) Add coding style guide (#2831) Add GOVERNANCE.md to describe governance (#2830) Add COMPATIBILITY.md to describe version compatibility (#2829) Refactor etcd config as discovery section with convenience types (#2843) Refactor x/lockfile into dbnode/server (#2862) [lint] Disable nlreturn linter (#2865) [m3cluster] Expose placement algorithm in placement service (#2858) [etcd] Set reasonable cluster connection/sync settings by default (#2860) [dbnode] Use bits.LeadingZeros64 to improve encoder performance (#2857) Cleanup m3nsch leftovers (#2856) Update ci-scripts to correct coverage tracking (#2854) [aggregator] Process campaign state without waiting for first campaign check interval (#2855) Bump go to 1.14 (#2853) [query] Remove single series error from M3 ...
What this PR does / why we need it:
Improves M3TSZ encoder performance by using
bits.LeadingZeros64
instead of custom loop code (up to 64 iterations) to count significant bits. The result is ~12% performance improvement (and a simpler code).Before the change:
BenchmarkM3TSZEncode-12 8128 139077 ns/op
After the change:
BenchmarkM3TSZEncode-12 9357 123138 ns/op
Special notes for your reviewer:
Does this PR introduce a user-facing and/or backwards incompatible change?:
NONE
Does this PR require updating code package or user-facing documentation?:
NONE