gh-131798: JIT: replace _CHECK_METHOD_VERSION with _CHECK_FUNCTION_VERSION_INLINE#135022
gh-131798: JIT: replace _CHECK_METHOD_VERSION with _CHECK_FUNCTION_VERSION_INLINE#135022Fidget-Spinner merged 10 commits intopython:mainfrom
Conversation
…ION_VERSION_INLINE Signed-off-by: Manjusaka <[email protected]>
Signed-off-by: Manjusaka <[email protected]>
…ka/method-version
Signed-off-by: Manjusaka <[email protected]>
Signed-off-by: Manjusaka <[email protected]>
brandtbucher
left a comment
brandtbucher
left a comment
There was a problem hiding this comment.
Very cool, thanks! A few suggestions:
Misc/NEWS.d/next/Core_and_Builtins/2025-06-02-20-13-37.gh-issue-131798.JQRFvR.rst
Outdated
Show resolved
Hide resolved
| 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:
class TestObject:
def test(self, arg):
return arg
test_bound_method = TestObject().test
There 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
…e-131798.JQRFvR.rst Co-authored-by: Brandt Bucher <[email protected]>
Signed-off-by: Manjusaka <[email protected]>
Signed-off-by: Manjusaka <[email protected]>
Signed-off-by: Manjusaka <[email protected]>
Fidget-Spinner
left a comment
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
|
@brandtbucher I update some idea about the test, PTAL when you got time. |
Signed-off-by: Manjusaka <[email protected]>
…ION_VERSION_INLINE (pythonGH-135022) Signed-off-by: Manjusaka <[email protected]> Co-authored-by: Brandt Bucher <[email protected]>
…ION_VERSION_INLINE (pythonGH-135022) Signed-off-by: Manjusaka <[email protected]> Co-authored-by: Brandt Bucher <[email protected]>
…ION_VERSION_INLINE (pythonGH-135022) Signed-off-by: Manjusaka <[email protected]> Co-authored-by: Brandt Bucher <[email protected]>
…ION_VERSION_INLINE (pythonGH-135022) Signed-off-by: Manjusaka <[email protected]> Co-authored-by: Brandt Bucher <[email protected]>
4 participants
Uh oh!
There was an error while loading. Please reload this page.