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

fix[venom]: promote additional memory locations to variables #4039

Merged
merged 22 commits into from
Oct 10, 2024

Conversation

harkal
Copy link
Collaborator

@harkal harkal commented May 21, 2024

What I did

How I did it

How to verify it

Commit message

promote additional memory locations (specifically, internal function
params allocated with `palloca`) to stack variables.

Description for the changelog

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@harkal harkal marked this pull request as ready for review May 21, 2024 13:51
Copy link
Member

@charles-cooper charles-cooper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small typo

vyper/venom/passes/mem2var.py Outdated Show resolved Hide resolved
Copy link

codecov bot commented May 28, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 90.31%. Comparing base (3371956) to head (f013a07).

Files Patch % Lines
vyper/venom/passes/mem2var.py 79.16% 3 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4039      +/-   ##
==========================================
- Coverage   91.22%   90.31%   -0.92%     
==========================================
  Files         108      108              
  Lines       15522    15543      +21     
  Branches     3410     3416       +6     
==========================================
- Hits        14160    14037     -123     
- Misses        930     1043     +113     
- Partials      432      463      +31     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@charles-cooper charles-cooper added this to the v0.4.0 milestone May 30, 2024
@charles-cooper
Copy link
Member

lgtm, @harkal lmk if you are ok with the changes i made and i will merge

@harkal
Copy link
Collaborator Author

harkal commented Oct 10, 2024

lgtm

@charles-cooper
Copy link
Member

some nice gains:

(vyper) ~/vyper $ time vyc --experimental-codegen tmp/branch_storm.vy | wc
      1       1    1575

real	0m1.217s
user	0m1.188s
sys	0m0.034s

(vyper) ~/vyper $ time vyc --experimental-codegen tmp/CurveStableSwapNG-0.4.1.vy | wc
      1       1   45227

real	0m4.460s
user	0m4.360s
sys	0m0.104s

vs master:

(vyper) ~/vyper $ time vyc --experimental-codegen tmp/branch_storm.vy | wc
      1       1    1619

real	0m1.197s
user	0m1.161s
sys	0m0.041s

(vyper) ~/vyper $ time vyc --experimental-codegen tmp/CurveStableSwapNG-0.4.1.vy | wc
      1       1   45435

real	0m4.436s
user	0m4.309s
sys	0m0.132s

@charles-cooper charles-cooper merged commit fdc05d6 into vyperlang:master Oct 10, 2024
156 checks passed
@charles-cooper charles-cooper deleted the fix/palloca branch October 10, 2024 19:27
@charles-cooper charles-cooper restored the fix/palloca branch October 10, 2024 19:27
@harkal harkal deleted the fix/palloca branch October 10, 2024 19:30
pcaversaccio added a commit to pcaversaccio/snekmate that referenced this pull request Oct 11, 2024
### 🕓 Changelog

This commit adds a Bash script, `generate_gas_snapshot.sh`, that includes
version and configuration information into the gas snapshot files to
improve traceability. Specifically, it prepends the `vyper` commit
version, the `forge` commit version, and `vyper` configuration settings
(e.g. optimisation mode) to the `snapshot` files. The script also
supports generating snapshots for the `default-venom` profile using the
`--venom` flag. Eventually, we pin the specific `vyper` commit
[`d8f4032355773cd5e57e8902bd50808490de39b5`](vyperlang/vyper@d8f4032)
as the subsequent commit
[`fdc05d6029816ef57294e98753d9a3d60c761ab8`](vyperlang/vyper@fdc05d6)
(PR [#4039](vyperlang/vyper#4039)) currently
breaks our `venom`-based tests (refer to issue
[#4288](vyperlang/vyper#4288) for details).

---------

Signed-off-by: Pascal Marco Caversaccio <[email protected]>
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.

2 participants