[do-not-merge] - packing gas testing #570
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why this should be merged
It shouldn't just a demo of testing packing efficency. If there are no problems with the testing method I will cut a PR to replace the existing packing method with the old one and keep the testing helpers to cover cases with multiple validators.
How this works
_defaultSubnetConversionData
_oldSubnetConversionDataPack
to mimic the previous method of usingabi.encodePacked
in a loop.abi.encodePacked
)and new (manual byte by byte packing) methods, and a third one to ensure that their outputs matchbytes calldata
tobytes memory
. This is a significant change since reading from calldata is a lot cheaper than memory but I don't think that it should change the outcome of the test since it's still an apples to apples comparison.How this was tested
Ran manual forge tests with different num InitialValidators counts:
Below are the results showing that doing
abi.encodePacked
in a loop is a lot more efficient given this testing methodology.How is this documented