Skip to content

gh-124613: Deactivate perf support in tests if the jit is set - #124794

Merged
pablogsal merged 1 commit into
python:mainfrom
pablogsal:gh-124613-2
Oct 4, 2024
Merged

gh-124613: Deactivate perf support in tests if the jit is set#124794
pablogsal merged 1 commit into
python:mainfrom
pablogsal:gh-124613-2

Conversation

@pablogsal

@pablogsalpablogsal commented Sep 30, 2024•
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

@pablogsalpablogsal changed the title Deactivate perf support if the jit is setgh-124613: Deactivate perf support if the jit is setSep 30, 2024
@bedevere-appbedevere-appBot mentioned this pull request Sep 30, 2024
Closed
@pablogsal

pablogsal commented Sep 30, 2024

Copy link
Copy Markdown
MemberAuthor

CC @brandtbucher@diegorusso

@pablogsal

pablogsal commented Sep 30, 2024

Copy link
Copy Markdown
MemberAuthor

Hummm if we are going to make the JIT the default maybe we should only deactivate in YES and not in Yes-off @brandtbucher WDYT?

@brandtbucher

brandtbucher commented Sep 30, 2024

Copy link
Copy Markdown
Member

Hummm if we are going to make the JIT the default maybe we should only deactivate in YES and not in Yes-off @brandtbucher WDYT?

I mean, a yes-off build can still have a JIT if the env var is set (ditto for unsetting it via an env var on yes builds). We really just want to deactivate the JIT, regardless of which mode it was built in or what env vars are set.

Check out test.support.without_optimizer, I think it's probably what you want here.

@brandtbucher

brandtbucher commented Sep 30, 2024•
edited
Loading

Copy link
Copy Markdown
Member

Oh, whoops, wrong PR. I thought this was the one that's skipping perf tests.

@brandtbucher

brandtbucher commented Sep 30, 2024•
edited
Loading

Copy link
Copy Markdown
Member

Yeah, we could just say that both yes and yes-off disable perf jit support. Your call... but yeah, that means we can't ship perf JIT support in the same build if we do that.

Perhaps we should just disable the JIT if perf support is activated at runtime? It requires a bit more surgery, but I think it's probably the best option.

@brandtbucherbrandtbucher mentioned this pull request Sep 30, 2024
Merged
@pablogsal

pablogsal commented Sep 30, 2024•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Yeah, we could just say that both yes and yes-off disable perf jit support. Your call... but yeah, that means we can't ship perf JIT support in the same build if we do that.

No, we surely want perf in yes-off.

Perhaps we should just disable the JIT if perf support is activated at runtime? It requires a bit more surgery, but I think it's probably the best option.

Yeah we should use both things I think. Currently it's not switched to yes-off no?

@brandtbucher

brandtbucher commented Oct 1, 2024•
edited
Loading

Copy link
Copy Markdown
Member

Currently it's not switched to yes-off no?

That's too many negatives for me to follow, haha.

What are you proposing, exactly? Disable the JIT when perf support is turned on at runtime? I believe this needs to cover PYTHON_PERF_JIT_SUPPORT, PYTHONPERFSUPPORT, -X perf, and -X perf_jit at startup, and sys.activate_stack_trampoline() and sys.deactivate_stack_trampoline() at runtime. Seems reasonable (maybe we warn if the JIT was activated with the env var to let the user know it's getting turned off?).

Also, the name PYTHON_PERF_JIT_SUPPORT is now more confusing than ever, haha.

@pablogsal

pablogsal commented Oct 1, 2024

Copy link
Copy Markdown
MemberAuthor

What are you proposing, exactly? Disable the JIT when perf support is turned on at runtime?

Yes, but that in a separate PR

For now I have modified this PR to use the test.support.without_optimizer so this always runs without the JIT. Is there a way to do that with an env var instead? That way I can call the subprocess without the JIT always

@brandtbucher

brandtbucher commented Oct 1, 2024

Copy link
Copy Markdown
Member

For now I have modified this PR to use the test.support.without_optimizer so this always runs without the JIT. Is there a way to do that with an env var instead? That way I can call the subprocess without the JIT always

PYTHON_JIT=0

@pablogsal

pablogsal commented Oct 1, 2024

Copy link
Copy Markdown
MemberAuthor

For now I have modified this PR to use the test.support.without_optimizer so this always runs without the JIT. Is there a way to do that with an env var instead? That way I can call the subprocess without the JIT always

PYTHON_JIT=0

Ok, moved this PR to that

pablogsal
pablogsal commented Oct 1, 2024
View reviewed changes
Comment threadLib/test/test_perf_profiler.py Outdated
pablogsal
pablogsal commented Oct 1, 2024
View reviewed changes
Comment threadLib/test/test_perf_profiler.py Outdated
pablogsal
pablogsal commented Oct 1, 2024
View reviewed changes
Comment threadLib/test/test_perf_profiler.py Outdated
@pablogsal
pablogsalforce-pushed the gh-124613-2 branch 2 times, most recently from 2a22218 to e5fc033CompareOctober 1, 2024 00:28
@erlend-aasland
erlend-aasland removed their request for review October 1, 2024 06:38
diegorusso
diegorusso reviewed Oct 1, 2024
View reviewed changes
Comment threadLib/test/test_perf_profiler.py Outdated
@pablogsal
pythongh-124613: Deactivate the JIT during perf tests
fdd1068
Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
@pablogsalpablogsal changed the title gh-124613: Deactivate perf support if the jit is setgh-124613: Deactivate perf support in tests if the jit is setOct 1, 2024
@pablogsal
pablogsal merged commit 5e9e506 into python:mainOct 4, 2024
@pablogsal
pablogsal deleted the gh-124613-2 branch October 4, 2024 00:00
@diegorussodiegorusso added the needs backport to 3.13 bugs and security fixes label Apr 7, 2026
@miss-islington-app

miss-islington-appBot commented Apr 7, 2026

Copy link
Copy Markdown

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@bedevere-app

bedevere-appBot commented Apr 7, 2026

Copy link
Copy Markdown

GH-148214 is a backport of this pull request to the 3.13 branch.

@bedevere-appbedevere-appBot removed the needs backport to 3.13 bugs and security fixes label Apr 7, 2026
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 7, 2026
@pablogsal@miss-islington
pythongh-124613: Deactivate perf support in tests if the jit is set (p…
08eaa0b
…ythonGH-124794)
pythongh-124613: Deactivate the JIT during perf tests
(cherry picked from commit 5e9e506)
Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com>
Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
Fidget-Spinner pushed a commit that referenced this pull request Apr 7, 2026
@miss-islington@pablogsal
[3.13]gh-124613: Deactivate perf support in tests if the jit is set (G…
9f15d25
…H-124794) (GH-148214)
gh-124613: Deactivate perf support in tests if the jit is set (GH-124794)
gh-124613: Deactivate the JIT during perf tests
(cherry picked from commit 5e9e506)
Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
Co-authored-by: Pablo Galindo Salgado <Pablogsal@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@diegorussodiegorussodiegorusso left review comments
@brandtbucherbrandtbucherbrandtbucher approved these changes
@corona10corona10Awaiting requested review from corona10

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@pablogsal@brandtbucher@diegorusso