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

delay loading of extensions as much as possible #48674

Merged
merged 3 commits into from
Mar 16, 2023
Merged

Conversation

KristofferC
Copy link
Member

(Diff best viewed with whitespace ignored).

To reduce the likelihood of cycles, we delay loading extensions until all packages have finished loading during a require call and we are ready to return to Main.

Fixes #48533

@KristofferC KristofferC added packages Package management and loading backport 1.9 Change should be backported to release-1.9 labels Feb 14, 2023
base/loading.jl Outdated Show resolved Hide resolved
@KristofferC KristofferC mentioned this pull request Feb 20, 2023
50 tasks
@KristofferC KristofferC mentioned this pull request Mar 7, 2023
52 tasks
base/loading.jl Outdated
Comment on lines 1208 to 1209
for pkgid in copy(keys(EXT_DORMITORY))
haskey(Base.loaded_modules, pkgid) || continue
Copy link
Member

Choose a reason for hiding this comment

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

would this be any better?

Suggested change
for pkgid in copy(keys(EXT_DORMITORY))
haskey(Base.loaded_modules, pkgid) || continue
for pkgid in intersect(keys(Base.loaded_modules), keys(Base.EXT_DORMITORY))

I also wonder if there is any meaningful way to sort this list to be predictable (if that is worthwhile to do), for example we could try to sort the result, but isless(::Base.PkgId, ::Base.PkgId) is not defined.

Copy link
Member Author

Choose a reason for hiding this comment

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

I did this and also sorted by UUID.

Copy link
Member

Choose a reason for hiding this comment

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

I hope folks don't decide to give their packages low or high uuids to hack trigger order.

Copy link
Member Author

Choose a reason for hiding this comment

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

Doesn't really matter, this is just to keep it consistent.

@KristofferC KristofferC merged commit e3043a8 into master Mar 16, 2023
@KristofferC KristofferC deleted the kc/ext_loading_2 branch March 16, 2023 14:50
oscardssmith pushed a commit to oscardssmith/julia that referenced this pull request Mar 20, 2023
KristofferC added a commit that referenced this pull request Mar 24, 2023
@KristofferC KristofferC removed the backport 1.9 Change should be backported to release-1.9 label Mar 31, 2023
KristofferC pushed a commit that referenced this pull request Apr 5, 2023
KristofferC pushed a commit that referenced this pull request Apr 5, 2023
KristofferC pushed a commit that referenced this pull request Apr 9, 2023
KristofferC pushed a commit that referenced this pull request Apr 9, 2023
KristofferC pushed a commit that referenced this pull request Apr 9, 2023
KristofferC added a commit that referenced this pull request Apr 9, 2023
Revert "delay loading of extensions as much as possible (#48674)"
KristofferC pushed a commit that referenced this pull request Apr 9, 2023
Xnartharax pushed a commit to Xnartharax/julia that referenced this pull request Apr 19, 2023
Xnartharax pushed a commit to Xnartharax/julia that referenced this pull request Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages Package management and loading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cycle creation issue with new weakdeps code
3 participants