JuliaApproximation / CompactBases.jl

Julia library for function approximation with compact basis functions
MIT License
16 stars 2 forks source link

RFC: Disable precompilation to avoid memory spike #38

Closed mortenpi closed 2 years ago

mortenpi commented 3 years ago

Doing using CompactBases makes Julia use a lot of memory (3+ GiB). This does not come from precompilation itself actually, since it also occurs when loading from the .ji. Now, running GC.gc() after using CompactBases does free all that memory, but often that's too late, as the OOM killer has terminated the Julia process already. Disabling precompilation does avoid the issue.

It will make it slightly longer to do using CompactBases (15s with __precompile(false)__, vs ~25s for precompilation and ~10s for loading from .ji).

codecov[bot] commented 3 years ago

Codecov Report

Merging #38 (b01fddb) into master (2a2a4b3) will increase coverage by 1.17%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #38      +/-   ##
==========================================
+ Coverage   64.05%   65.23%   +1.17%     
==========================================
  Files          22       22              
  Lines        1533     1562      +29     
==========================================
+ Hits          982     1019      +37     
+ Misses        551      543       -8     
Impacted Files Coverage Δ
src/CompactBases.jl 88.88% <ø> (+1.38%) :arrow_up:
src/fedvr.jl 87.09% <0.00%> (+0.05%) :arrow_up:
src/linear_operators.jl 81.53% <0.00%> (+0.28%) :arrow_up:
src/finite_differences.jl 70.61% <0.00%> (+0.30%) :arrow_up:
src/knot_sets.jl 73.25% <0.00%> (+0.31%) :arrow_up:
src/bsplines.jl 76.59% <0.00%> (+0.33%) :arrow_up:
src/fd_derivatives.jl 48.87% <0.00%> (+0.58%) :arrow_up:
src/materialize_dsl.jl 86.79% <0.00%> (+29.64%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2a2a4b3...b01fddb. Read the comment docs.

jagot commented 3 years ago

I am unsure about this; we should definitely get the compile time/memory usage down, but I frankly don't know where the problem stems from. As a workaround, you could force precompilation separately ] precompile, and to be really sure, restart the REPL. Does that change things? If not, we can probably merge this PR until we figured out a long-term solution.

mortenpi commented 3 years ago

As a workaround, you could force precompilation separately ] precompile, and to be really sure, restart the REPL. Does that change things?

Nope. The memory spike comes from loading the .ji it looks like.

mortenpi commented 3 years ago

I've updated this to make it optional -- with this you can disable precompilation for CompactBases by setting the COMPACTBASES_NO_PRECOMPILE environment variable. So the default behavior is still to precompile.

Just as a note, if you have already precompiled the package and then set the environment variable (for a given set of dependencies), then it won't have an effect. And the problem still occurs because the memory spike comes from loading in the .ji file.