Skip to content

blockifier: avoid redundant memory reads in read_felt_array - #15018

Open
gkaempfer wants to merge 2 commits into
main-v0.14.3from
claude/perf/blockifier-avoid-double-memory-read-73142
Open

blockifier: avoid redundant memory reads in read_felt_array#15018
gkaempfer wants to merge 2 commits into
main-v0.14.3from
claude/perf/blockifier-avoid-double-memory-read-73142

Conversation

@gkaempfer

Copy link
Copy Markdown
Contributor

Summary

Follow-up perf optimization on #15008 ("blockifier: handle null empty spans"), which added an empty-span check to read_felt_array in crates/blockifier/src/execution/syscalls/hint_processor.rs.

That check reads the array's start and end pointer cells via vm.get_maybe(...) to detect the null-empty-span case. In the non-empty (common) path, the function then re-read the same two memory cells via vm.get_relocatable(...) — a redundant Memory::get() lookup for addresses already fetched a few lines earlier.

read_felt_array runs on every array-typed syscall argument (calldata, emit_event keys/data, etc.), i.e. roughly once per syscall across a block, so the extra lookup is pure overhead in a hot path.

This PR reuses the already-fetched Option<MaybeRelocatable> values instead of re-reading memory, via a small helper (relocatable_from_memory_value) that reproduces VirtualMachine::get_relocatable's exact error semantics (MemoryError::UnknownMemoryCell vs MemoryError::ExpectedRelocatable) so behavior is unchanged in every case — empty span (null or real), non-empty span, missing memory cell, wrong-type memory cell.

Also strengthens test coverage: the existing "rejects mixed null/pointer span" test now asserts the exact error variant and address (previously just is_err()), and two new tests cover the felt-typed-end-pointer and unwritten-cell error paths, since preserving get_relocatable's error semantics is the main correctness risk of this kind of refactor.

Expected impact

Cuts the memory-lookup work of read_felt_array roughly in half for the non-empty-array case (2 lookups instead of 4), on a function invoked for essentially every array-typed syscall argument in every transaction.

Test plan

  • SEED=0 cargo test -p blockifier read_felt_array — 8 passed, 0 failed
  • SEED=0 cargo test -p blockifier --lib — 985 passed, 0 failed
  • cargo clippy -p blockifier --lib — clean
  • scripts/rust_fmt.sh — no changes needed
  • Reviewed by an Opus subagent for correctness (verified error-semantics parity against cairo-vm's Memory::get/get_relocatable) and style; findings applied.

Generated by Claude Code

claude added 2 commits August 24, 2026 13:30
read_felt_array's empty-span check already reads both the start and
end pointer cells via get_maybe; the non-empty path then re-read the
same two memory cells via get_relocatable. Reuse the values already
fetched instead of doing the memory lookup twice, since this function
runs on every array-typed syscall argument (calldata, event keys/data,
etc.) across a block.
Rename the helper to relocatable_from_memory_value (expect_* implies
panicking in Rust), move it below its public caller, and rephrase the
comment to state why the second read is skipped. Add tests pinning the
exact error variant and address for the missing-cell and wrong-type
cases, since preserving get_relocatable's error semantics is the main
correctness risk of this refactor.
@cursor

cursor Bot commented Aug 24, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Hot-path refactor of syscall array parsing with intended identical behavior; risk is limited to error-variant/address mismatches, which the new tests cover.

Overview
Avoids a second VM memory lookup in read_felt_array on the common non-empty path. Start/end cells already fetched with get_maybe for the empty-span check are converted in-place instead of being re-read via get_relocatable.

A small helper, relocatable_from_memory_value, maps those cached values to the same MemoryError variants (ExpectedRelocatable vs UnknownMemoryCell) that get_relocatable would have produced.

Tests now assert exact error type and address, and cover felt-typed end pointers and unwritten cells.

Reviewed by Cursor Bugbot for commit ecdf1be. Bugbot is set up for automated code reviews on this repo. Configure here.

@reviewable-StarkWare

Copy link
Copy Markdown

This change is Reviewable

@github-actions

Copy link
Copy Markdown

Artifacts upload workflows:

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.

4 participants