GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||
GeezerVison is showing github.com — gh-131798: JIT: replace _CHECK_METHOD_VERSION with _CHECK_FUNCTION_VERSION_INLINE by Zheaoli · Pull Request #135022 · python/cpython Conversation
|
| 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.
The reason will be displayed to describe this comment to others. Learn more.
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
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
If I'm correct, TestObject().test will just generate _CHECK_FUNCTION_VERSION code, It's a normal function.
Reference the test code here
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
@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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
…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]>
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@brandtbucher I update some idea about the test, PTAL when you got time. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.