perf: use saturating arithmetic in balance updates for SIMD vectorization#282
perf: use saturating arithmetic in balance updates for SIMD vectorization#282lodekeeper-z wants to merge 2 commits into
Conversation
…tion Replace `try std.math.add(u64, ...)` with saturating add (`+|`) in processRewardsAndPenalties. Overflow is impossible: max ETH supply ~120M ETH = ~1.2e17 Gwei, u64 max = ~1.8e19, giving 154x headroom. Saturating ops are branchless, enabling LLVM to auto-vectorize the balance loop. Isolated microbenchmark (500k validators, 100 iters, ReleaseFast): try std.math.add: 618.6 µs/iter saturating +|: 179.6 µs/iter speedup: 3.44x Full epoch benchmark (2.18M validators, mainnet state, 50 iters): rewards_and_penalties (checked): 119.4ms avg rewards_and_penalties (saturating): 117.5ms avg Full epoch (checked): 794.1ms avg Full epoch (saturating): 785.5ms avg 🤖 Generated with AI assistance
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request optimizes the validator balance update logic by switching from checked addition to saturating arithmetic. By removing unnecessary error-path branching, the compiler can now effectively auto-vectorize the loop, leading to significant performance gains in the inner loop processing while maintaining safety due to the mathematical impossibility of overflow in this context. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request optimizes the processRewardsAndPenalties function by replacing checked addition with saturating arithmetic, which enables SIMD auto-vectorization for improved performance. The reviewer provided feedback to align the code with the repository's style guide, specifically suggesting that explanatory comments be rephrased as prose and that debug assertions be added to verify the no-overflow invariant for safety and documentation purposes.
| // Use saturating arithmetic instead of checked (`std.math.add`): | ||
| // - Overflow is impossible: max ETH supply ~120M ETH = ~1.2e17 Gwei, | ||
| // u64 max = ~1.8e19, giving 154x headroom even with all rewards summed. | ||
| // - Saturating ops are branchless, enabling SIMD auto-vectorization of the | ||
| // balance loop (~3.4x speedup on the inner loop for 500k+ validators). |
There was a problem hiding this comment.
This comment is very helpful for explaining the rationale. To better align with the style guide's preference for prose in comments, consider rephrasing it as a paragraph.
From the style guide:
Comments are sentences, with a space after the slash, with a capital letter and a full stop, or a colon if they relate to something that follows. Comments are well-written prose describing the code, not just scribblings in the margin. (lines 304-306)
// Use saturating arithmetic for performance. Saturating operations are branchless, which
// enables SIMD auto-vectorization of the balance update loop. This is safe because a
// balance overflow is mathematically impossible: the maximum total ETH supply (~1.2e17 Gwei)
// is more than 150x smaller than the max u64 value (~1.8e19 Gwei).
References
- Comments should be well-written prose in the form of sentences, rather than lists of points. (link)
There was a problem hiding this comment.
Fixed in 29d60ec. Added std.debug.assert for the invariant and rewrote the comment as prose.
| for (rewards, penalties, balances, 0..) |reward, penalty, *balance, i| { | ||
| const slashing: u64 = if (i < slashings.len) slashings[i] else 0; | ||
| balance.* = (try std.math.add(u64, balance.*, reward)) -| penalty -| slashing; | ||
| balance.* = ((balance.* +| reward) -| penalty) -| slashing; |
There was a problem hiding this comment.
To uphold the style guide's emphasis on safety (Safety, performance, and developer experience. In that order., line 5), it's best to add a std.debug.assert here. This will verify the no-overflow assumption during testing without impacting performance in ReleaseFast builds. This also serves as executable documentation for the invariant you've identified.
From the style guide:
Assert all function arguments and return values, pre/postconditions and invariants. (line 51)
I've also removed a redundant set of parentheses for cleaner code.
std.debug.assert(balance.* <= std.math.maxInt(u64) - reward);
balance.* = (balance.* +| reward) -| penalty -| slashing;
References
- The style guide prioritizes safety over performance and mandates asserting all pre/postconditions and invariants. Adding a debug assertion maintains safety during testing without compromising release performance. (link)
| } else { | ||
| for (rewards, penalties, balances) |reward, penalty, *balance| { | ||
| balance.* = (try std.math.add(u64, balance.*, reward)) -| penalty; | ||
| balance.* = (balance.* +| reward) -| penalty; |
There was a problem hiding this comment.
Similarly, please add a std.debug.assert here to ensure safety during testing, in line with the style guide's emphasis on assertions.
std.debug.assert(balance.* <= std.math.maxInt(u64) - reward);
balance.* = (balance.* +| reward) -| penalty;
References
- The style guide prioritizes safety over performance and mandates asserting all pre/postconditions and invariants. Adding a debug assertion maintains safety during testing without compromising release performance. (link)
Add std.debug.assert for the no-overflow invariant (catches bugs in debug builds, compiles away in release). Rewrite comment as prose per TigerStyle. 🤖 Generated with AI assistance
lodekeeper-z
force-pushed
the
perf/saturating-balance-pr
branch
from
94d40af to
29d60ec
Compare
1 participant
Motivation
processRewardsAndPenaltiesiterates over all validator balances (2.18M on mainnet). The inner loop usedtry std.math.add(u64, ...)which generates a branch per addition, preventing LLVM from auto-vectorizing.Change
Replace
try std.math.addwith saturating add (+|). Overflow is mathematically impossible:Results
Isolated microbenchmark (500k validators, 100 iterations, ReleaseFast):
Full epoch benchmark (
bench_process_epoch, mainnet state at slot 13336576, 2.18M validators, 50 iterations, ReleaseFast):try std.math.add)+|)rewards_and_penalties(isolated)The inner loop speedup (3.44×) is real but
processRewardsAndPenaltiesis dominated by reward/penalty computation, so the end-to-end improvement is modest. The change is still worthwhile: it removes unnecessary error-path overhead with zero risk of behavior change.Verification
zig build— cleanzig build test:state_transition— passes🤖 Generated with AI assistance