GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-124613: Deactivate perf support in tests if the jit is set by pablogsal · Pull Request #124794 · python/cpython

Conversation

@pablogsal

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

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Issue: test_perf_profiler fails on Linux (both AArch64 and x86) #124613

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@pablogsal
pablogsal requested review from corona10 and erlend-aasland as code owners September 30, 2024 17:25
@bedevere-app bedevere-app Bot added the awaiting core review label Sep 30, 2024
@pablogsal pablogsal changed the title Deactivate perf support if the jit is set gh-124613: Deactivate perf support if the jit is set Sep 30, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Sep 30, 2024
test_perf_profiler fails on Linux (both AArch64 and x86) #124613
Closed
@pablogsal

pablogsal commented Sep 30, 2024

Copy link
Copy Markdown
Member Author

CC @brandtbucher @diegorusso
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@pablogsal

pablogsal commented Sep 30, 2024

Copy link
Copy Markdown
Member Author

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?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@brandtbucher

brandtbucher commented Sep 30, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@brandtbucher

brandtbucher commented Sep 30, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@brandtbucher brandtbucher mentioned this pull request Sep 30, 2024
gh-124613: Don't run perf tests in JIT builds #124792
Merged
@pablogsal

pablogsal commented Sep 30, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@brandtbucher

brandtbucher commented Oct 1, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@pablogsal
pablogsal force-pushed the gh-124613-2 branch from 3f4fabd to 036c09b Compare October 1, 2024 00:01
@pablogsal

pablogsal commented Oct 1, 2024

Copy link
Copy Markdown
Member Author

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@pablogsal
pablogsal force-pushed the gh-124613-2 branch from 036c09b to 9717e7a Compare October 1, 2024 00:13
@pablogsal

pablogsal commented Oct 1, 2024

Copy link
Copy Markdown
Member Author

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal commented Oct 1, 2024
View reviewed changes
Comment thread Lib/test/test_perf_profiler.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal commented Oct 1, 2024
View reviewed changes
Comment thread Lib/test/test_perf_profiler.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal commented Oct 1, 2024
View reviewed changes
Comment thread Lib/test/test_perf_profiler.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@pablogsal
pablogsal force-pushed the gh-124613-2 branch 2 times, most recently from 2a22218 to e5fc033 Compare October 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 thread Lib/test/test_perf_profiler.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@pablogsal
pythongh-124613: Deactivate the JIT during perf tests
fdd1068 
Signed-off-by: Pablo Galindo <[email protected]>
@pablogsal
pablogsal force-pushed the gh-124613-2 branch from e5fc033 to fdd1068 Compare October 1, 2024 16:13
@pablogsal pablogsal added the skip issue label Oct 1, 2024
@pablogsal pablogsal changed the title gh-124613: Deactivate perf support if the jit is set gh-124613: Deactivate perf support in tests if the jit is set Oct 1, 2024
@pablogsal pablogsal removed the skip issue label Oct 1, 2024
@pablogsal pablogsal added the skip news label Oct 1, 2024
@pablogsal
pablogsal requested a review from brandtbucher October 1, 2024 16:36
brandtbucher
brandtbucher approved these changes Oct 3, 2024
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Oct 3, 2024
@pablogsal
pablogsal merged commit 5e9e506 into python:main Oct 4, 2024
@pablogsal
pablogsal deleted the gh-124613-2 branch October 4, 2024 00:00
@bedevere-app bedevere-app Bot removed the awaiting merge label Oct 4, 2024
@diegorusso diegorusso added the needs backport to 3.13 only security fixes label Apr 7, 2026
@miss-islington-app

miss-islington-app Bot commented Apr 7, 2026

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app

bedevere-app Bot commented Apr 7, 2026

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only 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 <[email protected]>
Signed-off-by: Pablo Galindo <[email protected]>
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 <[email protected]>
Co-authored-by: Pablo Galindo Salgado <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@diegorusso diegorusso diegorusso left review comments
@brandtbucher brandtbucher brandtbucher approved these changes
@corona10 corona10 Awaiting requested review from corona10

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@pablogsal @brandtbucher @diegorusso
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.