-
-
Notifications
You must be signed in to change notification settings - Fork 8.3k
py/vm.c: Avoid heap-allocating slices for built-ins. #12518
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
Conversation
Code size report:
|
Not sure why, but this is failing on unix CI. |
It's failing the "doesn't heap allocate" when using emit-native. Need to add an exclusion for that test when using emit-native. |
Done. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #12518 +/- ##
=======================================
Coverage 98.44% 98.44%
=======================================
Files 171 171
Lines 22192 22204 +12
=======================================
+ Hits 21847 21859 +12
Misses 345 345 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
04fa3f6
to
ec15a0b
Compare
I've rebased this on latest master, to see how it goes after ~2 years. |
This is looking good now. It enables basic slices to work without allocating on the heap, and only increases C stack usage when that feature is used. I think it should be merged. |
This commit adds a fast-path optimisation for when a BUILD_SLICE is immediately followed by a LOAD/STORE_SUBSCR for a native type, to avoid needing to allocate the slice on the heap. In some cases (e.g. `a[1:3] = x`) this can result in no allocations at all. We can't do this for instance types because the get/set/delattr implementation may keep a reference to the slice. Adds more tests to the basic slice tests to ensure that a stack-allocated slice never makes it to Python, and also a heapalloc test that verifies (when using bytecode) that assigning to a slice is no-alloc. This work was funded through GitHub Sponsors. Signed-off-by: Jim Mussared <jim.mussared@gmail.com> Signed-off-by: Damien George <damien@micropython.org>
This is an alternative to #10160 and implements a different variation on @damz's idea. It's still worth considering making the compiler do this (via a new
SLICE_AND_SUBSCR
opcode), but this way also has the benefit of handing the is-instance-type check in a simple way.Unlike #10160 though it only helps for bytecode and not the native emitter.
Fast path optimisation for when a BUILD_SLICE is immediately followed by a LOAD/STORE_SUBSCR for a native type to avoid needing to allocate the slice on the heap.
In some cases (e.g.
a[1:3] = x
) this can result in no allocations at all.We can't do this for instance types because the get/set/delattr implementation may keep a reference to the slice.
This work was funded through GitHub Sponsors.