Skip to content

perf(mod-builder): store vars during preflight to avoid recomputation in trace fill#2371

Closed
jonathanpwang wants to merge 1 commit intodevelop-v2from
elated-ptolemy
Closed

perf(mod-builder): store vars during preflight to avoid recomputation in trace fill#2371
jonathanpwang wants to merge 1 commit intodevelop-v2from
elated-ptolemy

Conversation

@jonathanpwang
Copy link
Contributor

Previously, run_field_expression was called twice - once during preflight and again during trace filling - causing expensive vars computation to be duplicated.

This change:

  • Extends FieldExpressionCoreRecordMut to store computed vars
  • Updates preflight to save vars in the record after computation
  • Updates trace filler to read precomputed vars from the record
  • Adds generate_subrow_with_precomputed_vars to FieldExpr
  • Extracts derive_flags_from_opcode helper for reuse

The vars (intermediate field expression results) are now computed once during preflight and reused during trace generation, eliminating redundant BigUint arithmetic operations.

Previously, `run_field_expression` was called twice - once during
preflight and again during trace filling - causing expensive `vars`
computation to be duplicated.

This change:
- Extends FieldExpressionCoreRecordMut to store computed vars
- Updates preflight to save vars in the record after computation
- Updates trace filler to read precomputed vars from the record
- Adds generate_subrow_with_precomputed_vars to FieldExpr
- Extracts derive_flags_from_opcode helper for reuse

The vars (intermediate field expression results) are now computed once
during preflight and reused during trace generation, eliminating
redundant BigUint arithmetic operations.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings January 25, 2026 18:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements a performance optimization for the mod-builder circuit by storing intermediate field expression variables (vars) during preflight execution and reusing them during trace generation, eliminating redundant BigUint arithmetic operations.

Changes:

  • Extends FieldExpressionCoreRecordMut to include var_limbs field for storing precomputed intermediate variables
  • Updates preflight execution to compute and store vars in the record
  • Modifies trace filler to read precomputed vars instead of recomputing them
  • Adds generate_subrow_with_precomputed_vars method to FieldExpr that accepts precomputed vars
  • Extracts derive_flags_from_opcode helper function for code reuse

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
crates/circuits/mod-builder/src/core_chip.rs Core changes implementing vars storage in preflight and retrieval during trace filling; adds derive_flags_from_opcode helper
crates/circuits/mod-builder/src/builder.rs Adds generate_subrow_with_precomputed_vars method that skips var computation and uses precomputed values
crates/circuits/mod-builder/src/tests.rs Updates test infrastructure with increased buffer sizes, num_variables parameter, and switches to BabyBearPoseidon2Engine

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

};
use openvm_stark_sdk::{
any_rap_arc_vec, config::baby_bear_blake3::BabyBearBlake3Engine, engine::StarkFriEngine,
any_rap_arc_vec, config::baby_bear_poseidon2::BabyBearPoseidon2Engine, engine::StarkFriEngine,
Copy link

Copilot AI Jan 25, 2026

Choose a reason for hiding this comment

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

The change from BabyBearBlake3Engine to BabyBearPoseidon2Engine appears to be unrelated to the performance optimization described in the PR. Consider submitting this as a separate change or documenting it in the PR description if it's intentional.

Copilot uses AI. Check for mistakes.
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.

1 participant