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

Precompilation? #186

Open
timholy opened this issue Mar 4, 2023 · 4 comments
Open

Precompilation? #186

timholy opened this issue Mar 4, 2023 · 4 comments

Comments

@timholy
Copy link

timholy commented Mar 4, 2023

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
Copy link
Member

giordano commented Mar 4, 2023

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
Copy link
Member

giordano commented Mar 4, 2023

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
Copy link
Member

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
Copy link
Author

timholy commented Mar 5, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants