Skip to content

starknet_os: preallocate CompressionSet vectors to avoid reallocation - #15003

Open
gkaempfer wants to merge 2 commits into
mainfrom
claude/perf/starknet-os-compress-prealloc-58204
Open

starknet_os: preallocate CompressionSet vectors to avoid reallocation#15003
gkaempfer wants to merge 2 commits into
mainfrom
claude/perf/starknet-os-compress-prealloc-58204

Conversation

@gkaempfer

Copy link
Copy Markdown
Contributor

Context

Follow-up to #14779, which preallocated the two Vecs inside stateless_compression::utils::decompress that grew from Vec::new() even though their final length was already known from the decoded header.

That PR's sibling function, compress(), has the same pattern and is also on a production hot path: compress() is invoked from the compression_hint Cairo VM hint (crates/starknet_os/src/hints/hint_implementation/stateless_compression/implementation.rs), a hint registered in crates/starknet_os/src/hints/enum_definition.rs and executed once per block during OS/Cairo execution — not test-only code.

Problem

CompressionSet::new (called from compress) builds two Vecs via Vec::new() + push inside a loop over values: &[Felt], even though both are bounded by values.len() before the loop starts:

  • bucket_index_per_elm: exactly one push per loop iteration, so its final length is always exactly values.len().
  • repeating_value_bucket: pushed only on the "value already seen" branch, so its final length is at most values.len(), never more.

Growing both from capacity 0 means ~log2(N) reallocations and O(N) redundant (usize, usize)/usize copies per block, on a per-block hot path — the same class of inefficiency #14779 fixed on the decompress side.

Fix

Preallocate both Vecs with values.len() via Vec::with_capacity, mirroring the pattern already used elsewhere in this file (get_bucket_offsets, unpack_felts, unpack_felts_to, and the two call sites decompress already uses from #14779).

This is behavior-neutral — Vec::with_capacity cannot panic or change output; capacity isn't observable through any of the Vec APIs used here (len, iter, slice deref).

Verification

With sequencer_venv active and RUSTC_WRAPPER="" (sccache unavailable in this environment):

  • cargo build -p starknet_os — clean.
  • SEED=0 cargo test -p starknet_os stateless_compression — 35 passed, 0 failed.
  • cargo clippy -p starknet_os --all-targets — clean.
  • scripts/rust_fmt.sh — no diff beyond this change.

An independent Opus review verified: the capacity for bucket_index_per_elm is exact and for repeating_value_bucket is a safe upper bound never exceeded; no new panic or observable-behavior surface; the optimization is real (not a no-op) given mainnet block sizes; and the change matches this file's existing Vec::with_capacity conventions.


🤖 Generated with Claude Code


Generated by Claude Code

claude added 2 commits July 13, 2026 05:10
`decompress` runs once per block over the full state diff. Both
`unique_values` and `result` grow via push/extend from `Vec::new()`
even though their final lengths are already known from the decoded
header (sum of unique-value bucket lengths, and data_len,
respectively), causing avoidable reallocations for large state diffs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YHx5w4Xp5DBX5oBBTTFEvt
Follow-up to #14779, which preallocated the two Vecs in decompress().
The sibling compress() path (CompressionSet::new, invoked once per
block via the compression_hint Cairo VM hint) has the same pattern:
bucket_index_per_elm and repeating_value_bucket grow from Vec::new()
via push in a per-value loop, even though both are bounded by
values.len() before the loop starts.
@cursor

cursor Bot commented Aug 19, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Behavior-neutral capacity hints only; no logic, API, or output changes on a performance-sensitive but non-security path.

Overview
Preallocates Vec capacity in stateless compression so hot-path compress / decompress work avoids repeated growth and copies when final sizes are known up front.

In CompressionSet::new (used by compress), repeating_value_bucket and bucket_index_per_elm are created with Vec::with_capacity(values.len()) instead of empty vectors grown by push in the input loop.

In decompress, unique_values is sized to the sum of decoded bucket lengths and the output result vector is sized to data_len from the header, matching the existing with_capacity pattern elsewhere in this module.

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

@reviewable-StarkWare

Copy link
Copy Markdown

This change is Reviewable

@gkaempfer gkaempfer self-assigned this Aug 19, 2026
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