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-126255: Ignore warning about JIT being deactivated when perf support is active in `test_embed.InitConfigTests.test_initconfig_api` by mpage · Pull Request #126302 · python/cpython

Conversation

@mpage

@mpage mpage commented Nov 1, 2024 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

This is a temporary measure to get JIT tests passing again while other folks work on a more comprehensive solution.

  • Issue: test_embed.InitConfigTests.test_initconfig_api fails when cpython is built with --enable-experimental-jit=interpreter #126255

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@mpage
Ignore warning about JIT being deactivated when perf support is activ…
6b3584a 
…e in test_initconfig_api

This is a temporary measure to get JIT tests passing again. This should be reverted once a
more comprehensive fix lands.
@bedevere-app bedevere-app Bot added the tests Tests in the Lib/test dir label Nov 1, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Nov 1, 2024
test_embed.InitConfigTests.test_initconfig_api fails when cpython is built with --enable-experimental-jit=interpreter #126255
Closed
@mpage mpage added the skip news label Nov 1, 2024
@mpage
mpage requested review from Eclips4, Fidget-Spinner and pablogsal November 1, 2024 17:00
@mpage

mpage commented Nov 1, 2024

Copy link
Copy Markdown
Contributor Author

@Fidget-Spinner - Is there a way I can trigger the JIT tests on this?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mpage mpage mentioned this pull request Nov 1, 2024
gh-124855: Fix jit perf (and lejit) #126166
Closed
@Eclips4

Eclips4 commented Nov 1, 2024 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

@Fidget-Spinner - Is there a way I can trigger the JIT tests on this?

Unfortunately there's no way to trigger the JIT tests. The only way to do this - add change to JIT-related files (for example a newline)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Eclips4
Add a newline to optimizer.c to trigger JIT workflow
47a81da
@Eclips4

Eclips4 commented Nov 1, 2024

Copy link
Copy Markdown
Member

It seems that we still need a couple of changes from #126166 to solve this
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Eclips4 @Fidget-Spinner @pablogsal
Necessary changes made by Pablo and Ken
8d80f44 
Co-authored-by: Ken Jin <[email protected]>
Co-authored-by: Pablo Galindo <[email protected]>
@Eclips4

Eclips4 commented Nov 1, 2024

Copy link
Copy Markdown
Member

I'm pushing this forward because I want to see JIT CI green 🙂
mpage reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Eclips4
Fix linting issue
cb19fc2
Eclips4
Eclips4 reviewed Nov 1, 2024
View reviewed changes
Comment thread Lib/test/test_embed.py Outdated
'perf_profiling': 2,
}
config_dev_mode(preconfig, config)
using_jit = any(x.startswith("JIT") for x in get_build_info())

@Eclips4 Eclips4 Nov 1, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Unfortunately this won't work for --enable-experimental-jit=interpreter since it doesn't raise a warning about perf profiling support (because there's no conflict with tier 2 and perf profiler)

Sorry, something went wrong.

Uh oh!

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

@Eclips4 Eclips4 Nov 1, 2024 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

...and get_build_info probably does not work on Windows

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Eclips4
Ignore stderr
79c966c
@Eclips4
Eclips4 force-pushed the gh-126255-temp-workaround-embed-test-failure branch from f122803 to 79c966c Compare November 1, 2024 22:19
brandtbucher
brandtbucher approved these changes Nov 1, 2024
View reviewed changes

@brandtbucher brandtbucher left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Looks good, thanks for the fix. Can you maybe just add a comment to the pylifcycle.c change to explain that this is because perf works fine with the tier two interpreter, so we just need to check on "real" JIT builds?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Eclips4 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@bedevere-app bedevere-app Bot added the awaiting merge label Nov 1, 2024
@Eclips4
Add a comment about tier 2 interpreter
c6462c4
@Eclips4
Eclips4 marked this pull request as ready for review November 1, 2024 22:47
@Eclips4
Eclips4 requested a review from ericsnowcurrently as a code owner November 1, 2024 22:47
@bedevere-app bedevere-app Bot added awaiting core review and removed awaiting merge labels Nov 1, 2024
@Eclips4
Eclips4 enabled auto-merge (squash) November 1, 2024 22:50
@Eclips4
Eclips4 disabled auto-merge November 1, 2024 22:50
@Eclips4
Eclips4 enabled auto-merge (squash) November 1, 2024 22:51
@Eclips4
Eclips4 merged commit f0c6fcc into python:main Nov 1, 2024
@bedevere-app bedevere-app Bot removed the awaiting core review label Nov 1, 2024
picnixz pushed a commit to picnixz/cpython that referenced this pull request Dec 8, 2024
@mpage @Eclips4
@Fidget-Spinner @pablogsal @picnixz
pythongh-126255: Ignore warning about JIT being deactivated when perf…
0aa2f83 
… support is active in `test_embed.InitConfigTests.test_initconfig_api` (python#126302)

Temporarily ignore warnings about JIT deactivation when perf support is active. 
This will be reverted as soon as a way is found to determine at run time whether the interpreter was built with JIT. Currently, this is not possible on Windows.


Co-authored-by: Kirill Podoprigora <[email protected]>
Co-authored-by: Ken Jin <[email protected]>
Co-authored-by: Pablo Galindo <[email protected]>
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
@mpage @Eclips4
@Fidget-Spinner @pablogsal @ebonnal
pythongh-126255: Ignore warning about JIT being deactivated when perf…
1c88e3e 
… support is active in `test_embed.InitConfigTests.test_initconfig_api` (python#126302)

Temporarily ignore warnings about JIT deactivation when perf support is active. 
This will be reverted as soon as a way is found to determine at run time whether the interpreter was built with JIT. Currently, this is not possible on Windows.


Co-authored-by: Kirill Podoprigora <[email protected]>
Co-authored-by: Ken Jin <[email protected]>
Co-authored-by: Pablo Galindo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Eclips4 Eclips4 Eclips4 left review comments
@brandtbucher brandtbucher brandtbucher approved these changes
@pablogsal pablogsal Awaiting requested review from pablogsal
@Fidget-Spinner Fidget-Spinner Awaiting requested review from Fidget-Spinner
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner

Uh oh!

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

Assignees

No one assigned

Labels

skip news tests Tests in the Lib/test dir

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

@mpage @Eclips4 @brandtbucher
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.