Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-126255: Ignore warning about JIT being deactivated when perf support is active in test_embed.InitConfigTests.test_initconfig_api #126302

Merged

Conversation

mpage
Copy link
Contributor

@mpage mpage commented Nov 1, 2024

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

…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.
@mpage
Copy link
Contributor Author

mpage commented Nov 1, 2024

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

@Eclips4
Copy link
Member

Eclips4 commented Nov 1, 2024

@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)

@Eclips4
Copy link
Member

Eclips4 commented Nov 1, 2024

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

Co-authored-by: Ken Jin <[email protected]>
Co-authored-by: Pablo Galindo <[email protected]>
@Eclips4
Copy link
Member

Eclips4 commented Nov 1, 2024

I'm pushing this forward because I want to see JIT CI green 🙂

@@ -1780,8 +1781,13 @@ def test_initconfig_api(self):
'perf_profiling': 2,
}
config_dev_mode(preconfig, config)
using_jit = any(x.startswith("JIT") for x in get_build_info())
Copy link
Member

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)

Copy link
Member

@Eclips4 Eclips4 Nov 1, 2024

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

@Eclips4 Eclips4 force-pushed the gh-126255-temp-workaround-embed-test-failure branch from f122803 to 79c966c Compare November 1, 2024 22:19
Copy link
Member

@brandtbucher brandtbucher left a 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?

@Eclips4 Eclips4 marked this pull request as ready for review November 1, 2024 22:47
@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
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants