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

Avoid depending on LazyArraysBandedMatricesExt from within LazyArraysBlockBandedMatricesExt #345

Merged

Conversation

topolarity
Copy link
Contributor

LazyArrays reasonably assumes that an extension w/ triggers {B,C} to can depend on an extension w/ triggers {A,C} - as long as B depends on A.

Unfortunately as detailed in JuliaLang/julia#55557, Julia's extension loading does not actually guarantee that. Extension loading is purely trigger-based, so if C is the loaded after {A,B} then both of these extensions will trigger "simultaneously" and they will be loaded in an indeterminate order. That can mean that the first extension loads after the second, even though it expected to depend on it.

That behavior is arguably a bug and is likely to be improved in a future version of Julia, but for now this works around the problem by removing the inter-extension dependency.

Julia's loading system ought to allow an extension w/ triggers {B,C}
to depend on an extension w/ triggers {A,C}, under the condition that
B depends on A (i.e. the triggers for the second extension are a strict
subset of the triggers for the first extension)

Unfortunately Julia's purely trigger-based mechanism for extensions does
not actually do that. If `C` is the loaded after `{A,B}` then both of
these extensions will trigger "simultaneously" and they will be loaded
in an indeterminate order.

That problem is the same as the "cycle" in
JuliaLang/julia#55557, so-called because pre-
compilation will try to load each of the simultaneously-triggered
extensions "before" the other repeatedly, leading to an ordering cycle.

This works around the problem by removing the inter-extension dependency.
Copy link

codecov bot commented Sep 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.68%. Comparing base (25426ee) to head (94fd13b).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #345   +/-   ##
=======================================
  Coverage   95.68%   95.68%           
=======================================
  Files          17       17           
  Lines        3155     3155           
=======================================
  Hits         3019     3019           
  Misses        136      136           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dlfivefifty
Copy link
Member

Thanks for this!

@dlfivefifty dlfivefifty merged commit 2e0d3fa into JuliaArrays:master Sep 5, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants