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: typechecker hotspot #3318

Merged

Conversation

charles-cooper
Copy link
Member

@charles-cooper charles-cooper commented Mar 9, 2023

What I did

fix #3316

How I did it

How to verify it

check that the example in #3316 compiles in under a second

Commit message

`get_possible_types_from_node` had blowup for deeply nested expressions.
this commit caches intermediate results using dynamic programming to
prune the calculation time.

note that caching has a funny interaction with the type inference
algorithm for loop iterators (i.e., try all different types for the loop
iterator, and pick the first one which does not throw an exception) -
since the loop iterator inference algorithm depends on optimistically
trying different types, some cached values can be invalid in the case
that they are generated inside of the loop iterator inference algorithm,
because the exception is caught and handled, but the state changes
(i.e., changes to the cache) are not rolled back. to solve this, this
commit introduces a simple commit/rollback mechanism to deal with state
changes in the type checker. it remains to be seen whether this
mechanism will be more generally useful in the future for other
potentially stateful type-checking operations, or if a better long-term
solution is to simply redesign how the type checker infers loop
iterators.

Description for the changelog

Cute Animal Picture

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

get_possible_types_from_node had blowup for deeply nested expressions.
this caches intermediate results using dynamic programming to prune the
calculation time.
@pcaversaccio
Copy link
Collaborator

I quickly checked, and can confirm my example compiles under a second with the following init code:

0x6102fd610011610000396102fd610000f360003560e01c346102eb576381d18d8781186102e457602436106102eb576004356040527ffffffffffffffffffffffffffffffffffffffffffffffffdb731c958f34d94c16004351361005a576000606052602060606102e2565b680755bf798b4a1bf1e460043513156100ca5760166060527f4d6174683a207761645f657870206f766572666c6f770000000000000000000060805260605060605180608001601f826000031636823750506308c379a06020526020604052601f19601f6060510116604401603cfd5b6503782dace9d960043580604e1b9050056040526b8000000000000000000000006bb17217f7d1cf79abc9e3b3986040518060601b905005018060601d90506060526bb17217f7d1cf79abc9e3b39860605102604051036040526d02d16720577bd19bf614176fe9ea6040516c10fe68e7fd37d0007b713f765060405101028060601d90500160805279d835ebba824c98fb31b83b2ca45c0000000000000000000000006040516e0587f503bb6ea29d25fcb7401964506080516d04a4fd9f2a8b96949216d2255a6c6040516080510103028060601d905001020160a0526e05180bb14799ab47a8a8cb2a527d576040516e02c72388d9f74f51a9331fed693f156040516db1bbb201f443cf962f1a1d3db4a56040516d1a521255e34f6a5061b25ef1c9c46040516d0277594991cfc85f6e2461837cd96040516c240c330e9fb2d9cbaf0fd5aafc60405103028060601d905001028060601d905003028060601d905001028060601d905003028060601d90500160c05260c05160a0510560e05274029d9dc38563c32e5c2f6dc192ee70ef65f9978af360e0510260605160c3037f800000000000000000000000000000000000000000000000000000000000000081146102eb576000037fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff8113156102c55781811b6102cc565b81816000031d5b90509050600081126102eb576101005260206101005bf35b5060006000fd5b600080fda165767970657283000302000b

vyper/semantics/analysis/utils.py Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Apr 25, 2023

Codecov Report

Merging #3318 (9c8a610) into master (7a64d4b) will increase coverage by 0.01%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master    #3318      +/-   ##
==========================================
+ Coverage   88.79%   88.81%   +0.01%     
==========================================
  Files          84       84              
  Lines       10637    10654      +17     
  Branches     2221     2223       +2     
==========================================
+ Hits         9445     9462      +17     
  Misses        790      790              
  Partials      402      402              
Impacted Files Coverage Δ
vyper/semantics/analysis/__init__.py 100.00% <100.00%> (ø)
vyper/semantics/analysis/local.py 91.78% <100.00%> (+0.04%) ⬆️
vyper/semantics/analysis/utils.py 92.45% <100.00%> (+0.39%) ⬆️
vyper/semantics/namespace.py 97.22% <100.00%> (ø)
vyper/semantics/types/__init__.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@charles-cooper charles-cooper merged commit 66930fd into vyperlang:master Apr 25, 2023
@charles-cooper charles-cooper deleted the fix/typechecker_hotspot branch May 31, 2023 17:38
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.

Vyper compiler fails to compile long unsafe expression
4 participants