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

move GeometryBasics to Makie requires block #1322

Merged
merged 1 commit into from
Jan 6, 2023
Merged

Conversation

ranocha
Copy link
Member

@ranocha ranocha commented Jan 6, 2023

I looked at the load times and latency issues with Julia v1.9 beta. Using the branch JuliaLang/julia#48075, this reduces the time of using Trixi for me from

julia> @time @eval using Trixi
  9.252053 seconds (19.39 M allocations: 1.462 GiB, 5.46% gc time, 28.00% compilation time: 36% of which was recompilation)

on main to

julia> @time @eval using Trixi
  8.004904 seconds (17.29 M allocations: 1.301 GiB, 5.61% gc time, 24.59% compilation time: 24% of which was recompilation)

on this branch.

As far as I can tell, the functions I moved are not called outside of the Makie recipes stuff. We lose the ability to use specify compat bounds on GeometryBasics.jl in this way, but I hope this is no problem. This could be solved/improved in the future with conditional dependencies introduced in Julia v1.9, see https://pkgdocs.julialang.org/dev/creating-packages/#Conditional-loading-of-code-in-packages-(Extensions)

What's your take on this, @jlchan ?

Copy link
Contributor

@jlchan jlchan left a comment

Choose a reason for hiding this comment

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

Nice find! I like this fix - reducing the number of independently loaded packages makes sense, and it makes it more clear that GeometryBasics is just used with GLMakie.

@ranocha ranocha enabled auto-merge (squash) January 6, 2023 14:50
@sloede
Copy link
Member

sloede commented Jan 6, 2023

Great work! Will there be similar opportunities for other packages?

@ranocha
Copy link
Member Author

ranocha commented Jan 6, 2023

I'm looking into it

@ranocha
Copy link
Member Author

ranocha commented Jan 6, 2023

@sloede: Here you are jlchan/StartUpDG.jl#73 (but is is not as pronounced as the result here)

@ranocha ranocha merged commit dbcea9c into main Jan 6, 2023
@ranocha ranocha deleted the hr/GeometryBasics branch January 6, 2023 17:48
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.

3 participants