Skip to content

Conversation

@Fidget-Spinner
Copy link
Member

@Fidget-Spinner Fidget-Spinner commented Dec 29, 2025

#143187 caused a pretty serious regression in JIT performance, mainly because we stop the trace prior to the ENTER_EXECUTOR. This PR restores the old behavior, stopping the trace at the ENTER_EXECUTOR, allowing it to link up to the executor.

The perf regression can be clearly seen here (going up means things got slower):

image

@Fidget-Spinner
Copy link
Member Author

Before:
graphviz (34)

After (notice how more things are linked):

graphviz (35)

Copy link
Contributor

@diegorusso diegorusso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that #143187 add a test. Can we have it here as well?

@Fidget-Spinner
Copy link
Member Author

I fixed the test in the test suite so that it catches it. I verified that without the change, the test fails. With the change, the test passes.

@Fidget-Spinner
Copy link
Member Author

Also verified that:

At commit 713684d (before my previous PR), this test passes.
At commit daa9aa4 (the first commit with my PR), this test fails.
At commit a9548b8 (current PR commit), this test passes.

So this does indeed restore the old behavior!

@Fidget-Spinner
Copy link
Member Author

For the following code:

TIER2_THRESHOLD = 4001
def f():
    for x in range(TIER2_THRESHOLD + 3):
        for y in range(TIER2_THRESHOLD + 3):
            z = x + y

import sys
f()
sys._dump_tracelets("hello.gvz")

Current main (buggy):

graphviz (36)

This PR (notice how the traces now link up):

graphviz (37)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants