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

perf: remove compiler hotspots #2981

Merged
merged 11 commits into from
Jul 25, 2022

Conversation

charles-cooper
Copy link
Member

@charles-cooper charles-cooper commented Jul 23, 2022

What I did

reduce build time.

time vyper Vault.vy went from 0m13.121s to 0m3.521s
time vyper CurveCryptoSwap2.vy went from 0m53.054s to 0m5.259s

How I did it

investigating the time spent in various compiler phases, turns out 90% was spent in just three places.

the relevant lines triggered re-traversals of the entire AST, resulting
in O(n^2) build times. for non-trivial contracts near the 24KB limit,
this resulted in near-minute long build times.

this commit also adds timer utility functions so that the profiling can be easily done again.

How to verify it

Commit message

Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)

Description for the changelog

Cute Animal Picture

image

the changed lines triggered re-traversals of the entire AST, resulting
in O(n^2) build times. for non-trivial contracts near the 24KB limit,
this resulted in near-minute long build times.
@charles-cooper charles-cooper marked this pull request as ready for review July 23, 2022 19:25
@charles-cooper charles-cooper changed the title remove compiler hotspots [perf] remove compiler hotspots Jul 23, 2022
@charles-cooper charles-cooper changed the title [perf] remove compiler hotspots perf: remove compiler hotspots Jul 23, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jul 23, 2022

Codecov Report

Merging #2981 (0aa4ea5) into master (feb142b) will decrease coverage by 0.08%.
The diff coverage is 48.00%.

@@            Coverage Diff             @@
##           master    #2981      +/-   ##
==========================================
- Coverage   88.09%   88.01%   -0.09%     
==========================================
  Files          97       97              
  Lines       10852    10868      +16     
  Branches     2574     2572       -2     
==========================================
+ Hits         9560     9565       +5     
- Misses        832      844      +12     
+ Partials      460      459       -1     
Impacted Files Coverage Δ
vyper/ast/folding.py 91.59% <ø> (-0.14%) ⬇️
vyper/utils.py 84.68% <31.57%> (-5.32%) ⬇️
vyper/ast/expansion.py 93.93% <100.00%> (-0.18%) ⬇️
vyper/ast/nodes.py 93.17% <100.00%> (+0.33%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us.

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Jul 23, 2022

This pull request introduces 1 alert when merging 3612c7f into feb142b - view on LGTM.com

new alerts:

  • 1 for Module is imported with 'import' and 'import from'

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Jul 24, 2022

This pull request introduces 1 alert when merging efead2b into 6090953 - view on LGTM.com

new alerts:

  • 1 for Module is imported with 'import' and 'import from'

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Jul 24, 2022

This pull request introduces 2 alerts when merging bd3a4fb into 72bb12e - view on LGTM.com

new alerts:

  • 1 for Module is imported with 'import' and 'import from'
  • 1 for Redundant assignment

@charles-cooper charles-cooper enabled auto-merge (squash) July 25, 2022 03:27
@lgtm-com
Copy link
Contributor

lgtm-com bot commented Jul 25, 2022

This pull request introduces 1 alert when merging 0aa4ea5 into 72bb12e - view on LGTM.com

new alerts:

  • 1 for Module is imported with 'import' and 'import from'

@charles-cooper charles-cooper merged commit be2c59a into vyperlang:master Jul 25, 2022
@charles-cooper charles-cooper deleted the perf/compiler_hotspots branch July 25, 2022 04:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants