Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reuse optimized IR for bytecode dependencies #15182

Closed
wants to merge 1 commit into from

Conversation

cameel
Copy link
Member

@cameel cameel commented Jun 5, 2024

The quick and easy solution for #15179, which only reuses the IR.

@cameel cameel self-assigned this Jun 5, 2024
@cameel

This comment was marked as outdated.

@cameel
Copy link
Member Author

cameel commented Jun 7, 2024

Here's some more benchmarking with #15183:

Project legacy IR (0.8.26) IR (this PR) Speedup
openzeppelin 10 s 41 s 36 s 12%
uniswap-v4 22 s 259 s 140 s 46%
eigenlayer 80 s 1324 s 622 s 53%
sablier-v2 153 s 1952 s 889 s 54%

Some important notes:

  • This time the numbers are averages from several runs. The small difference for OZ is actually reproducible and not just a fluke.
  • The previous results for Uniswap were on a commit from 2024-05-10. I updated to 2024-06-06 before this benchmark and apparently it now compiles a lot slower even with a release binary. I think they must have added more tests. Still, this seems to make the effect of the PR even more pronounced.
  • Sablier fails with StackTooDeep when compiled via IR. I still included it because this seems to happen towards the end compilation and the result seems in line with the other projects in the table, all of which did complete without errors.

@cameel
Copy link
Member Author

cameel commented Jun 7, 2024

And here is the timing from external tests (extracted using the script from #15023 (comment)):

Project Before After Diff
brink 4 s 4 s 0 s
colony 100 s 99 s -1 s
elementfi 98 s 82 s -16 s
ens 26 s 35 s 9 s
euler 30 s 32 s 2 s
gnosis
gp2 38 s 30 s -8 s
pool-together 35 s 33 s -2 s
uniswap 47 s 37 s -10 s
yield_liquidator 22 s 22 s 0 s
zeppelin 177 s 149 s -28 s

They numbers don't seem particularly reliable though. For example the longer running time for ens is unexpected and I suspect it's just the CI variance, but then most of the other results don't tell us anything either because they're in a similar range.

Also, I'm not sure how OpenZeppelin in CI takes ~150 s to compile. The main difference is that it's being compiled with Hardhat rather than Foundry, but such a difference still seems odd. Is it downloading something rather than just building? Is the CI machine that slow?

@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 22, 2024
@cameel cameel removed the stale The issue/PR was marked as stale because it has been open for too long. label Jun 23, 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 Jul 8, 2024
@cameel cameel removed the stale The issue/PR was marked as stale because it has been open for too long. label Jul 9, 2024
@cameel
Copy link
Member Author

cameel commented Jul 16, 2024

I'm closing this since we won't be merging it. The actual fix is based on caching optimized IR instead.

@cameel cameel closed this Jul 16, 2024
@ethereum ethereum deleted a comment from github-actions bot Jul 16, 2024
@ethereum ethereum deleted a comment from github-actions bot Jul 16, 2024
@cameel cameel mentioned this pull request Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant