Uh oh!
There was an error while loading. Please reload this page.
gh-131798: JIT: replace _CHECK_METHOD_VERSION with _CHECK_FUNCTION_VERSION_INLINE - #135022
Conversation
…ION_VERSION_INLINE Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
brandtbucher
left a comment
There was a problem hiding this comment.
Very cool, thanks! A few suggestions:
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| class TestObject: | ||
| def test(self, *args, **kwargs): | ||
| return args[0] | ||
| test_object = TestObject() | ||
| test_bound_method = TestObject.test.__get__(test_object) | ||
There was a problem hiding this comment.
Can you move this into test_method_guards_removed_or_reduced? global_identity needs to be at module scope for other reasons that don't apply here.
Also, I think it can be simplified to something like:
classTestObject:
deftest(self, arg):
returnargtest_bound_method=TestObject().testThere was a problem hiding this comment.
If I'm correct, TestObject().test will just generate _CHECK_FUNCTION_VERSION code, It's a normal function.
Reference the test code here
There was a problem hiding this comment.
Can you move this into
test_method_guards_removed_or_reduced?global_identityneeds to be at module scope for other reasons that don't apply here.
BTW, if I push the test object into the test_method_guards_removed_or_reduced. I think the method will not be treated by a const. So we the _CHECK_FUNCTION_VERSION_INLINE is not exist in final opcode.
There was a problem hiding this comment.
@Zheaoli is right here. Can you please move this back to global scope? It seems tests are failing as the global method is not being promoted to a constant.
There was a problem hiding this comment.
Can you please move this back to global scope? It seems tests are failing as the global method is not being promoted to a constant.
Sure
Uh oh!
There was an error while loading. Please reload this page.
…e-131798.JQRFvR.rst Co-authored-by: Brandt Bucher <brandtbucher@gmail.com>
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
Fidget-Spinner
left a comment
There was a problem hiding this comment.
Sorry for forgetting to review this. This looks like a good optimization --- _CHECK_FUNCTION_VERSION_INLINE has one fewer memory indirection than _CHECK_METHOD_VERSION
Zheaoli
commented
Jun 15, 2025
@brandtbucher I update some idea about the test, PTAL when you got time. |
…ION_VERSION_INLINE (pythonGH-135022) Signed-off-by: Manjusaka <me@manjusaka.me> Co-authored-by: Brandt Bucher <brandtbucher@gmail.com>
…ION_VERSION_INLINE (pythonGH-135022) Signed-off-by: Manjusaka <me@manjusaka.me> Co-authored-by: Brandt Bucher <brandtbucher@gmail.com>
…ION_VERSION_INLINE (pythonGH-135022) Signed-off-by: Manjusaka <me@manjusaka.me> Co-authored-by: Brandt Bucher <brandtbucher@gmail.com>
…ION_VERSION_INLINE (pythonGH-135022) Signed-off-by: Manjusaka <me@manjusaka.me> Co-authored-by: Brandt Bucher <brandtbucher@gmail.com>
Uh oh!
There was an error while loading. Please reload this page.