Skip to content

Conversation

@shuklaayush
Copy link
Collaborator

@shuklaayush shuklaayush commented Dec 24, 2025

This pr primarily adds a page_indices_since_checkpoint buffer to track page accesses between checkpoints. This buffer is used to initialize the memory trace heights of the next segment

Resolves INT-5778

@github-actions

This comment has been minimized.

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 24, 2025

CodSpeed Performance Report

Merging #2332 will degrade performance by 79.78%

Comparing fix/e2-checkpoint-unique-memory-access (f1c9fba) with main (d7eab70)

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

Summary

❌ 8 regressions
✅ 16 untouched
⏩ 36 skipped1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Mode Benchmark BASE HEAD Efficiency
WallTime benchmark_execute_metered[quicksort] 9.1 ms 44.8 ms -79.78%
WallTime benchmark_execute_metered[keccak256] 11.1 ms 46.8 ms -76.25%
WallTime benchmark_execute_metered[fibonacci_iterative] 14.3 ms 49.1 ms -70.92%
WallTime benchmark_execute_metered[bubblesort] 11.2 ms 46.7 ms -75.9%
WallTime benchmark_execute_metered[revm_transfer] 23.9 ms 59.1 ms -59.47%
WallTime benchmark_execute_metered[sha256] 9.3 ms 45 ms -79.4%
WallTime benchmark_execute_metered[revm_snailtracer] 10 ms 42.2 ms -76.37%
WallTime benchmark_execute_metered[fibonacci_recursive] 16.8 ms 52.7 ms -68.12%

Footnotes

  1. 36 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@shuklaayush shuklaayush force-pushed the fix/e2-checkpoint-unique-memory-access branch from 8b1f528 to b2c1320 Compare December 25, 2025 02:21
@github-actions

This comment has been minimized.

@shuklaayush shuklaayush force-pushed the fix/e2-checkpoint-unique-memory-access branch from b2c1320 to 41c0b39 Compare December 25, 2025 02:34
@github-actions

This comment has been minimized.

@shuklaayush shuklaayush force-pushed the fix/e2-checkpoint-unique-memory-access branch from 41c0b39 to 379ce3a Compare December 25, 2025 02:55
@github-actions

This comment has been minimized.

@shuklaayush shuklaayush force-pushed the fix/e2-checkpoint-unique-memory-access branch 2 times, most recently from 5257223 to ab5831e Compare December 30, 2025 20:52
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@shuklaayush shuklaayush force-pushed the fix/e2-checkpoint-unique-memory-access branch from e26cce2 to 76e5eb7 Compare December 30, 2025 21:20
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@shuklaayush shuklaayush force-pushed the fix/e2-checkpoint-unique-memory-access branch 3 times, most recently from e999c6e to 49e579a Compare December 31, 2025 15:55
@github-actions

This comment has been minimized.

@shuklaayush shuklaayush force-pushed the fix/e2-checkpoint-unique-memory-access branch from 49e579a to 766d567 Compare December 31, 2025 16:12
@github-actions

This comment has been minimized.

@shuklaayush shuklaayush force-pushed the fix/e2-checkpoint-unique-memory-access branch from 766d567 to e6239b2 Compare December 31, 2025 16:48
@shuklaayush shuklaayush force-pushed the fix/e2-checkpoint-unique-memory-access branch from 516e113 to 051accd Compare December 31, 2025 17:09
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@shuklaayush shuklaayush force-pushed the fix/e2-checkpoint-unique-memory-access branch 2 times, most recently from 323507d to 63d85b9 Compare December 31, 2025 18:59
@github-actions

This comment has been minimized.

@shuklaayush shuklaayush changed the title fix: keep track of unique memory accesses since last checkpoint fix: keep track of memory accesses since last checkpoint Dec 31, 2025
Comment on lines +261 to +276
// Reset trace heights for memory chips as 0
// SAFETY: boundary_idx is a compile time constant within bounds
unsafe {
*trace_heights.get_unchecked_mut(self.boundary_idx) = 0;
}
if let Some(merkle_tree_idx) = self.merkle_tree_index {
// SAFETY: merkle_tree_idx is guaranteed to be in bounds
unsafe {
*trace_heights.get_unchecked_mut(merkle_tree_idx) = 0;
}
let poseidon2_idx = trace_heights.len() - 2;
// SAFETY: poseidon2_idx is trace_heights.len() - 2, guaranteed to be in bounds
unsafe {
*trace_heights.get_unchecked_mut(poseidon2_idx) = 0;
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't reset access adapter heights here because access adapters are shared between memory and normal traces and it won't be correct to reset them to 0. what we want ideally is to remove only the contribution of memory chips to access adapter heights

@shuklaayush shuklaayush force-pushed the fix/e2-checkpoint-unique-memory-access branch from 63d85b9 to d55c729 Compare January 2, 2026 09:40
@github-actions

This comment has been minimized.

@shuklaayush shuklaayush marked this pull request as ready for review January 2, 2026 10:09
@github-actions
Copy link

github-actions bot commented Jan 5, 2026

group app.proof_time_ms app.cycles app.cells_used leaf.proof_time_ms leaf.cycles leaf.cells_used
verify_fibair (+18 [+8.1%]) 241 322,610 2,058,654 - - -
fibonacci (-9 [-0.9%]) 1,019 1,500,209 2,100,402 - - -
regex (+43 [+1.9%]) 2,332 4,137,502 17,695,216 - - -
ecrecover (-4 [-0.5%]) 737 122,859 (-2328 [-0.1%]) 2,262,772 - - -
pairing (-28 [-1.9%]) 1,454 1,745,742 25,408,302 - - -

Commit: f1c9fba

Benchmark Workflow

Copy link
Contributor

@jonathanpwang jonathanpwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I added a non-debug assert to ensure the buffer len is not exceeded, since it would lead to UB behavior if it was exceeded at runtime. I think branch prediction will mean it doesn't incur a large perf diff.

@jonathanpwang jonathanpwang merged commit 51dd038 into main Jan 5, 2026
70 of 72 checks passed
@jonathanpwang jonathanpwang deleted the fix/e2-checkpoint-unique-memory-access branch January 5, 2026 02:30
@shuklaayush
Copy link
Collaborator Author

shuklaayush commented Jan 5, 2026

I added a non-debug assert to ensure the buffer len is not exceeded, since it would lead to UB behavior if it was exceeded at runtime. I think branch prediction will mean it doesn't incur a large perf diff.

We should also add it to the aot version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants