-
Notifications
You must be signed in to change notification settings - Fork 1
Cache LDE + Scaling improvements #494
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
Changes from all commits
0126881
ada4503
398e18f
d8e432f
24b44e4
4ce7a24
ec49bf6
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 |
|---|---|---|
|
|
@@ -341,19 +341,35 @@ pub trait IsStarkProver< | |
| let num_cols = columns.len(); | ||
|
|
||
| #[cfg(feature = "parallel")] | ||
| let iter = (0..num_rows).into_par_iter(); | ||
| #[cfg(not(feature = "parallel"))] | ||
| let iter = 0..num_rows; | ||
| let hashed_leaves: Vec<Commitment> = { | ||
| (0..num_rows) | ||
| .into_par_iter() | ||
| .map_init( | ||
| || vec![FieldElement::<E>::zero(); num_cols], | ||
| |row_buf, row_idx| { | ||
| let br_idx = reverse_index(row_idx, num_rows as u64); | ||
| for col_idx in 0..num_cols { | ||
| row_buf[col_idx] = columns[col_idx][br_idx].clone(); | ||
| } | ||
| BatchedMerkleTreeBackend::<E>::hash_data(row_buf) | ||
| }, | ||
| ) | ||
| .collect() | ||
| }; | ||
|
|
||
| let hashed_leaves: Vec<Commitment> = iter | ||
| .map(|row_idx| { | ||
| let br_idx = reverse_index(row_idx, num_rows as u64); | ||
| let row: Vec<FieldElement<E>> = (0..num_cols) | ||
| .map(|col_idx| columns[col_idx][br_idx].clone()) | ||
| .collect(); | ||
| BatchedMerkleTreeBackend::<E>::hash_data(&row) | ||
| }) | ||
| .collect(); | ||
| #[cfg(not(feature = "parallel"))] | ||
| let hashed_leaves: Vec<Commitment> = { | ||
| let mut row_buf = vec![FieldElement::<E>::zero(); num_cols]; | ||
| (0..num_rows) | ||
| .map(|row_idx| { | ||
| let br_idx = reverse_index(row_idx, num_rows as u64); | ||
| for col_idx in 0..num_cols { | ||
| row_buf[col_idx] = columns[col_idx][br_idx].clone(); | ||
| } | ||
| BatchedMerkleTreeBackend::<E>::hash_data(&row_buf) | ||
| }) | ||
| .collect() | ||
| }; | ||
|
|
||
| let tree = BatchedMerkleTree::<E>::build_from_hashed_leaves(hashed_leaves)?; | ||
| let root = tree.root; | ||
|
|
@@ -641,7 +657,7 @@ pub trait IsStarkProver< | |
| air_trace_pairs: &[AirTracePair<'_, Field, FieldExtension, PI>], | ||
| metadatas: &[Round1Metadata<Field, FieldExtension>], | ||
| domains: &[Domain<Field>], | ||
| twiddle_caches: &[LdeTwiddles<Field>], | ||
| twiddle_caches: &[Arc<LdeTwiddles<Field>>], | ||
| main_pool: &mut [Vec<FieldElement<Field>>], | ||
| aux_pool: &mut [Vec<FieldElement<FieldExtension>>], | ||
| ) where | ||
|
|
@@ -657,7 +673,13 @@ pub trait IsStarkProver< | |
| .zip(domains.iter().zip(twiddle_caches.iter())) | ||
| { | ||
| let result = Self::reconstruct_round1( | ||
| *air, *trace, domain, metadata, twiddles, main_pool, aux_pool, | ||
| *air, | ||
| *trace, | ||
| domain, | ||
| metadata, | ||
| &**twiddles, | ||
| main_pool, | ||
| aux_pool, | ||
| ) | ||
| .expect("reconstruct_round1 failed in debug-checks"); | ||
| temp_results.push(result); | ||
|
|
@@ -1518,20 +1540,29 @@ pub trait IsStarkProver< | |
| let phase_start = Instant::now(); | ||
|
|
||
| let mut domains = Vec::with_capacity(num_airs); | ||
| let mut twiddle_caches: Vec<LdeTwiddles<Field>> = Vec::with_capacity(num_airs); | ||
| let mut twiddle_caches: Vec<Arc<LdeTwiddles<Field>>> = Vec::with_capacity(num_airs); | ||
| let mut max_main_cols = 0usize; | ||
| let mut max_aux_cols = 0usize; | ||
|
|
||
| // Share twiddles across tables with equal lde_size. Relies on all AIRs | ||
| // using the same ProofOptions (blowup_factor, coset_offset). | ||
| let mut twiddle_by_size: std::collections::HashMap<usize, Arc<LdeTwiddles<Field>>> = | ||
| std::collections::HashMap::new(); | ||
|
|
||
| for (air, trace, _pub_inputs) in &*air_trace_pairs { | ||
| let trace_length = trace.num_rows(); | ||
| let domain = new_domain(*air, trace_length); | ||
| let twiddles = LdeTwiddles::new(&domain); | ||
|
|
||
| let lde_size = domain.interpolation_domain_size * domain.blowup_factor; | ||
| let twiddles = twiddle_by_size | ||
| .entry(lde_size) | ||
| .or_insert_with(|| Arc::new(LdeTwiddles::new(&domain))); | ||
|
Comment on lines
+1556
to
+1559
Contributor
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. Medium – Incorrect cache key if AIRs have different
Using only Consider asserting the invariant at the cache hit, e.g.: // or store (interpolation_domain_size, coset_offset) as key
debug_assert_eq!(
domain.interpolation_domain_size,
first_domain_with_this_lde_size.interpolation_domain_size
);Or use |
||
|
|
||
| max_main_cols = max_main_cols.max(trace.num_main_columns); | ||
| max_aux_cols = max_aux_cols.max(air.num_auxiliary_rap_columns()); | ||
|
|
||
| domains.push(domain); | ||
| twiddle_caches.push(twiddles); | ||
| twiddle_caches.push(Arc::clone(twiddles)); | ||
| } | ||
|
|
||
| // Allocate K independent LDE column buffer pool sets for parallel table processing. | ||
|
|
@@ -1573,7 +1604,7 @@ pub trait IsStarkProver< | |
| let idx = chunk_start + j; | ||
| let (air, trace, _) = &air_trace_pairs[idx]; | ||
| let domain = &domains[idx]; | ||
| let twiddles = &twiddle_caches[idx]; | ||
| let twiddles = &*twiddle_caches[idx]; | ||
|
|
||
| let result = if air.is_preprocessed() { | ||
| Self::commit_preprocessed_trace( | ||
|
|
@@ -1705,7 +1736,7 @@ pub trait IsStarkProver< | |
| let idx = chunk_start + j; | ||
| let (air, trace, _) = &air_trace_pairs[idx]; | ||
| let domain = &domains[idx]; | ||
| let twiddles = &twiddle_caches[idx]; | ||
| let twiddles = &*twiddle_caches[idx]; | ||
|
|
||
| if air.has_aux_trace() { | ||
| let num_aux_cols = trace.num_aux_columns; | ||
|
|
||
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.
Low – Parallel and sequential branches diverge on contract violation
If
inv_twiddles.len() < evals.len() / 2(broken contract):zipsilently truncates to the shorter iterator →foldedis shorter thanevals.len() / 2, and*evals = foldedquietly installs a wrong-length vector. Downstream code proceeds with corrupted state.evals[j]withjreachinghalf - 1will index out ofinv_twiddles→ panic.In practice the contract is maintained by callers, but if it breaks the parallel path gives no diagnostic while the sequential path at least terminates loudly. A
debug_assert_eq!(inv_twiddles.len(), evals.len() / 2)at the top of the function would catch this in both builds without runtime cost in release.