GH-118093: Handle some polymorphism before requiring progress in tier two#122843
Uh oh!
There was an error while loading. Please reload this page.
Conversation
brandtbucher added performance 
Include/internal/pycore_optimizer.h Outdated
| _PyBloomFilter bloom; | ||
| _PyExecutorLinkListNode links; | ||
| PyCodeObject *code; // Weak (NULL if no corresponding ENTER_EXECUTOR). | ||
| int chain_depth; |
There was a problem hiding this comment.
Since this is only 2 bits of information, and valid and linked are only 1 bit each, could we pack them together?
There was a problem hiding this comment.
I packed these three into bitfields of a uint16_t.
Python/optimizer.c Outdated
| _Py_CODEUNIT *instr, | ||
| _PyExecutorObject **exec, | ||
| int Py_UNUSED(stack_entries)) | ||
| int Py_UNUSED(stack_entries), bool Py_UNUSED(progress_needed)) |
There was a problem hiding this comment.
Put progress_needed on its own line?
There was a problem hiding this comment.
Fixed here and elsewhere.
Python/optimizer.c Outdated
| _PyStackRef *stack_pointer, _PyExecutorObject **executor_ptr) | ||
| _PyStackRef *stack_pointer, _PyExecutorObject **executor_ptr, int chain_depth) | ||
| { | ||
| // The first instruction in a chain and the MAX_CHAIN_DEPTH'th instruction |
There was a problem hiding this comment.
Do you mean "the first instruction in the last executor in a chain"?
I read this as meaning that the first and fourth instructions must make progress.
There was a problem hiding this comment.
I replaced both "instruction"s with "executor"s.
| goto done; | ||
| } | ||
| } | ||
| if (opcode == ENTER_EXECUTOR){ |
There was a problem hiding this comment.
Why are we are done?
If we are wanting specialized traces, they may overlap.
There was a problem hiding this comment.
I've added a comment justifying the decision.
2 participants
Despite our best intentions, we currently don't handle polymorphism at all in tier two, since our current forward progress requirement means that we need to deopt the first instruction of every new trace.
This changes our trace stitching for side exits to not require progress until a certain tree depth is reached (currently four). It also changes the optimizer to rejoin with any traces encountered during projection, since this does a better job of keeping us on trace in loops (we can take four side exits per iteration before requiring progress) and keeps the amount of tier two code from exploding.
As a simple microbenchmark, this goes from ~10% slower when run with the JIT enabled to ~25% faster after this change.
Overall, the benchmarks are 0.6% faster, including nice improvements on all of our interpreter-heavy benchmarks. The stats show that we're actually executing more traces and a bit (~1%) more tier one code now, but that's expected since side exit chains require up to 4x as much "warming up" as before.