Skip to content

Stack shuffling refactor#14851

Closed
r0qs wants to merge 3 commits intodevelopfrom
stackShufflingPerformance
Closed

Stack shuffling refactor#14851
r0qs wants to merge 3 commits intodevelopfrom
stackShufflingPerformance

Conversation

@r0qs
Copy link
Member

@r0qs r0qs commented Feb 13, 2024

No description provided.

@r0qs r0qs force-pushed the stackShufflingPerformance branch 5 times, most recently from 4102ad6 to 5f291e4 Compare February 13, 2024 15:47
@r0qs r0qs force-pushed the stackShufflingPerformance branch from 5f291e4 to 16e9a74 Compare February 13, 2024 16:11
@r0qs r0qs self-assigned this Feb 19, 2024
@r0qs r0qs force-pushed the stackShufflingPerformance branch 2 times, most recently from c8aceb0 to cc502d2 Compare February 19, 2024 13:57
@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Mar 5, 2024
@argotorg argotorg deleted a comment from github-actions bot Mar 5, 2024
@r0qs r0qs removed the stale The issue/PR was marked as stale because it has been open for too long. label Mar 5, 2024
@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Mar 20, 2024
@argotorg argotorg deleted a comment from github-actions bot Mar 25, 2024
@r0qs r0qs removed the stale The issue/PR was marked as stale because it has been open for too long. label Mar 25, 2024
@r0qs r0qs force-pushed the stackShufflingPerformance branch from cc502d2 to 700c5fa Compare March 25, 2024 12:01
@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Apr 8, 2024
@cameel cameel removed the stale The issue/PR was marked as stale because it has been open for too long. label Apr 8, 2024
@r0qs r0qs force-pushed the stackShufflingPerformance branch from 700c5fa to b93d4ac Compare April 22, 2024 14:09
@argotorg argotorg deleted a comment from github-actions bot Apr 22, 2024
@r0qs r0qs force-pushed the stackShufflingPerformance branch from b93d4ac to aba9a72 Compare April 22, 2024 14:21
@r0qs
Copy link
Member Author

r0qs commented Apr 22, 2024

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

@cameel
Copy link
Collaborator

cameel commented Apr 22, 2024

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

@cameel
Copy link
Collaborator

cameel commented Apr 22, 2024

It would be interesting to see it benchmarked the way I did in #14854 (comment)

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
Copy link
Collaborator

cameel commented Apr 22, 2024

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.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label May 7, 2024
@argotorg argotorg deleted a comment from github-actions bot May 7, 2024
@r0qs r0qs removed the stale The issue/PR was marked as stale because it has been open for too long. label May 7, 2024
@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label May 22, 2024
@argotorg argotorg deleted a comment from github-actions bot May 22, 2024
@r0qs r0qs removed the stale The issue/PR was marked as stale because it has been open for too long. label May 22, 2024
@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Jun 6, 2024
@r0qs r0qs removed the stale The issue/PR was marked as stale because it has been open for too long. label Jun 7, 2024
@argotorg argotorg deleted a comment from github-actions bot Jun 7, 2024
@github-actions
Copy link

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 github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Jun 21, 2024
@r0qs r0qs removed the stale The issue/PR was marked as stale because it has been open for too long. label Jun 28, 2024
@github-actions
Copy link

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 github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Jul 12, 2024
@github-actions
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

closed-due-inactivity stale The issue/PR was marked as stale because it has been open for too long.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants