Common: Cache Parameter Values + activated EIPs for current Hardfork / SM copy() fix

[holgerd77]

First-round performance tests suggests that Common param() reads take a significant amount of time, see:

> import { Chain, Common, Hardfork } from '@ethereumjs/common'
undefined
> let c = new Common({ chain: Chain.Mainnet })
undefined
> console.time('Total'); for (let i = 0; i < 100000; i++) { let p = c.param('gasPrices', 'push') }; console.timeEnd('Total');
Total: 151.637ms

(a similar issue was already mentioned years ago by Patricio from HardHat in some separate peformance tracking issue, just to mention)

Time used in the 100s of miliseconds is significant, since the iteration set of 100000 is relatively close to real world conditions and an amount realistic to be called in e.g. the EVM during a block execution (since the param() method is used for every opcode gas calculation e.g.).

Looking at the code confirms that this is likely inefficiently coded, since every param() read again iterates through both all hardfork files and additionally through all EIP files.

This PR therefore aims to introduce a cache with the parameters at hand for currently active set of both hardforks and EIPs. This cache will be rebuild on a hardfork change or an EIP addition (which both either doesn't happen at all or at least doesn't happen often during a Common object livecycle).

[codecov]

Codecov Report

Merging #2994 (d914cbf) into master (fcc910e) will decrease coverage by 0.01%.
The diff coverage is 93.18%.

Additional details and impacted files

Flag	Coverage Δ
block	88.73% <80.00%> (+0.06%)	⬆️
blockchain	92.58% <ø> (ø)
client	87.35% <ø> (-0.05%)	⬇️
common	98.18% <93.85%> (-0.44%)	⬇️
ethash	∅ <ø> (∅)
evm	70.43% <100.00%> (ø)
rlp	∅ <ø> (∅)
statemanager	84.46% <100.00%> (+0.02%)	⬆️
trie	89.82% <ø> (+0.32%)	⬆️
tx	95.97% <100.00%> (-0.01%)	⬇️
util	86.78% <ø> (ø)
vm	79.20% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

[holgerd77]

Ok, as already pointed out in a call with Jochem: so this works and is siginifcant! 🎉

I tested by running the blockchain tests on the VM with npm run test:blockchain -- --hardfork=Shanghai, this repeatedly gives a ~ 12 seconds gain on the run:

1. With cache: 5:12.865 (m:ss.miliseconds)
2. Without cache: 5:24.238
3. Without cache: 5:23.659
4. With cache: 5:13.083

Also the console.time() test from above (151.637ms) shows relatively impressive gains:

> import { Chain, Common, Hardfork } from '@ethereumjs/common'
undefined
> let c = new Common({ chain: Chain.Mainnet })
undefined
> console.time('Total'); for (let i = 0; i < 100000; i++) { let p = c.param('gasPrices', 'push') }; console.timeEnd('Total');
Total: 6.211ms

Lol.

I fixed one additional thing since tests failed. If tests are passing would be ready for review. 🙂
(I would think this doesn't need additional test cases, since this kind of parameter access is tested in all sorts of ways and scenarios all over the place and beyond we do have tests for the param() method directly in Common as well)

[acolytec3]

This looks great overall. What is the significance of changing all the private elements to protected?

[holgerd77]

This looks great overall. What is the significance of changing all the private elements to protected?

That's our "standard" for the libraries, so preferring protected over private. This makes it easier for people to subclass Common (or another library) and adopt stuff (e.g. overwriting a then protected method). This is not possible if things are declared private. And from the outer visibility there's no difference.

[jochem-brouwer]

Some remarks and questions :) LGTM overall!

[holgerd77]

@acolytec3 @jochem-brouwer thanks for the reviews, have answered and/or applied changes. Ready for re-review once tests pass! 🙂

[jochem-brouwer]

LGTM!