solidity icon indicating copy to clipboard operation
solidity copied to clipboard

Stack shuffling refactor

Open r0qs opened this issue 1 year ago • 5 comments

r0qs avatar Feb 13 '24 12:02 r0qs

Benchmarks results:

File Pipeline Bytecode size Time Exit code
verifier.sol legacy 4874 bytes 0.13 s 0
verifier.sol via-ir 4351 bytes 0.59 s 0
OptimizorClub.sol legacy 0 bytes 0.46 s 1
OptimizorClub.sol via-ir 22193 bytes 3.38 s 0
chains.sol legacy 5845 bytes 0.17 s 0
chains.sol via-ir 23043 bytes 15.12 s 0
  • This branch:
File Pipeline Bytecode size Time Exit code
verifier.sol legacy 4874 bytes 0.14 s 0
verifier.sol via-ir 4351 bytes 0.58 s 0
OptimizorClub.sol legacy 0 bytes 0.47 s 1
OptimizorClub.sol via-ir 22193 bytes 3.33 s 0
chains.sol legacy 5845 bytes 0.17 s 0
chains.sol via-ir 23043 bytes 12.15 s 0

r0qs avatar Apr 22 '24 14:04 r0qs

So only chains.sol is affected, just like in #14112? Odd. We need more benchmark contracts.

cameel avatar Apr 22 '24 14:04 cameel

It would be interesting to see it benchmarked the way I did in https://github.com/ethereum/solidity/pull/14854#issuecomment-2070222067

This would show pure compilation time on some actual projects (I chose Uniswap v4 and Seaport since their latest versions compile without any workarounds) but with less variance than we see in CI. Or at least in a way that makes it easier to run multiple times and average the results.

cameel avatar Apr 22 '24 16:04 cameel

On the topic of the PR itself - I tried to review it but the diff is almost useless due to all the moved code. It also has no description or changelog and commit descriptions are not very informative, so I'm not really sure what to expect here. The title sounds as if you were just making the code nicer but it does not seem to be all that you're doing here. If you could separate the refactors from actual functional changes, it would be much easier to review.

cameel avatar Apr 22 '24 17:04 cameel

This pull request is stale because it has been open for 14 days with no activity. It will be closed in 7 days unless the stale label is removed.

github-actions[bot] avatar Jun 21 '24 12:06 github-actions[bot]

This pull request is stale because it has been open for 14 days with no activity. It will be closed in 7 days unless the stale label is removed.

github-actions[bot] avatar Jul 12 '24 12:07 github-actions[bot]

This pull request was closed due to a lack of activity for 7 days after it was stale.

github-actions[bot] avatar Jul 20 '24 12:07 github-actions[bot]