fix: call graph stability #3370
Conversation
the use of `set` (which does not guarantee order of its elements) instead of `dict` (which guarantees insertion order) led to instability across runs of the compiler between different versions of python. this commit patches the problem by using a derivation of `dict` to track the call graph instead of `set`.
Codecov Report
📣 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 #3370 +/- ##
==========================================
- Coverage 88.88% 88.73% -0.15%
==========================================
Files 85 85
Lines 10706 10709 +3
Branches 2233 2233
==========================================
- Hits 9516 9503 -13
- Misses 788 805 +17
+ Partials 402 401 -1
... and 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
It might be worth adding a unit & fuzz test here to guarantee that the ordering is always checked as part of the test suite in the future. |
|
Will add a test for call graph stability before merging. Do we need other fuzz tests to check bytecode is stable (i.e. it could be influenced for other reasons besides call graph stability) or just checking the call graph is in a certain order is fine? |
|
Maybe we should also check for |
|
i mean we can, but if bytecode stability is being tested, checking ir and asm stability is overkill IMO. since whoever runs into those test failures will probably dig into the ir and asm anyways |
|
@pcaversaccio added some tests, take a look? |
There was a problem hiding this comment.
LGTM, just one comment: it might be useful to fuzz the built-in decorators (view, nonreentrant etc.) as well so we're sure that any of the current (and any future introduced) decorators don't affect the call graph stability. It should have no effect at all but such tests can prevent and/or detect future introduced regressions.
the use of
set(which does not guarantee order of its elements) instead ofdict(which guarantees insertion order) led to instability across runs of the compiler between different versions of python. this commit patches the problem by using a derivation ofdictto track the call graph instead ofset.What I did
fix #3369
How I did it
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