Skip to content
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

fix: Incorrect lazy initialization of memory data structure #1113

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

zlangley
Copy link
Contributor

No description provided.

@@ -310,15 +310,7 @@ impl<F: PrimeField32> Memory<F> {
.block_data
.get(&(address_space, pointer))
.copied()
.unwrap_or_else(|| {
for i in 0..size {
Copy link
Contributor Author

@zlangley zlangley Dec 17, 2024

Choose a reason for hiding this comment

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

This was the main bug. If size is large (e.g., 32), then there might not be a block at address, but it's wrong to overwrite blocks at, say, address + 24. Changing this to self.initial_block_size still does not seem correct. It's obviously correct to just write blocks as needed, rather than trying to be smart about it, which is what this PR aims to do.

Copy link

Benchmarks

group app_log_blowup app_total_cells_used app_total_cycles app_total_proof_time_ms leaf_log_blowup leaf_total_cells_used leaf_total_cycles leaf_total_proof_time_ms max_segment_length instance alloc
ecrecover_program
2
10,251,804
195,066
(+104.0 [+5.4%])
2,037.0
-
-
-
-
1048476 64cpu-linux-arm64 mimalloc
fibonacci_program
2
51,615,800
3,000,274
(-7.0 [-0.1%])
5,552.0
-
-
-
-
1048476 64cpu-linux-arm64 mimalloc
regex_program
2
238,890,449
8,381,808
(+64.0 [+0.4%])
17,419.0
-
-
-
-
1048476 64cpu-linux-arm64 mimalloc
verify_fibair
2
(-1,140 [-0.0%])
48,127,147
(-130 [-0.0%])
397,164
(+26.0 [+0.8%])
3,174.0
-
-
-
-
1048476 64cpu-linux-arm64 mimalloc

Commit: fcb151b

Benchmark Workflow

@luffykai luffykai merged commit 3b505ac into main Dec 17, 2024
16 checks passed
@luffykai luffykai deleted the memory-fix branch December 17, 2024 23:50
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.

2 participants