Skip to content

feat(bench): add additional benchmarks#1240

Open
glottologist wants to merge 9 commits into
masterfrom
jason/benchmarks
Open

feat(bench): add additional benchmarks#1240
glottologist wants to merge 9 commits into
masterfrom
jason/benchmarks

Conversation

@glottologist

@glottologist glottologist commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

Describe the changes
A clear and concise description of what the bug fix, feature, or improvement is.

Related Issue(s)
Please link to the issue(s) that will be closed with this PR.

Checklist

  • Tests have been added/updated for the changes.
  • Documentation has been updated for the changes (if applicable).
  • The code follows Rust's style guidelines.

Additional Context
Add any other context about the pull request here.

Summary by CodeRabbit

  • Tests

    • Added numerous Criterion performance benchmarks covering sampling, shadow transactions, packing, reward calculations, Merkle operations, storage pricing/fees, solution-hash computation, VDF inner-loop, database workloads (block/index/chunk/ingress), and block-tree mark-tip performance.
  • Chores

    • Enabled Criterion as a dev-dependency across crates and registered multiple new custom benchmark targets and suites for focused performance measurement.

@coderabbitai

coderabbitai Bot commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds Criterion-based benchmark targets and multiple new benchmark binaries across many crates (Cargo.toml dev-dependency and [[bench]] entries) and adds new Criterion benchmark source files under crates/*/benches/. No public API or exported symbols were changed.

Changes

Cohort / File(s) Summary
Workspace & crate Cargo updates
crates/efficient-sampling/Cargo.toml, crates/irys-reth/Cargo.toml, crates/packing/Cargo.toml, crates/reward-curve/Cargo.toml, crates/types/Cargo.toml, crates/database/Cargo.toml, crates/domain/Cargo.toml, crates/vdf/Cargo.toml
Added criterion.workspace = true to dev-dependencies and declared crate-local [[bench]] targets (many with harness = false, one with required-features).
Efficient Sampling bench
crates/efficient-sampling/benches/sampling_bench.rs
New Criterion benchmarks for Ranges::next_recall_range, Ranges::reconstruct, and recall_range_is_valid using deterministic seeds and multiple input sizes.
IrYS Reth / shadow tx bench
crates/irys-reth/benches/shadow_tx_bench.rs
New benchmarks for shadow-transaction encode/decode/detect paths, including valid decode and rejection cases.
Packing benches
crates/packing/benches/packing_bench.rs
New packing benchmarks: in-place XOR, owned XOR/packing, Rust vs C entropy, and batch packing across network tiers; includes helper tier config.
Reward curve bench
crates/reward-curve/benches/reward_bench.rs
New benchmark exercising reward_between across multiple timestamp intervals and edge cases.
Types: merkle/pricing/solution_hash benches
crates/types/benches/merkle_bench.rs, crates/types/benches/pricing_bench.rs, crates/types/benches/solution_hash_bench.rs
Added Merkle tree/proof, pricing (term/perm/exp_neg_fp18), and solution-hash benchmarks with deterministic inputs and throughput reporting.
VDF bench update
crates/vdf/benches/vdf_bench.rs
Added bench_vdf_sha_inner_loop and registered it into the existing Criterion group to exercise inner-loop iteration counts.
Database benches
crates/database/benches/block_index_bench.rs, crates/database/benches/chunk_cache_bench.rs, crates/database/benches/ingress_proof_bench.rs
New DB benchmarks for block-index insert/read/delete, chunk-cache write/read, and ingress-proof store operations using temporary DB instances and test DB initialisation.
Domain bench
crates/domain/benches/block_tree_bench.rs
New benchmark measuring BlockTree::mark_tip across chain depths with helpers to build sealed blocks and linear chains.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • JesseTheRobot
  • DanMacDonald
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change: adding multiple new benchmark files across the codebase, which is clearly supported by the 12+ new benchmark files added.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jason/benchmarks

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@glottologist glottologist changed the title feat(bench): add additional benchmarks feat(bench): add additional benchmarks Mar 24, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/reward-curve/benches/reward_bench.rs`:
- Around line 30-35: The benchmark case name "large_t_underflow" is misleading
because at 240 years the decay factor is tiny but positive (decay_fp18 <
TOKEN_SCALE) so no underflow occurs; rename the case string "large_t_underflow"
to a clearer name such as "large_t" or "far_future" wherever it appears in the
benches (the tuple entry containing SECS_PER_YEAR, TOKEN_SCALE, decay_fp18 and
the string label), and update any test assertions, logs, or match arms that
reference "large_t_underflow" so they match the new name while leaving the
benchmark logic (use of SECS_PER_YEAR, safe_sub, decay_fp18) unchanged.

In `@crates/types/benches/pricing_bench.rs`:
- Around line 17-19: The helper function make_config() currently returns
ConsensusConfig::testnet(), which violates the guideline to use
ConsensusConfig::testing() for test/test-bench consistency; update make_config()
to return ConsensusConfig::testing() instead (replace ConsensusConfig::testnet()
with ConsensusConfig::testing() in the make_config function), unless testnet()
was intentionally chosen for production-like benchmarks—if so, add an inline
comment explaining the intent.

In `@crates/types/benches/solution_hash_bench.rs`:
- Line 9: Replace the hardcoded CHUNK_SIZE constant in the benchmark with the
canonical MAX_CHUNK_SIZE exported from the merkle module: remove or replace the
existing const CHUNK_SIZE: u64 = 256 * 1024; and import merkle::MAX_CHUNK_SIZE
(or the correct path to MAX_CHUNK_SIZE) and use that value (cast to u64 if
necessary) so the benchmark stays synchronized with production chunk size.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: d73704d1-524e-4f9b-860e-434dfdfc847a

📥 Commits

Reviewing files that changed from the base of the PR and between 6044ecc and 0f9db4c.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (13)
  • crates/efficient-sampling/Cargo.toml
  • crates/efficient-sampling/benches/sampling_bench.rs
  • crates/irys-reth/Cargo.toml
  • crates/irys-reth/benches/shadow_tx_bench.rs
  • crates/packing/Cargo.toml
  • crates/packing/benches/packing_bench.rs
  • crates/reward-curve/Cargo.toml
  • crates/reward-curve/benches/reward_bench.rs
  • crates/types/Cargo.toml
  • crates/types/benches/merkle_bench.rs
  • crates/types/benches/pricing_bench.rs
  • crates/types/benches/solution_hash_bench.rs
  • crates/vdf/benches/vdf_bench.rs

Comment thread crates/reward-curve/benches/reward_bench.rs Outdated
Comment thread crates/types/benches/pricing_bench.rs
Comment thread crates/types/benches/solution_hash_bench.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 10

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/efficient-sampling/benches/sampling_bench.rs`:
- Around line 61-70: Replace the current b.iter_custom loop with Criterion's
b.iter_batched so allocation of the Ranges buffer is done in the setup closure
and only ranges.reconstruct(&step_seeds, &partition_hash) is timed;
specifically, in the replacement use a setup closure that creates let ranges =
Ranges::new(64_840) (and captures or clones step_seeds/partition_hash as needed)
and a bench closure that calls ranges.reconstruct(&step_seeds, &partition_hash)
(or mutates a passed-in Ranges) and returns the measured result, using an
appropriate BatchSize so the allocation cost from Ranges::new is not included in
the measured reconstruct time.
- Around line 36-45: The benchmark currently uses b.iter_custom with
per-iteration setup calling Ranges::new and then timing
ranges.next_recall_range, which still incurs allocation/cache noise; replace
this pattern with Criterion's b.iter_batched (or iter_batched_ref) to separate
setup from timing: prepare Ranges::new in the setup closure and return it to the
timed closure that calls ranges.next_recall_range(1, &seeds[0],
&partition_hash), choosing SmallInput or LargeInput as appropriate and adjusting
batch size so allocations are not measured; update the bench function to call
b.iter_batched/setup and move creation of Ranges into the setup closure while
keeping only next_recall_range inside the timed closure.

In `@crates/irys-reth/benches/shadow_tx_bench.rs`:
- Around line 23-64: Bench closures in
bench_encode/bench_decode_roundtrip/bench_detect_and_decode currently omit
criterion::black_box and don't consume reject-path results which lets the
optimizer elide work and hides failures; update the closures that call
encode_prefixed_input, try_decode_prefixed, and detect_and_decode_from_parts
(and the reject-path benches) to wrap inputs and outputs with
criterion::black_box and to explicitly consume/assert results (use expect/unwrap
for success paths and assert_err or unwrap_err/black_box the Err for reject
paths) so the compiler cannot optimize away the measured work and failures are
surfaced; reference functions/values: bench_encode, bench_decode_roundtrip,
bench_detect_and_decode, encode_prefixed_input, try_decode_prefixed,
detect_and_decode_from_parts, make_block_reward_tx, SHADOW_TX_DESTINATION_ADDR.

In `@crates/packing/benches/packing_bench.rs`:
- Around line 71-83: The benchmark bench_packing_xor_owned calls
packing_xor_vec_u8 and ignores its return which allows the optimizer to elide
work; fix by passing the function's return through criterion::black_box so the
result is observed (use black_box around the return value inside the bench.iter
closure). Update the bench_packing_xor_owned closure to consume
packing_xor_vec_u8(...) via black_box to prevent optimization; reference the
functions bench_packing_xor_owned and packing_xor_vec_u8.
- Around line 181-182: The C-batch benchmark currently allocates and mutates the
same Vec<Vec<u8>> via let mut data: Vec<Vec<u8>> = (0..batch_size).map(|_|
vec![0_u8; CHUNK_SIZE]).collect(); which can bias results; change this benchmark
to use Criterion's iter_batched (or iter_batched_ref) pattern similar to the
other batch benchmark: create the fresh data in the setup closure (allocating
batch_size buffers of CHUNK_SIZE) and then mutate/process it in the test
closure, referencing the same identifiers (data, CHUNK_SIZE, batch_size) so the
allocation and mutation are separated between setup and measured phases and the
code path mirrors the other benchmark for consistent results.
- Around line 153-154: The benchmark currently creates `data: Vec<Vec<u8>>` once
(using `CHUNK_SIZE` and `batch_size`) and then calls
`capacity_pack_range_with_data`, which mutates the buffers, leading to
subsequent iterations operating on already-packed data; modify the benchmark to
produce fresh zero-filled `data` per iteration (either by using Criterion's
`iter_batched`/`iter_batched_ref` to allocate `data` for each run or by
resetting/zeroing each `Vec<u8>` inside the iteration closure) so each call to
`capacity_pack_range_with_data` gets unmodified buffers; update the code
locations that create `data` (the `data` binding and its use with
`capacity_pack_range_with_data`) accordingly so the benchmark measures
consistent, fresh inputs.

In `@crates/types/benches/merkle_bench.rs`:
- Around line 42-55: The benchmark bench_generate_data_root currently measures
chunk→root by regenerating leaves inside the loop (generate_leaves_from_chunks +
generate_data_root), so update it to be unambiguous: either rename the benchmark
group from "merkle/generate_data_root" to something like
"merkle/generate_leaves_and_data_root" to reflect the measured work, or split
into two benchmarks—keep bench_generate_data_root that only calls
generate_data_root on precomputed leaves and add a separate
bench_generate_leaves that measures generate_leaves_from_chunks—ensure you
reference the existing functions generate_leaves_from_chunks and
generate_data_root and adjust the group.bench_function/BenchmarkId names
accordingly.
- Around line 48-57: Benchmarked calls in the closure use fixed inputs/unused
outputs and can be optimized away; wrap the inputs and results with
criterion::black_box to prevent optimizer elision: when invoking
generate_leaves_from_chunks and generate_data_root inside the Benchmark closure
that iterates over chunks (see generate_leaves_from_chunks, generate_data_root,
and chunks), pass chunks.iter().map(|c| Ok(black_box(c.clone()))) into
generate_leaves_from_chunks and wrap the final generate_data_root(...) call in
black_box(...) (or wrap its return) so both inputs and outputs are protected.

In `@crates/types/benches/pricing_bench.rs`:
- Around line 28-31: Bench calls in the benchmark closure (the b.iter in the
group.bench_function) are missing black_box, which lets the optimizer skew
timings; wrap all inputs to calculate_term_fee (e.g., bytes, epochs, &config,
irys_price) with std::hint::black_box and also wrap the final result after
.expect(...) with black_box so the call and its return value cannot be
constant-folded or eliminated; update the closure passed to
group.bench_function/BenchmarkId::from_parameter to use black_box for inputs and
the expect result inside b.iter.

In `@crates/types/benches/solution_hash_bench.rs`:
- Around line 20-28: Wrap the compute_solution_hash invocations in the
Criterion::black_box to prevent the optimizer from eliding or rearranging the
calls: in the closure passed to group.bench_function for both "256KB_chunk" and
"empty_chunk", call black_box(compute_solution_hash(black_box(&chunk_256k),
black_box(offset), black_box(&seed))) (and similarly for the empty slice) so
that both the function return value and its inputs are black-boxed during
measurement.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 405b7ef7-473d-4271-96ca-71a1b85006ec

📥 Commits

Reviewing files that changed from the base of the PR and between 0f9db4c and 8c6a675.

📒 Files selected for processing (6)
  • crates/efficient-sampling/benches/sampling_bench.rs
  • crates/irys-reth/benches/shadow_tx_bench.rs
  • crates/packing/benches/packing_bench.rs
  • crates/types/benches/merkle_bench.rs
  • crates/types/benches/pricing_bench.rs
  • crates/types/benches/solution_hash_bench.rs

Comment thread crates/efficient-sampling/benches/sampling_bench.rs Outdated
Comment thread crates/efficient-sampling/benches/sampling_bench.rs Outdated
Comment thread crates/irys-reth/benches/shadow_tx_bench.rs
Comment thread crates/packing/benches/packing_bench.rs
Comment thread crates/packing/benches/packing_bench.rs Outdated
Comment thread crates/packing/benches/packing_bench.rs Outdated
Comment thread crates/types/benches/merkle_bench.rs Outdated
Comment thread crates/types/benches/merkle_bench.rs
Comment thread crates/types/benches/pricing_bench.rs
Comment thread crates/types/benches/solution_hash_bench.rs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 8

♻️ Duplicate comments (1)
crates/irys-reth/benches/shadow_tx_bench.rs (1)

57-64: ⚠️ Potential issue | 🟠 Major

Reject-path benches should assert Ok(None) rather than black-boxing raw results.

Lines 58 and 63 currently discard the contract outcome by black-boxing Result<Option<_>, _>. This can hide regressions (Err or Some) while still producing timings.

Suggested patch
     let non_shadow_addr = Some(Address::from([0x01; 20]));
     group.bench_function(BenchmarkId::from_parameter("non_shadow_reject"), |b| {
-        b.iter(|| black_box(detect_and_decode_from_parts(non_shadow_addr, &encoded)));
+        b.iter(|| {
+            let is_rejected = detect_and_decode_from_parts(non_shadow_addr, &encoded)
+                .expect("non_shadow_reject should not error")
+                .is_none();
+            black_box(is_rejected)
+        });
     });

     let non_shadow_data = vec![0_u8; 100];
     group.bench_function(BenchmarkId::from_parameter("no_prefix_reject"), |b| {
-        b.iter(|| black_box(detect_and_decode_from_parts(shadow_addr, &non_shadow_data)));
+        b.iter(|| {
+            let is_rejected = detect_and_decode_from_parts(shadow_addr, &non_shadow_data)
+                .expect("no_prefix_reject should not error")
+                .is_none();
+            black_box(is_rejected)
+        });
     });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/irys-reth/benches/shadow_tx_bench.rs` around lines 57 - 64, The
benchmark currently black-boxes the raw Result from detect_and_decode_from_parts
(called in the closures for BenchmarkId "non_shadow_reject" and
"no_prefix_reject"), which can hide regressions; change the bench closures to
invoke detect_and_decode_from_parts(non_shadow_addr, &encoded) and
detect_and_decode_from_parts(shadow_addr, &non_shadow_data) and assert that the
outcome equals Ok(None) (e.g. with assert_eq! or matches!) inside the b.iter
closure instead of black_box(...), keeping the same identifiers
(non_shadow_addr, encoded, shadow_addr, non_shadow_data,
detect_and_decode_from_parts) so the benches fail on Err or Some and only
measure the intended reject path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/efficient-sampling/benches/sampling_bench.rs`:
- Around line 38-40: The closure currently discards the result of
ranges.next_recall_range(1, &seeds[0], &partition_hash) by ending the statement
with a semicolon, causing the closure to return () and defeating Criterion's
black_box protection; fix it by removing the trailing semicolon so the closure
returns the computed value from next_recall_range (the call on ranges) instead.
- Line 30: The benchmark currently calls make_seeds(1000) but only uses
seeds[0], which wastes allocation; change the call to make_seeds(1) (or
otherwise produce a single seed) and update any dependent uses to index that
single element, i.e., adjust where the variable seeds and its indexing are used
(referencing make_seeds and the seeds variable) so the benchmark only allocates
the minimal number of seeds.
- Around line 85-88: Replace the `.expect("valid range")` call inside the
benchmark closure with `.unwrap()` to avoid the extra panic-message formatting
overhead on the hot path; specifically, update the invocation of
recall_range_is_valid(...) inside the b.iter(|| { ... }) loop so it uses
unwrap() instead of expect() to reduce benchmark noise.
- Around line 60-62: The call to ranges.reconstruct currently has a trailing
semicolon which discards its return value so Criterion's black_box cannot
observe the computed result; remove the semicolon so the closure returns the
value from ranges.reconstruct (the same fix applied in bench_next_recall_range).
Locate the closure where ranges.reconstruct(&step_seeds, &partition_hash) is
invoked and make it return that expression instead of ending with a semicolon,
ensuring the benchmark receives the reconstructed value for proper black-boxing.

In `@crates/irys-reth/benches/shadow_tx_bench.rs`:
- Around line 52-54: The benchmark currently only asserts the Result from
detect_and_decode_from_parts but not the Option inside it; change the call in
the closure so that the positive path requires Ok(Some(_)) — i.e., call
detect_and_decode_from_parts(shadow_addr, &encoded), assert the Result is Ok,
then assert the Option is Some and unwrap it (e.g. expect or match to require
Some) before passing to black_box; reference detect_and_decode_from_parts and
the bench closure so the benchmark measures the decode work and not an Ok(None)
reject path.

In `@crates/packing/benches/packing_bench.rs`:
- Around line 27-55: The "testing" and "testnet" PackingTier entries in
build_packing_tiers() are exercising the same packing workload because only
iterations and chain_id are used by packing and ConsensusConfig::testing() and
::testnet() currently share those values; fix by removing the redundant
"testing" tier or by making the two tiers differ in a field that actually
affects packing (e.g., change the iterations or chain_id for the testnet entry),
updating the array in build_packing_tiers() (and the PackingTier entries) so
each tier maps to a distinct packing workload.

In `@crates/types/benches/merkle_bench.rs`:
- Line 70: Replace the direct index access proofs[0] with
proofs.first().expect(...) to avoid panics with unclear messages: call
proofs.first().expect("bench setup returned no proofs") and adjust the
destructuring to handle the borrowed tuple (e.g. dereference the & returned by
first() or clone as needed) so you bind proof_bytes and target safely; update
the let binding that currently uses (ref proof_bytes, target) to use the value
from proofs.first().expect(...) in merkle_bench.rs.

In `@crates/types/benches/pricing_bench.rs`:
- Around line 44-58: The term_fee is computed once with a hard-coded proof count
(10) but the benchmark loop varies proofs (1, 10, 100), so compute term_fee
inside the loop using the current proofs value to keep inputs consistent; move
the call to calculate_term_fee(bytes, config.epoch.submit_ledger_epoch_length,
&config, proofs, irys_price).expect("term fee") into the group.bench_function
closure (or immediately before calling calculate_perm_fee_from_config) and then
pass that per-case term_fee into calculate_perm_fee_from_config(bytes, &config,
proofs, irys_price, term_fee).

---

Duplicate comments:
In `@crates/irys-reth/benches/shadow_tx_bench.rs`:
- Around line 57-64: The benchmark currently black-boxes the raw Result from
detect_and_decode_from_parts (called in the closures for BenchmarkId
"non_shadow_reject" and "no_prefix_reject"), which can hide regressions; change
the bench closures to invoke detect_and_decode_from_parts(non_shadow_addr,
&encoded) and detect_and_decode_from_parts(shadow_addr, &non_shadow_data) and
assert that the outcome equals Ok(None) (e.g. with assert_eq! or matches!)
inside the b.iter closure instead of black_box(...), keeping the same
identifiers (non_shadow_addr, encoded, shadow_addr, non_shadow_data,
detect_and_decode_from_parts) so the benches fail on Err or Some and only
measure the intended reject path.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c652daa1-94d7-422a-a440-6e5b8c980971

📥 Commits

Reviewing files that changed from the base of the PR and between 8c6a675 and 153da62.

📒 Files selected for processing (6)
  • crates/efficient-sampling/benches/sampling_bench.rs
  • crates/irys-reth/benches/shadow_tx_bench.rs
  • crates/packing/benches/packing_bench.rs
  • crates/types/benches/merkle_bench.rs
  • crates/types/benches/pricing_bench.rs
  • crates/types/benches/solution_hash_bench.rs

Comment thread crates/efficient-sampling/benches/sampling_bench.rs Outdated
Comment thread crates/efficient-sampling/benches/sampling_bench.rs Outdated
Comment thread crates/efficient-sampling/benches/sampling_bench.rs Outdated
Comment thread crates/efficient-sampling/benches/sampling_bench.rs
Comment thread crates/irys-reth/benches/shadow_tx_bench.rs
Comment thread crates/packing/benches/packing_bench.rs Outdated
Comment thread crates/types/benches/merkle_bench.rs
Comment thread crates/types/benches/pricing_bench.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/domain/benches/block_tree_bench.rs`:
- Around line 59-69: The benchmark currently measures repeated mark_tip calls on
an already-marked tip (tree.mark_tip on the same block inserted in the setup),
so it measures steady-state re-marking instead of the cost of transitioning a
fresh tip; change the bench so each iteration creates or inserts a new block
(use extend_chain/random_block, seal_block, tree.add_block) and then calls
tree.mark_tip exactly once on that newly-inserted block (do not pre-mark it in
setup), ensuring mark_tip is invoked on a fresh state per iteration to capture
the true new-tip transition cost for mark_tip and related paths.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 59a212b0-3fdd-433e-8e34-ed7feb7cdd92

📥 Commits

Reviewing files that changed from the base of the PR and between 153da62 and c3a41e7.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (6)
  • crates/database/Cargo.toml
  • crates/database/benches/block_index_bench.rs
  • crates/database/benches/chunk_cache_bench.rs
  • crates/database/benches/ingress_proof_bench.rs
  • crates/domain/Cargo.toml
  • crates/domain/benches/block_tree_bench.rs

Comment thread crates/domain/benches/block_tree_bench.rs
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