| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
Woops. Thank you for the fix. I forgot && in C doesn't operate like and in Python :).
Sorry, something went wrong.
| uint32_t func_version = function_check_args(descr, 2, LOAD_ATTR) && | ||
| function_get_version(descr, LOAD_ATTR); | ||
| if (func_version == 0) { | ||
| if (!function_check_args(descr, 2, LOAD_ATTR)) { |
There was a problem hiding this comment.
Should we write the function version into the 16 bit dk_version field like we were discussing the other day?
Sorry, something went wrong.
There was a problem hiding this comment.
Maybe, but probably in another PR that checks version for all specialized calls.
Sorry, something went wrong.
There was a problem hiding this comment.
Can we write a test for this?
I can't think of one.
Sorry, something went wrong.
We can write something stable, then if it deopts the test fails: class A:
@property
def foo(s): pass
a = A()
def f():
a.foo
for _ in range(8):
f()
# dissassemble f and inspect that LOAD_ATTR_PROPERTY is inside
for _ in range(x):
f()
# dissassemble f and inspect that LOAD_ATTR_PROPERTY is still inside in every single iteration, else fail |
Sorry, something went wrong.
In general, I think we should probably have better testing that isn't as heavy as the stuff in test_dis.py. The stuff in test_opcache is good for testing regressions in behavior, but not useful for catching things like this. It shouldn't be too hard to set up a simple test harness that's able to assert that an opcode in co_code maps to another opcode at the same position in _co_code_adaptive after a specific number of calls. That would be capable of testing that simple, specific optimizations and deoptimizations work without lots of ongoing maintenance every time we change the bytecode. We could even extend it check cache values, so that we're confident that things like exponential backoff keep working as expected. |
Sorry, something went wrong.
…TRIBUTE_OVERRIDDEN specialization)
| Back | FazBrowse Home | New Git URL |
Stats show that LOAD_ATTR_PROPERTY has a near-100% failure rate. This is because it always sets the cached function version to 1, regardless of the actual value. LOAD_ATTR_GETATTRIBUTE_OVERRIDDEN has a similar bug, but it doesn't manifest itself since the incorrect function version is never used.
This fixes both bugs, which brings LOAD_ATTR hit rates up from ~80% to ~83% when running the pyperformance suite (although LOAD_ATTR_GETATTRIBUTE_OVERRIDDEN doesn't actually appear in the suite at all).