strip cldr artifact before build #687
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
It builds now, but when actually using the |
|
Nice that it builds. The new error seems unrelated, possibly this is new since we updated the manifests. I've seen it before that it downloads MKL on first usage. That seems to fail now, not sure if that is only on TeamCity or everywhere, I don't see an artifact for this build that I can download. I've added 56eae5f to see if that helps. That should already download and bundle all lazy artifacts, which seems preferable anyway, granted it doesn't download many more artifacts we don't actually use. |
It doesn't upload artifacts if the build fails, I've changed that behavior and triggered a new build |
|
Nice, thanks! This seems to work well, also tried the cli locally. It doesn't download MKL on first use anymore. Though now the ribasim_cli.zip grew from 250 MB to 435 MB, due to the new MKL artifact that is 640 MB unzipped. The reason we need MKL is because we use https://github.com/SciML/LinearSolve.jl which depends on it. Not sure if it is feasible/desirable to try to avoid it. |
Unpacked, they are now 1.57 GB. I feel like we reached the point that investigating these large binary sizes should become a higher priority. |
Hofer-Julian
approved these changes
Oct 20, 2023
visr
added a commit
that referenced
this pull request
Oct 25, 2023
Alternative to #687, which wasn't working well with PackageCompiler due to long paths still. This switches to a fork. A PR to merge this upstream has already been submitted: JuliaTime/TimeZones.jl#450 Clearly this is temporary, but since we use Manifests this should work ok.
Hofer-Julian
pushed a commit
that referenced
this pull request
Oct 25, 2023
Alternative to #687, which wasn't working well with PackageCompiler due to long paths still, leading to broken builds. This switches to a fork. A PR to merge this upstream has already been submitted: JuliaTime/TimeZones.jl#450 Clearly this is temporary, but since we use Manifests this should work ok.
visr
added a commit
that referenced
this pull request
Oct 25, 2023
Unfortunately #685 didn't work, since the same problematic artifact is included in older releases. This works around the same issue by stripping the cldr artifact of all unnecessary files.
visr
added a commit
that referenced
this pull request
Oct 25, 2023
Alternative to #687, which wasn't working well with PackageCompiler due to long paths still, leading to broken builds. This switches to a fork. A PR to merge this upstream has already been submitted: JuliaTime/TimeZones.jl#450 Clearly this is temporary, but since we use Manifests this should work ok.
Hofer-Julian
pushed a commit
that referenced
this pull request
Oct 26, 2023
Hofer-Julian
pushed a commit
that referenced
this pull request
Feb 26, 2024
Takes a good chunk out of #319. Fixes #1153. Fixes #687 (comment) Our `ribasim_cli_windows.zip` is 535 MB on main, this reduces it to 227 MB. Unpacked, it goes from 1.9 GB to 1.0 GB. Libribasim shrinks by the same amount. This is because we no longer include two 400 MB artifacts, MinGW and MKL. Three PackageCompiler keyword arguments come into play here: - set to true: `include_preferences::Bool`: If true, store all preferences visible by the project in package_dir in the app bundle. Defaults to true. - set to false: `include_lazy_artifacts::Bool`: if lazy artifacts should be included in the bundled artifacts, defaults to false. - set to false: `include_transitive_dependencies::Bool`: If true, explicitly put all transitive dependencies into the sysimage. This only makes a difference if some packages do not load all their dependencies when themselves are loaded. Defaults to true. I added an explicit `include_preferences=true`. This is mostly to highlight that we are making use of Preferences to tell LinearSolve that we don't want to load MKL_jll. This would be enough outside of PackageCompiler, however more was needed. The `include_lazy_artifacts=false` is restoring the default behavior. We had set it to true to stop it from downloading MKL_jll the first time the CLI was used. When I added this option, it would still do that, so more was needed. The `include_transitive_dependencies=false` is deviating from the default, but this causes several issues. I'm not sure, but for MKL_jll perhaps the problem was that the code was already fixed to the default load MKL_jll behavior in the sysimage. One more note about choosing to opt out of using MKL_jll, [by default we use KLUFactorization](https://github.com/SciML/LinearSolve.jl/blob/v2.24.0/src/default.jl#L114), since in `[solver]` we set `sparse = true` by default. It could be that `sparse = false` will be a bit slower. I haven't taken the time to measure, but I doubt it is worth the binary size, since `sparse = false` only makes sense for small models. A nice side effect is that this combination of keys also took MinGW out of the sysimage. The fact that we got it might be a PackageCompiler bug, though I could not repoduce this in small test executables. Some references for those interested: https://julialang.github.io/PackageCompiler.jl/stable/refs.html#PackageCompiler.create_app https://juliapackaging.github.io/Preferences.jl/dev/ https://github.com/SciML/LinearSolve.jl/pull/425/files I snuck in some extra preferences. When precompiling, OrdinaryDiffEq always takes very long since it precompiles a lot of combinations of solvers and specializations. I had a look at the docs and precompilation code linked below, and concluded that we probably don't gain too much from these precompile workloads. Our default solver, QNDF, is not precompiled, and for snappy CLI startup we have our own precompilation script that runs Ribasim, which is probably more effective anyway. That means developers using Julia have to spend less time waiting for OrdinaryDiffEq to precompile, possibly at the cost of waiting slightly longer to compile a first Ribasim run from Julia. I haven't measured. Another benefit is that we will create less code; the `lib/julia/sys.dll` shrinks by 50 MB to 672 MB due to less unused OrdinaryDiffEq code. https://docs.sciml.ai/DiffEqDocs/stable/features/low_dep/ https://github.com/SciML/OrdinaryDiffEq.jl/blob/v6.71.0/src/OrdinaryDiffEq.jl#L236-L334 https://github.com/trixi-framework/libtrixi/blob/v0.1.4/LibTrixi.jl/Project.toml The builds from this branch do run the test models just fine, don't download anything on start, and start more quickly.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Unfortunately #685 didn't work, since the same problematic artifact is included in older releases.
This works around the same issue by stripping the cldr artifact of all unnecessary files.