Skip to content

Cut short extension load cycles during precompilation#53112

Closed
IanButterworth wants to merge 3 commits intoJuliaLang:masterfrom
IanButterworth:ib/ext_precomp_cycle
Closed

Cut short extension load cycles during precompilation#53112
IanButterworth wants to merge 3 commits intoJuliaLang:masterfrom
IanButterworth:ib/ext_precomp_cycle

Conversation

@IanButterworth
Copy link
Copy Markdown
Member

@IanButterworth IanButterworth commented Jan 30, 2024 •
edited
Loading

Depends on

Builds on #53111
Fixes #53081

Cuts the cycle short and highlights when two or more extensions are each triggered during the precompilation of eachother, which causes a dependency cycle in nested precompilation that was seen in #53081.

Note if this is considered safe, the @warns could just be @debugs and the user doesn't need to be notified.

i.e. on master this confusing error occurs, and it's actually coming from a double nested precompile worker precompiling SymbolicsForwardDiffExt but trying to load SymbolicsPreallocationToolsExt which the parent worker is in the process of precompiling

(@v1.11) pkg> precompile
Precompiling project...
  103 dependencies successfully precompiled in 34 seconds. 40 already precompiled.
  1 dependency had output during precompilation:
┌ Symbolics → SymbolicsPreallocationToolsExt
│  ┌ Warning: Module SymbolicsPreallocationToolsExt with build ID ffffffff-ffff-ffff-0008-f643010bcdb7 is missing from the cache.
│  │ This may mean SymbolicsPreallocationToolsExt [d479e226-fb54-5ebe-a75e-a7af7f39127f] does not support precompilation but is imported by a module that does.
│  └ @ Base loading.jl:2121
│  ┌ Error: Error during loading of extension SymbolicsPreallocationToolsExt of Symbolics, use `Base.retry_load_extensions()` to retry.

With this PR

(@v1.11) pkg> precompile
Precompiling project...
  103 dependencies successfully precompiled in 36 seconds. 40 already precompiled.
  2 dependencies had output during precompilation:
┌ FillArrays → FillArraysPDMatsExt
│  ┌ Warning: Dependency cycle detected in extension precompilation: FillArraysPDMatsExt > FillArraysSparseArraysExt > FillArraysPDMatsExt
│  └ @ Base loading.jl:1396
└  
┌ Symbolics → SymbolicsPreallocationToolsExt
│  ┌ Warning: Dependency cycle detected in extension precompilation: SymbolicsPreallocationToolsExt > SymbolicsForwardDiffExt > SymbolicsPreallocationToolsExt
│  └ @ Base loading.jl:1396
└

The cycle issue being this strong dependency branch

Symbolics > SymbolicUtils > LabelledArrays > PreallocationTools > ForwardDiff

triggers both of these Symbolics extensions without the need for any further packages present.

[extensions]
SymbolicsForwardDiffExt = "ForwardDiff"
SymbolicsPreallocationToolsExt = ["ForwardDiff", "PreallocationTools"]

So they both get triggered during the precompilation job of the other one.

Note that FillArraysPDMatsExt doesn't error on master like SymbolicsPreallocationToolsExt does. Not sure why not.

cc. @ChrisRackauckas

@KristofferC KristofferC added this to the 1.11 milestone Feb 7, 2024
@KristofferC
Copy link
Copy Markdown
Member

KristofferC commented Feb 7, 2024

This seems better than the status quo so we should try it out imo.

@oscardssmith
Copy link
Copy Markdown
Member

oscardssmith commented Feb 7, 2024

Should this be backported to 1.10?

@IanButterworth IanButterworth mentioned this pull request Feb 8, 2024
Merged
@IanButterworth
Copy link
Copy Markdown
Member Author

IanButterworth commented Feb 9, 2024

@KristofferC I've made this a @debug, so will silently work now, as I don't think there's much to teach the user about what not to do?

@KristofferC
Copy link
Copy Markdown
Member

KristofferC commented Feb 9, 2024

I guess we should add a test.

Should be enough to generate A, B, C, D make, extension AExtC, AExtD, and make A depend on B and make B depend on C, D?

KristofferC
KristofferC reviewed Feb 9, 2024
View reviewed changes
@kpobrien kpobrien mentioned this pull request Feb 11, 2024
Closed
@KristofferC KristofferC mentioned this pull request Mar 7, 2024
Open
@IanButterworth IanButterworth mentioned this pull request Mar 28, 2024
Closed
@IanButterworth
Copy link
Copy Markdown
Member Author

IanButterworth commented May 23, 2024

Not needed as we now skip cycles in extensions by not precompiling those extensions until loading time (where it somehow works) re. #53972

cc. @KristofferC

@IanButterworth IanButterworth deleted the ib/ext_precomp_cycle branch May 23, 2024 02:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@KristofferC KristofferC Awaiting requested review from KristofferC

Assignees

No one assigned

Labels

package extensions

Projects

None yet

Milestone

1.11

Development

Successfully merging this pull request may close these issues.

Error precompiling SymbolicsPreallocationToolsExt

3 participants

@IanButterworth @KristofferC @oscardssmith