Improve inferrabililty

[timholy]

This makes the methods of this package less vulnerable to invalidation, should improve precompilability, and result in a small runtime performance improvement:

master:

julia> img = rand(RGB{N0f8}, 100, 100);

julia> @btime begin
           TiffImages.save("/tmp/file.tiff", img)
           imgs = TiffImages.load("/tmp/file.tiff")
       end;
  417.934 μs (561 allocations: 110.66 KiB)

This branch:

julia> @btime begin
           TiffImages.save("/tmp/file.tiff", img)
           imgs = TiffImages.load("/tmp/file.tiff")
       end;
  391.745 μs (570 allocations: 112.08 KiB)

[timholy]

Some of the individual commits have commit messages that may be worth reading (particularly if you think some of the changes here make the code uglier):

• 17f0032
• b7fccb7

[timholy]

CC @IanButterworth

[timholy]

I noticed this first by reading the code, but as I changed method dispatch I sometimes hit it in real life. I think it's pretty important to eliminate this before it becomes a FileIO default.

[tlnagy]

Ack. Good catch. I need to just delete this method and figure out a different way to detect strided bilevel TIFFs

[tlnagy]

Also, I'm 99% sure replacing io with tf:TiffFile would still work and fix the type piracy.

[timholy]

Having "finished" this, were I to do it over again I'd probably add a S<:Stream type parameter to TiffFile; it would eliminate the ugliest parts of this (the splitting of, e.g., Base.read out to _read) and might even yield more performance benefits and longer successful inference chains. I'll wait for feedback before making that change, though.

[tlnagy]

Thanks for taking the time to put this code base through the wringer! I'm not sure I understand how S<:Stream would help in TiffFile? The type definition already specifies io is of type Stream?

I didn't realize that Stream should be parameterized, so yes, I think the way forward is to parameterize TiffFile with S<:Stream to avoid runtime dispatch.

So in that case, we could add an io(tf:TiffFile) = tf.io accessor for the underlying Stream using compile-type dispatch? Because that would be cleaner.

[timholy]

Down to

julia> @btime begin
           TiffImages.save("/tmp/file.tiff", img)
           imgs = TiffImages.load("/tmp/file.tiff")
       end;
  370.517 μs (545 allocations: 111.09 KiB)

which is now getting into the territory of considerable improvement. I took the liberty of shifting you from TiffFile(T, args...) to TiffFile{T}(args...), mirroring how our Array etc. constructors work these days. Hope that's OK.

[codecov-io]

Codecov Report

Merging #36 (a93818a) into master (5f7e640) will increase coverage by 0.30%.
The diff coverage is 96.00%.

@@            Coverage Diff             @@
##           master      #36      +/-   ##
==========================================
+ Coverage   87.84%   88.15%   +0.30%     
==========================================
  Files          12       12              
  Lines         543      557      +14     
==========================================
+ Hits          477      491      +14     
  Misses         66       66              

Impacted Files	Coverage Δ
src/TiffImages.jl	100.00% <ø> (ø)
src/ifds.jl	94.30% <92.00%> (+0.29%)	⬆️
src/layout.jl	88.46% <92.30%> (+0.22%)	⬆️
src/files.jl	83.92% <100.00%> (ø)
src/load.jl	94.00% <100.00%> (+0.25%)	⬆️
src/tags.jl	94.44% <100.00%> (+0.26%)	⬆️
src/types/dense.jl	97.01% <100.00%> (-0.05%)	⬇️
src/utils.jl	82.92% <100.00%> (+0.42%)	⬆️

---

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 5f7e640...a93818a. Read the comment docs.

[tlnagy]

I'm happy with these changes now that we parameterize TiffFile on the stream type! Thanks @timholy.

[IanButterworth]

Nice! Happy to update #35 after this merges

[tlnagy]

TiffImages.jl master (with this PR) vs ImageMagick for a large TIFF

julia> using TiffImages
[ Info: Precompiling TiffImages [731e570b-9d59-4bfa-96dc-6df516fadf69]

julia> @time TiffImages.load("/home/tlnagy/Downloads/Gcamp6F_BSA_60px_3_stack.ome.tif");
  2.291251 seconds (3.37 M allocations: 664.546 MiB, 2.18% gc time)

julia> @time TiffImages.load("/home/tlnagy/Downloads/Gcamp6F_BSA_60px_3_stack.ome.tif");
  0.739291 seconds (276.28 k allocations: 504.956 MiB, 2.49% gc time)

julia> using ImageMagick

julia> @time ImageMagick.load("/home/tlnagy/Downloads/Gcamp6F_BSA_60px_3_stack.ome.tif");
  4.011342 seconds (2.70 M allocations: 1.540 GiB, 4.56% gc time)

julia> @time ImageMagick.load("/home/tlnagy/Downloads/Gcamp6F_BSA_60px_3_stack.ome.tif");
  3.180862 seconds (6.72 k allocations: 1.408 GiB, 0.51% gc time)

Faster on initial load and way faster on subsequent loads (~5x). Also way less memory usage.

[timholy]

Thanks for merging! Note I didn't do anything to solve the piracy issue.

Looking forward to seeing this in action!

[timholy]

I am so glad to be making progress on moving away from ImageMagick! I'm impressed (and slightly puzzled) that the latency is already lower. That should improve even more with #35.

[tlnagy]

Thanks for merging! Note I didn't do anything to solve the piracy issue.

Fix over in #38. Added another test for this code branch because coverage was a little shallow 😅