-
Notifications
You must be signed in to change notification settings - Fork 21
perf: optimize operator tracking with bitmap implementation #1073
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
Conversation
dfc934c to
e995290
Compare
|
Commit: 412b866 SP1 Execution Results
|
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #1073 +/- ##
==========================================
+ Coverage 74.31% 74.43% +0.12%
==========================================
Files 492 501 +9
Lines 40960 42336 +1376
==========================================
+ Hits 30439 31514 +1075
- Misses 10521 10822 +301
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 33 files with indirect coverage changes 🚀 New features to boost your workflow:
|
98f7ea2 to
24bbe62
Compare
b455eee to
1400d32
Compare
8441be6 to
e90fb0a
Compare
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.
ACK e90fb0a
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.
There is something to be said about whether this kind of optimization is even needed given that there are not going to be that many operators. Barring that, I've left a few nits (both on the APIs and the docs) but there are no major blockers.
e90fb0a to
a26e6a1
Compare
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.
ACK a26e6a1
a26e6a1 to
8e47dc5
Compare
The optimization is not that significant. I worked on it because it was brought up in some old PR. |
fd30b90 to
7f2b91c
Compare
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.
Did another pass after the recent changes and left a few more comments. I think it'll be good to go for me once those have been addressed.
e7de9a5 to
cbc22e0
Compare
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.
ACK cbc22e0
|
@evgenyzdanovich or @irnb can someone from @alpenlabs/chain-protocol review this as well please? |
Description
Implements a memory-efficient
OperatorBitmapstructure to replaceVec<OperatorIdx>across the bridge subprotocol to reduce memory usage for operator set tracking while improving performance of set operations.I used Claude Code for generating tests and updating docstrings.
Type of Change
Notes to Reviewers
Checklist
in the body of this PR.
Related Issues