-
Notifications
You must be signed in to change notification settings - Fork 12
perf(epoch): replace AutoHashMap with array lookup in reward/penalty caches #286
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
base: main
Are you sure you want to change the base?
Changes from all commits
ed2ff78
afa70bd
d7525b6
8f728c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,4 @@ | ||
| const std = @import("std"); | ||
| const Allocator = std.mem.Allocator; | ||
| const ForkSeq = @import("config").ForkSeq; | ||
| const BeaconConfig = @import("config").BeaconConfig; | ||
| const BeaconState = @import("fork_types").BeaconState; | ||
|
|
@@ -38,7 +37,6 @@ const RewardPenaltyItem = struct { | |
| /// consumer should deinit `rewards` and `penalties` arrays | ||
| pub fn getRewardsAndPenaltiesAltair( | ||
| comptime fork: ForkSeq, | ||
| allocator: Allocator, | ||
| config: *const BeaconConfig, | ||
| epoch_cache: *const EpochCache, | ||
| state: *BeaconState(fork), | ||
|
|
@@ -55,10 +53,11 @@ pub fn getRewardsAndPenaltiesAltair( | |
| @memset(penalties, 0); | ||
|
|
||
| const is_in_inactivity_leak = isInInactivityLeak(epoch_cache.epoch, try state.finalizedEpoch()); | ||
| // effectiveBalance is multiple of EFFECTIVE_BALANCE_INCREMENT and less than MAX_EFFECTIVE_BALANCE | ||
| // so there are limited values of them like 32, 31, 30 | ||
| var reward_penalty_item_cache = std.AutoHashMap(u64, RewardPenaltyItem).init(allocator); | ||
| defer reward_penalty_item_cache.deinit(); | ||
| // effectiveBalance is a multiple of EFFECTIVE_BALANCE_INCREMENT and bounded by max effective balance | ||
| // (32 ETH pre-Electra, 2048 ETH post-Electra). Use a fixed array for O(1) lookup. | ||
| const max_effective = comptime if (fork.gte(.electra)) preset.MAX_EFFECTIVE_BALANCE_ELECTRA else preset.MAX_EFFECTIVE_BALANCE; | ||
| const max_increment = comptime max_effective / EFFECTIVE_BALANCE_INCREMENT + 1; | ||
| var reward_penalty_item_cache: [max_increment]?RewardPenaltyItem = .{null} ** max_increment; | ||
|
|
||
| const inactivity_penality_multiplier: u64 = | ||
| if (fork == ForkSeq.altair) INACTIVITY_PENALTY_QUOTIENT_ALTAIR else INACTIVITY_PENALTY_QUOTIENT_BELLATRIX; | ||
|
|
@@ -74,7 +73,7 @@ pub fn getRewardsAndPenaltiesAltair( | |
|
|
||
| const effective_balance_increment = effective_balance_increments[i]; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's a little bit unsafe because
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Valid concern. The u16 type allows values up to 65535 but the array is at most 2049. With the fork-aware sizing now in place, the arrays are even smaller (33 for phase0/pre-Electra). Changing
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. go for it @lodekeeper-z |
||
|
|
||
| const reward_penalty_item = if (reward_penalty_item_cache.get(effective_balance_increment)) |rpi| rpi else blk: { | ||
| const reward_penalty_item = if (reward_penalty_item_cache[effective_balance_increment]) |rpi| rpi else blk: { | ||
| const base_reward = effective_balance_increment * cache.base_reward_per_increment; | ||
| const ts_weigh = PARTICIPATION_FLAG_WEIGHTS[TIMELY_SOURCE_FLAG_INDEX]; | ||
| const tt_weigh = PARTICIPATION_FLAG_WEIGHTS[TIMELY_TARGET_FLAG_INDEX]; | ||
|
|
@@ -93,7 +92,7 @@ pub fn getRewardsAndPenaltiesAltair( | |
| .timely_source_penalty = @divFloor(base_reward * ts_weigh, WEIGHT_DENOMINATOR), | ||
| .timely_target_penalty = @divFloor(base_reward * tt_weigh, WEIGHT_DENOMINATOR), | ||
| }; | ||
| try reward_penalty_item_cache.put(effective_balance_increment, rpi); | ||
| reward_penalty_item_cache[effective_balance_increment] = rpi; | ||
| break :blk rpi; | ||
| }; | ||
|
|
||
|
|
||
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.
for pre-electra, use
MAX_EFFECTIVE_BALANCEto save memoryThere 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.
Fixed in 8f728c3. Now uses comptime fork selection:
MAX_EFFECTIVE_BALANCE(33 slots) for pre-Electra forks,MAX_EFFECTIVE_BALANCE_ELECTRA(2049 slots) for Electra+. Same for processSlashings.