Skip to content

Refactor julia initialization by finding and loading the sysimage earlier #58231

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

Merged
merged 5 commits into from
May 2, 2025

Conversation

gbaraldi
Copy link
Member

This changes the order of initialization slightly. The goal is to be able to initialize executables and share libraries without having to explicitly call init functions.

This still doesn't address the thread safety

@gbaraldi gbaraldi requested a review from topolarity April 25, 2025 17:35
jl_gc_notify_image_load(sysimage.data, sysimage.size);

if (jl_options.cpu_target == NULL)
jl_options.cpu_target = "native";
Copy link
Member

Choose a reason for hiding this comment

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

Should this be the default value in jloptions.c?

@topolarity
Copy link
Member

Is this enough to fix --trim executables (c.f. #58197)?

@@ -461,9 +461,7 @@ JL_DLLEXPORT jl_gcframe_t **jl_autoinit_and_adopt_thread(void)
" (this should not happen, please file a bug report)\n");
exit(1);
}
const char *image_path = jl_pathname_for_handle(handle);
jl_enter_threaded_region(); // This should maybe be behind a lock, but it's harmless if done twice
Copy link
Member

Choose a reason for hiding this comment

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

Was this deletion intentional?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. It got moved into the jl_init_with_handle call (the threaded region maybe should still be here :\ but I dunno)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah it was specifically the thread region I was curious about - I'm OK with deleting it for now, we can re-visit soon with thread-safety

@topolarity topolarity marked this pull request as ready for review April 29, 2025 18:56
gbaraldi and others added 4 commits April 29, 2025 19:51
This makes it clearer that sysimage resolution and `jl_options`
setup are the responsibility of `jl_api.c`, not the initialization
system.
@topolarity topolarity added the backport 1.12 Change should be backported to release-1.12 label Apr 30, 2025
@topolarity
Copy link
Member

I cannot abide CI any longer. I'm going to merge this as-is (it has passed 80% of CI several times now)

Pkg server issues have been worked around, but 5-10+ manual Windows restarts per run, random failures, and cmdlineargs-related failures (see https://buildkite.com/julialang/julia-master/builds/47119#01968dd3-efa0-414c-8aae-c2592c8a46cf/902-1488 and #58172 (comment)) make it almost impossible to get a clean run through.

@topolarity topolarity merged commit 6495270 into master May 2, 2025
5 of 7 checks passed
@topolarity topolarity deleted the gb/refactor2 branch May 2, 2025 00:33
@topolarity
Copy link
Member

Thanks @gbaraldi !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.12 Change should be backported to release-1.12
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants