gh-137838: Fix JIT trace buffer overrun by pre-reserving exit stub space #138173
Conversation
|
All commit authors signed the Contributor License Agreement. |
corona10
force-pushed
the
gh-137800-poc
branch
from
77e04ea to
214be19
Compare
corona10
changed the title
corona10
changed the title
corona10
force-pushed
the
gh-137800-poc
branch
from
214be19 to
0776a59
Compare
Python/optimizer.c
Outdated
| // Leave space for possible trailing _EXIT_TRACE | ||
| int max_length = buffer_size-2; | ||
| // Leave space for possible trailing _EXIT_TRACE and estimated exit stubs | ||
| // Reserve 20% of buffer space for exit stubs (empirically sufficient) |
There was a problem hiding this comment.
Can we have a tighter and more logical bound than this?
There was a problem hiding this comment.
10% is too small. Let me test from 15%.
There was a problem hiding this comment.
No I mean something based on math. Like a loose bound would be:
Every instruction needs a _DEOPT or _EXIT_TRACE
_DEOPT might have _ERROR_POP_N before it
Therefore from n instructions we must reserve 2n instructions.
This bound is overly loose, but it would be good to work it out
There was a problem hiding this comment.
Hmm, let me think about this differently. I’ll share the next commit with an improvement that tracks each instruction based on a mathematical assumption
|
Hi @Fidget-Spinner, I adjusted the calculation of max_length to take a more conservative approach. On my local setup, both of the following tests passed:
I’m not entirely sure if this approach is ideal from the JIT maintainers’ perspective, so I’d appreciate it if you could take a look before we ask them for a review. |
…g exit stub space" This reverts commit 0776a59.
|
Hmm, not sure if this 20% magic number is the correct way.. Let me take a look other approach that calculate max_length properly. |
2 participants
Uh oh!
There was an error while loading. Please reload this page.