py/vm.c: Avoid heap-allocating slices for built-ins. #12518
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.56% 98.57%
=======================================
Files 169 169
Lines 21946 21958 +12
=======================================
+ Hits 21632 21644 +12
Misses 314 314 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
py/vm.c
Outdated
| // this for instance types because the get/set/delattr | ||
| // implementation may keep a reference to the slice. | ||
| byte op = *ip++; | ||
| mp_obj_slice_t slice = { .base = { .type = &mp_type_slice }, .start = start, .stop = stop, .step = step }; |
There was a problem hiding this comment.
This possibly increases C stack usage of this function. Will need to quantify that.
There was a problem hiding this comment.
Indeed... +24 bytes of stack.
I've added a commit to address this (using alloca). Slight code size increase though (was +120, now +152).
There was a problem hiding this comment.
Reverted this for now... too complicated to figure out how this works with MICROPY_ENABLE_PYSTACK.
There was a problem hiding this comment.
I've pushed a commit to factor the new code into a "no inline" function. And tweaked the code a little wrt to variable usage.
It now has the same C stack usage as prior to this PR.
04fa3f6 to
ec15a0b
Compare
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 <[email protected]>
|
I've rebased this on latest master, to see how it goes after ~2 years. |
Gets VM C stack usage back to its original value. Signed-off-by: Damien George <[email protected]>
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_SUBSCRopcode), 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.