JuliaAstro / FITSIO.jl

Flexible Image Transport System (FITS) file support for Julia
http://juliaastro.org/FITSIO.jl/
MIT License
55 stars 29 forks source link

Precompilation? #186

Open timholy opened 1 year ago

timholy commented 1 year ago

Not a big deal, but I thought it worth reporting that at least one person has likely used this package as an indication of julia's overall performance, and may have been misled by compilation time: see the first comment at https://www.youtube.com/watch?v=x4oi0IKf52w&lc=UgzzQ_aNhgxC30YXVYx4AaABAg. The latency is pretty short but it could be worth considering precompilation?

giordano commented 1 year ago

I don't know, some precompilation statements probably wouldn't hurt anyway, but it looks already decently fast to me:

% julia --startup-file=no --project=/tmp -q
julia> @time @eval using FITSIO
  0.067589 seconds (128.19 k allocations: 8.997 MiB, 3.78% compilation time)

julia> using Downloads

julia> file = Downloads.download("https://fits.gsfc.nasa.gov/samples/UITfuv2582gc.fits");

julia> @time @eval read(FITS(file)[1]);
  0.225615 seconds (531.93 k allocations: 29.220 MiB, 98.88% compilation time)

julia> @time @eval read(FITS(file)[1]);
  0.001829 seconds (66 allocations: 1.003 MiB)

This despite the fact FITS files have the problem that deserialising from disk is intrinsically type-unstable

giordano commented 1 year ago

With

diff --git a/src/FITSIO.jl b/src/FITSIO.jl
index ac38c23..05a947c 100644
--- a/src/FITSIO.jl
+++ b/src/FITSIO.jl
@@ -253,4 +253,14 @@ include("header.jl")  # FITSHeader methods
 include("image.jl")  # ImageHDU methods
 include("table.jl")  # TableHDU & ASCIITableHDU methods

+mktempdir() do dir
+    fname = joinpath(dir, "file.fits")
+    for data in (zeros(Float64, 2), zeros(Float64, 2, 2), zeros(Float32, 2), zeros(Float32, 2, 2))
+        FITS(fname, "w") do f
+            write(f, data)
+        end
+        read(FITS(fname, "r")[1])
+    end
+end
+
 end # module

I get with Julia v1.8

  0.135980 seconds (237.09 k allocations: 17.650 MiB, 9.57% gc time, 2.08% compilation time) # @time @eval using FITSIO
  0.161866 seconds (28.97 k allocations: 2.481 MiB, 98.47% compilation time) # @time @eval read(FITS(file)[1]);
  0.001846 seconds (66 allocations: 1.003 MiB) # @time @eval read(FITS(file)[1]);

and with v1.9:

  0.116300 seconds (169.84 k allocations: 12.254 MiB, 12.26% compilation time) # @time @eval using FITSIO
  0.005214 seconds (457 allocations: 1.030 MiB, 56.71% compilation time) # @time @eval read(FITS(file)[1]);
  0.001757 seconds (66 allocations: 1.003 MiB) # @time @eval read(FITS(file)[1]);

But I feel like the current 0.2 s for first read is well below the threshold of being annoying.

@mileslucas anything else we may want to precompile?

mileslucas commented 1 year ago

3D cubes are not uncommon, we could add an extra entry for Float32 and 64 with zeros(2,2,2). We could think about entries for read_header, too. I generally agree, I don't think FITSIO has problematic load times- having a couple precompile statements would probably be nice but we shouldn't overthink it.

timholy commented 1 year ago

I agree this doesn't seem like a terribly important package for precompilation. Fine to close this if you prefer.