Repository navigation
gh-119476: Eliminate Unnecessary Function Version Checks in Tier Two Interpreter - #119491
saulshanabrook wants to merge 7 commits into
Conversation
|
Hey @brandtbucher, I am trying to add a test case for this but am a bit confused as to why it's not working currently. With this test, it doesn't eliminate the second def thing(f):
x = 0
for _ in range(100):
x += f() + f()
return x
# Specify an unused arg in the function so that
# CALL_PY_GENERAL is used over CALL_PY_EXACT_ARGS
res, ex = self._run_with_optimizer(thing, lambda _=1: 1)What's confusing me is that the trace never seems to get to the second one to optimize. It seems like it quits before even getting there. If I like at the opts returned, I see it does include it twice: However, the optimize bytecode case for Is it stopping the trace after one function call? Even if I turn off the watcher invalidating the trace whenever the function version changes, it still behaves like this. |
|
What happens if we use a global function instead of a local lambda? I’m not at my computer, but it looks to me like the optimizer is unable to continue projecting into the first call, and is stopping there… |
|
If I do something like this (or if I define the function inside the other function) I get similar behavior: def f(*args):
return 1
def thing(init):
x = init
for _ in range(100):
x += f() + f()
return x
res, ex = self._run_with_optimizer(thing, 0) |
|
I’ll take a look. |
|
It looks like |
|
I'm sure it's not "correct", but copying-and-pasting the body of |
|
I can try adding a test using the exact args version once #119510 is merged... Maybe that will work? |
Yup! That PR just landed a minute ago. |
|
@brandtbucher Could you run the benchmarks on this? I updated the tests to use the new opcode and it seems to be working. |
Fidget-Spinner
left a comment
Fidget-Spinner
left a comment
There was a problem hiding this comment.
We definitely want this change. It will reduce deopt points in tier 2 when the mythical partial evaluation pass is added in.
Sorry for dropping the ball on this!
|
This PR is stale because it has been open for 30 days with no activity. |
3 participants
This PR removes duplicate
_CHECK_FUNCTIONuops in the tier two interpreter. It does this in a similar way to removing the_GUARD_TYPE_VERSIONin #119365.It stores the current function version we have checked against in the symbol and then when we want to check again if the function is the same version, we remove the check, iff the function version hasn't changed since then. We know if it has changed by adding a watcher that will trigger whenever a function version changes.
Note that
_CHECK_FUNCTIONis only produced byCALL_PY_GENERALnot byCALL_PY_EXACT_ARGS.TODO: