Skip to content

perf(forge): avoid unnecessary gas_snapshots clones#13969

Merged
mablr merged 5 commits intofoundry-rs:masterfrom
ArshLabs:refactor/remove-gas-snapshots-unnecessary-clones
Apr 30, 2026
Merged

perf(forge): avoid unnecessary gas_snapshots clones#13969
mablr merged 5 commits intofoundry-rs:masterfrom
ArshLabs:refactor/remove-gas-snapshots-unnecessary-clones

Conversation

@ArshLabs
Copy link
Copy Markdown
Contributor

@ArshLabs ArshLabs commented Mar 27, 2026

Motivation

run_tests_inner() iterates gas_snapshots (a BTreeMap<String, BTreeMap<String, String>>) twice, once for snapshot checking and once for emitting, using .clone().into_iter() each time. The map is accumulated across the per-suite loop and must survive each iteration, so ownership can't be taken. Neither path needs it.

Solution

Switches both to .iter(), dropping two unnecessary deep copies per test contract.

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@ArshLabs
Copy link
Copy Markdown
Contributor Author

ArshLabs commented Apr 5, 2026

bumping this, let me know if any changes are needed.

zerosnacks
zerosnacks previously approved these changes Apr 7, 2026
@zerosnacks zerosnacks changed the title refactor(forge): avoid unnecessary gas_snapshots clones in test runner optimization(forge): avoid unnecessary gas_snapshots clones in test runner Apr 7, 2026
mablr
mablr previously approved these changes Apr 23, 2026
Copy link
Copy Markdown
Collaborator

@mablr mablr left a comment

Choose a reason for hiding this comment

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

sgtm, thanks

@mablr mablr enabled auto-merge (squash) April 23, 2026 08:26
figtracer
figtracer previously approved these changes Apr 23, 2026
@mablr mablr dismissed stale reviews from figtracer, zerosnacks, and themself via c87e77f April 23, 2026 08:39
@mablr mablr requested review from figtracer and zerosnacks April 23, 2026 08:39
@DaniPopes DaniPopes changed the title optimization(forge): avoid unnecessary gas_snapshots clones in test runner perf(forge): avoid unnecessary gas_snapshots clones Apr 25, 2026
@ArshLabs
Copy link
Copy Markdown
Contributor Author

@zerosnacks @mablr anything else to update from my end?

Copy link
Copy Markdown
Collaborator

@mablr mablr left a comment

Choose a reason for hiding this comment

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

lgtm, pending another approval.

@mablr mablr merged commit 415cef0 into foundry-rs:master Apr 30, 2026
28 of 30 checks passed
@github-project-automation github-project-automation Bot moved this to Done in Foundry Apr 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants