| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
Is the _PyBinopSpecializationDescr supposed to be extensible?
If so we'll need a means for extensions to free the structs they have allocated. Or is the intention that they are statically allocated?
If they aren't extensible, maybe the VM should allocate them and clean them up?
Sorry, something went wrong.
I assumed it's extensible and the extension manages it. The alternative is that it has a void* field that the extension sets and manages. The advantage of letting extensions allocate is that if there is no dynamic information, you don't need to allocate a new one for each instruction that uses the specialisation. |
Sorry, something went wrong.
|
Are there any microbenchmarks that show this improves performance? |
Sorry, something went wrong.
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Michael Droettboom <mdboom@gmail.com>
| } | ||
| break; | ||
| } | ||
| /* _GUARD_BINARY_OP_EXTEND is not a viable micro-op for tier 2 because it uses the 'this_instr' variable */ |
There was a problem hiding this comment.
FYI, this change made it so that any traces containing BINARY_OP_EXTEND can't be JIT-compiled. Is there a way to rewrite this to be tier-2 friendly? Mutating inline caches doesn't work there.
Sorry, something went wrong.
There was a problem hiding this comment.
We probably don't have to NULL the cache, just change the opcode to BINARY_OP and hope nobody tries to use the pointer in the cache anymore. Will make it harder to debug if they do.
Sorry, something went wrong.
|
Also, based on the benchmarking results that were posted, it looks like the coverage, sqlalchemy_declarative, and sqlalchemy_imperative benchmarks crashed on this branch but succeeded on the base commit. Were these fixed before merging? |
Sorry, something went wrong.
|
This seems to cause the refleaks unit test to fail for the test_datetime module. The leak seems to be related to the ZoneInfoTest class, which uses the array.array type. |
Sorry, something went wrong.
|
The crash should have been fixed. I'll check the leak. Re tier 2 - maybe there's a way but we can just deopt them when jitting if not. |
Sorry, something went wrong.
The follow code is enough to reprodue the issue: import array
import gc
def func():
a=array.array('B', b'hello world')
# call array_binop_specialize() 3 times, once per loop
print("specialize")
for _ in range(10): a[1]
print("specialize")
for _ in range(10): a[1]
print("specialize")
for _ in range(10): a[1]
func()
# array_subscr_free() is not called
del func
gc.collect()The specialization calls PyMem_Malloc() in array_binop_specialize(), but nothing calls PyMem_Free(): array_subscr_free() is not called. |
Sorry, something went wrong.
|
It seems that array_subscr_guard behaves incorrectly. It never returns 0 (at least I can't make it return 0), which is needed to trigger array_subscr_free. |
Sorry, something went wrong.
I don't think that's the issue. Free needs to happen when the code object is freed, and currently it's not. |
Sorry, something went wrong.
…ze BINARY_OP/SUBSCR, apply to arrays (python#133396)" This reverts commit 082dbf7.
|
I'm going to revert this so it doesn't delay the release today. It's not terribly important for this to be in 3.14. |
Sorry, something went wrong.
|
@iritkatriel I just ran JIT benchmarks on the head of this branch. Those three benchmarks are still broken, and it makes the JIT 1% slower (with individual benchmarks slowing down up to 25%, one benchmark using 8% more memory): So we should make sure to benchmark the JIT when this lands again, or find a new approach (I don't love having strong references in the inline caches, even just pointers to owned memory). |
Sorry, something went wrong.
…Y_OP/SUBSCR, apply to arrays (python#133396)
…ze BINARY_OP/SUBSCR, apply to arrays (python#133396)" (python#133498)
| Back | FazBrowse Home | New Git URL |
Benchmark results: https://github.com/faster-cpython/benchmarking-public/blob/main/results/bm-20250502-3.14.0a7%2B-ec344eb/bm-20250502-azure-x86_64-iritkatriel-array_subscr-3.14.0a7%2B-ec344eb-pystats-vs-base.md
Specialization stats reflect the change, no overall improvement in perf.