KristofferC / PGFPlotsX.jl

Plots in Julia using the PGFPlots LaTeX package
Other
301 stars 40 forks source link

Requires doubles the load time #227

Closed KristofferC closed 4 years ago

KristofferC commented 4 years ago

Master


julia> @time using PGFPlotsX
  0.729749 seconds (1.68 M allocations: 92.581 MiB, 2.23% gc time)

without Requries:

julia> @time using PGFPlotsX
  0.401542 seconds (665.73 k allocations: 41.712 MiB, 1.70% gc time)

This is without loading any of the extra packages. My guess is that this is mostly a problem with the implementation of Requires and could perhaps be fixed. Without any of the conditional packages it should be approximately zero work (definitely not in the 0.3 s range).

tpapp commented 4 years ago

I am not sure how to move forward with this:

  1. open an issue for Requires with an MWE, or
  2. move the conditionally loaded code to intersection modules, eg PGFPlotsXDataFrames, which at this stage would be loaded on demand by the user, and then
  3. wait for something like JuliaLang/Pkg.jl#1285 to be implemented and apply that.

I wouldn't be against 2 for now, but I understand that it would complicate usage for some people.

KristofferC commented 4 years ago

The overhead seems crazy considering that it actually doesn't do anything. So I guess this is here to annoy us enough to figure out exactly why this happens and if we can fix it (in Requires or using some home made version)

KristofferC commented 4 years ago

Better on master:

julia> @time using PGFPlotsX
  0.476114 seconds (981.58 k allocations: 58.434 MiB, 1.57% gc time)

Edit: Now with StatsPlots removed:

julia> @time using PGFPlotsX
  0.368157 seconds (698.54 k allocations: 42.866 MiB, 1.93% gc time)
julia> @time using PGFPlotsX # without Requires
  0.287308 seconds (505.19 k allocations: 31.939 MiB)
KristofferC commented 4 years ago

On current master (with Requires):

julia> @time using PGFPlotsX
  0.205683 seconds (424.92 k allocations: 28.728 MiB)

I think this is at the point where we don't have to care anymore. :tada: