GH-88691: Shrink the CALL caches
#103230
GH-88691: Shrink the CALL caches
#103230
Conversation
brandtbucher
added
performance
|
Updating to get rid of the cached function version, too. We're only using this to confirm that the function is still "simple" and (in the case of As with the recent |
| int defcount = (int)PyTuple_GET_SIZE(func->func_defaults); | ||
| assert(defcount <= code->co_argcount); | ||
| int min_args = code->co_argcount - defcount; | ||
| DEOPT_IF(argcount > code->co_argcount, CALL); |
There was a problem hiding this comment.
Why do we need these checks at all?
I suspect that I added them, but I'm not sure why.
If func->func_version == func_version then the only variable is argcount which is oparg + is_meth.
So we only need to know which of the two possible values of is_meth is safe.
Can precompute those at specialization, so we only need to check is_meth at runtime.
There was a problem hiding this comment.
Why do we need these checks at all? I suspect that I added them, but I'm not sure why.
It looks like we just had space for it at the time (when cache sizes were fixed).
If
func->func_version == func_versionthen the only variable isargcountwhich isoparg + is_meth. So we only need to know which of the two possible values ofis_methis safe. Can precompute those at specialization, so we only need to checkis_methat runtime.
The obvious way to do this would be to split this into two specializations: one that expects is_meth, and one that doesn't.
With that said, I'm not sure it's worth breaking up. There would be a decent amount of code duplication, and this opcode is relatively rare (~0.1% of all instructions executed). So maybe we should just leave it.
(Maybe if we really wanted to, we could use the low bit of the func_version cache entry.)
| DEOPT_IF(func->func_version != func_version, CALL); | ||
| DEOPT_IF(!_PyFunction_IsSimple(func), CALL); | ||
| PyCodeObject *code = (PyCodeObject *)func->func_code; | ||
| DEOPT_IF(code->co_argcount != argcount, CALL); |
There was a problem hiding this comment.
If we keep the version number check, we can convert this to a check on is_meth, which might be quicker as it saves reaching into the function object.
|
When you're done making the requested changes, leave the comment: |
|
Makes sense. I'll revert my latest commit and land just the |
This reverts commit d02b607.
3 participants
Nothing fancy here, just recompute
min_argsin theCALL_PY_WITH_DEFAULTSimplementation rather than caching it. It's not a particularly expensive thing to recompute, and is only used by this one particular flavor of call. So it probably isn't worth two bytes of cache space.No perf impact.