perf(test-forks): memoize per-fork gas costs - #3303
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/amsterdam #3303 +/- ##
================================================
Coverage 93.49% 93.49%
================================================
Files 624 624
Lines 37056 37056
Branches 3394 3394
================================================
Hits 34647 34647
Misses 1653 1653
Partials 756 756
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
LouisTsai-Csie
left a comment
LouisTsai-Csie
left a comment
There was a problem hiding this comment.
Some comments on the new test cases: I'm unsure if they properly test cache behavior.
| AUTHORIZATION_EXISTING_AUTHORITY = auto() | ||
|
|
||
|
|
||
| MEMOIZED_FORK_METHODS = ("gas_costs",) |
There was a problem hiding this comment.
Should we move it under BaseForkMeta?
| def _all_fork_classes() -> List[Type[BaseFork]]: | ||
| """ | ||
| Return every concrete fork class. | ||
|
|
||
| Transition forks are excluded: they are assembled from | ||
| `TransitionBaseClass` rather than `BaseFork`, so they do not declare the | ||
| memoized methods at all and delegate to a concrete fork per block. | ||
| """ | ||
| return sorted(get_forks(), key=str) |
There was a problem hiding this comment.
This should be an fixture instead of a function, so we dont need to sort the array for each invocation.
Something like this:
@pytest.fixture
def tx_gas_limit(fork: Fork, gas_benchmark_value: int) -> int:
"""Return the transaction gas limit cap."""
return fork.transaction_gas_limit_cap() or gas_benchmark_value
| first = getattr(fork, method_name)() | ||
| assert getattr(fork, method_name)() is first |
There was a problem hiding this comment.
I am not very sure about the intention of this comparison, is it necessary?
There was a problem hiding this comment.
Let me check, this is Claude generated, will report back 😄 👍
2 participants
Description
Fork.gas_costs()was recomputed on every call. Each EIP layer in the fork hierarchy implements it asreplace(super().gas_costs(), ...), so a single call on a late fork walks the whole chain and allocates oneGasCostsper layer: 16dataclasses.replacecalls for Amsterdam, at 787 us per call.The filler calls it in the inner loop of the benchmark gas searches (
transaction_intrinsic_cost_calculatorinside_binary_search_iterations),where it accounted for ~17% ofdataclasses.replacetime in a py-spy profile of a stateful fill.BaseForkMeta.__new__now wraps everygas_costsoverride in a per-fork cache, so thesuper()chain runs once per fork rather than once per call. Wrapping in the metaclass rather than at each of the 18 definition sites is what makes this work:Fork.gas_costs()resolves through the MROto the last EIP that overrode it, and only a cache on that override can return before the chain runs. It also means a new EIP cannot silently opt out.Each override keeps its own cache keyed on
cls, so a half-assembled value from the middle of asuper()chain can never be handed out as the final one, and no fork can serve another fork's value. The abstract declaration onBaseForkis left alone soabcstill reports it unimplemented.Sharing one object is safe because
GasCostsis a frozen dataclass whose every field is anint-- there is nothing a caller could mutate. TheMEMOIZED_FORK_METHODSdocstring states that contract for anything added later, and a unit test enforces it.Measured, per call:
End to end on
fill --fork=Prague tests/prague/eip7623_increase_calldata_cost/(1467 fixtures, single process, 3 runs each): 53.16 s -> 38.99 s,a 1.36x speedup.Output is unchanged:
GasCostsis byte-identical for all 35 forks, dumped and diffed against the unmodified base.Related Issues or PRs
N/A.
Checklist
ruff check,ruff format --checkandmypyare clean repo-wide (justis unavailable in thisenvironment; the 3 remaining mypy errors are the pre-existingrust_pyspec_glue/ethashimport stubs that--group optimizedsupplies).pytest packages/testing/srcis 1941 passed, 31 skipped, 19 xfailed, 2 xpassed.<type>(<area>): <title>