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-124855: Fix jit perf (and lejit) #126166

Closed
wants to merge 8 commits into from

Conversation

Fidget-Spinner
Copy link
Member

@Fidget-Spinner Fidget-Spinner commented Oct 30, 2024

JIT builds are currently failing on main. This supersedes #126163. This wasn't caught because we don't run the JIT tests on pylifecycle.c

@pablogsal
Copy link
Member

I think we need to add skips to the tests in the Perf trampoline when the jit is activated

@pablogsal
Copy link
Member

I have pushed a bunch of fixes to ensure we run the tests that check for anything perf without the JIT as we do in other places

@@ -1768,6 +1768,8 @@ def test_init_set_config(self):
self.check_all_configs("test_init_set_config", config,
api=API_ISOLATED)

@unittest.skipIf("-D_Py_JIT" in (sysconfig.get_config_var('PY_CORE_CFLAGS') or ''),
Copy link
Member

Choose a reason for hiding this comment

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

This fails on windows for some reason

Copy link
Member

Choose a reason for hiding this comment

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

I think with my fix we can actually run the test so I am switching to that

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm I'm abit stumped. No clue why your fix isn't working :(.

Copy link
Member

Choose a reason for hiding this comment

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

It's because the test is running on isolated mode and therefore it ignores the environment flags

Copy link
Member

Choose a reason for hiding this comment

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

I pushed a fix that should do the trick

Signed-off-by: Pablo Galindo <[email protected]>
@Fidget-Spinner
Copy link
Member Author

Still failing on Windows... not sure why

@Eclips4
Copy link
Member

Eclips4 commented Oct 30, 2024

Okay, I finally set up my Windows environment and it seems that there is no PY_CORE_CFLAGS. Perhaps we can't currently determine if JIT is enabled on non-configure builds. Maybe it's time to add a PY_JIT variable to pyconfig.h?

@pablogsal
Copy link
Member

pablogsal commented Oct 31, 2024

Maybe we can land a version of this that just ignores warning lines in stderr and then undo that hack when we have JIT detection on windows.

This way we can move on and avoid having to revert

@mpage
Copy link
Contributor

mpage commented Nov 1, 2024

Maybe we can land a version of this that just ignores warning lines in stderr and then undo that hack when we have JIT detection on windows.

This way we can move on and avoid having to revert

I put up #126302 as a possible work-around.

@Eclips4
Copy link
Member

Eclips4 commented Nov 2, 2024

Closing it due to merge of #126302. Thank you Pablo and Ken.

@Eclips4 Eclips4 closed this Nov 2, 2024
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.

4 participants