-
Notifications
You must be signed in to change notification settings - Fork 4k
perf(staking): optimize endblock by reducing bech32 conversions #24354
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
This change significantly improves the performance of staking endblock operations by: 1. Eliminating unnecessary Bech32 address conversions in getLastValidatorsByAddr and sortNoLongerBonded 2. Using string(byte[]) directly instead of codec.BytesToString 3. Updating error messages with more helpful context 4. Using AddRaw for better performance on integer addition These optimizations are especially important for chains with many validators, reducing the time spent in EndBlock which is a critical part of block processing. Backport of commit cccdbd9
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughThe changes update the Changes
Sequence Diagram(s)sequenceDiagram
participant C as Caller
participant A as ApplyAndReturnValidatorSetUpdates
participant G as GetValidator
participant S as StateCheck
participant U as UpdatePower
C->>A: Call ApplyAndReturnValidatorSetUpdates
A->>G: Retrieve validator info
G-->>A: Return validator object or error
alt Error occurs during retrieval
A->>C: Return wrapped error
else Validator found
A->>S: Check validator state (e.g., jailed, unexpected status)
alt Invalid state
A->>C: Return error with context
else Valid state
A->>U: Compare and update validator power if changed
U-->>A: Updated validator power
A->>C: Return validator set updates
end
end
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (16)
🔇 Additional comments (14)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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 but let's keep the logic changes here minimal to what is needed for a performance improvement PR
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.
Great! Really glad this is getting in
…os#24354) Co-authored-by: Dev Ojha <[email protected]> Co-authored-by: Alex | Interchain Labs <[email protected]>
* port https://github.com/cosmos/cosmos-sdk/pull/24354/files * changelog * gofmt
* port https://github.com/cosmos/cosmos-sdk/pull/24354/files * changelog * gofmt
This is a rebased version of #23869 with some minor tweak and benchmark. All credits and bit thanks go to @ValarDragon for the original patch and idea!
Performance gains in the test setup to original code
Map key patch
((654391 - 435196) / 654391) * 100 ~= 33 %
Map value patch
((654391 - 421474) / 654391) * 100 ~= 35 %
Average
Original
(648741 + 649484 + 667771 + 651568) / 4 = 654391 ns/op
Map key patch
(433791 + 432525 + 432654 + 441817) / 4 = 435196 ns/op
Map value patch
(421966 + 422146 + 419467 + 422317) / 4 = 421474 ns/op
Benchmarks on my box
BenchmarkMustMarshal
BenchmarkMustMarshal-12 60243637 19.92 ns/op
BenchmarkMustUnmarshal
BenchmarkMustUnmarshal-12 100000000 11.69 ns/op