gh-146031: Allow keeping specialization enabled when specifying eval frame function#146032
gh-146031: Allow keeping specialization enabled when specifying eval frame function#146032DinoV wants to merge 2 commits intopython:mainfrom
Conversation
3adb88b to
3c76262
Compare
3c76262 to
f3b7b90
Compare
|
With the flag set, some calls to any given Python function will go through the custom function and some won't. As soon as any function is run in the interpreter, every function called by it (transitively) could be invisible to the custom frame evaluator. |
Yep, it's then up to the replacement interpreter to decide what it wants to do here. If they're just a wrapper than they probably don't want to set the flag. If they're actually interpreting the byte code than they are responsible for the implementation of the inlined calls and can dispatch to them in their own inlined interpreter. |
|
This seems reasonable. The performance impact should be zero for CPython (as the new flag is only checked if PEP 523 is active). |
| @@ -318,4 +318,7 @@ PyAPI_FUNC(_PyFrameEvalFunction) _PyInterpreterState_GetEvalFrameFunc( | |||
| PyInterpreterState *interp); | |||
| PyAPI_FUNC(void) _PyInterpreterState_SetEvalFrameFunc( | |||
There was a problem hiding this comment.
I don't think we want to change this API again. It break PyTorch every time we do so.
Maybe add another function to set the flag.
| JUMP_TO_LABEL(start_frame); \ | ||
| #define DISPATCH_INLINED(NEW_FRAME) \ | ||
| do { \ | ||
| assert(tstate->interp->eval_frame == NULL || \ |
There was a problem hiding this comment.
Can you use your IS_PEP523_HOOKED macro here?
| func_calls = [c for c in actual_calls if c == "func"] | ||
| self.assertEqual(len(func_calls), 0) | ||
|
|
||
| # But the normal interpreter loop still shouldn't be inlining things | ||
| func_outer_calls = [c for c in actual_calls if c == "func_outer"] | ||
| self.assertNotEqual(len(func_outer_calls), 0) |
There was a problem hiding this comment.
It is better to use the .count method of arrays here. I.e. self.assertEqual(actual_calls.count('func'), 0)
An eval frame evaluator doesn't necessarily want to disable specialization of Python -> Python calls... The custom eval frame can either hook the specializing functions its self or they can specify vectorcall on a function object which prevents the nested dispatch.
This adds an argument to
_PyInterpreterState_SetEvalFrameFuncwhich allows the specification of whether or not specialization should be allowed. Setting it to 1 will allow the specializer to optimize Python -> Python calls. It's up to the eval frame replacement to make sure that this will work for them.📚 Documentation preview 📚: https://cpython-previews--146032.org.readthedocs.build/