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

Fix type piracy issue #38

Merged
merged 4 commits into from
Mar 1, 2021
Merged

Fix type piracy issue #38

merged 4 commits into from
Mar 1, 2021

Conversation

tlnagy
Copy link
Owner

@tlnagy tlnagy commented Feb 25, 2021

Fixes the issue brought up by @timholy in #36 (comment). Added this branch to testing as well.

@tlnagy tlnagy mentioned this pull request Feb 25, 2021
src/files.jl Outdated Show resolved Hide resolved
@tlnagy
Copy link
Owner Author

tlnagy commented Feb 25, 2021

This also fixes a pretty serious bug that caused TiffImages to ignore stripped offsets, which actually yielded a substantial performance improvement. So this PR introduces a ~5x performance regression vs master. This condition is likely common enough that it's worth having a separate branch to handle the case were an image is striped but the stripes are contiguous and can therefore be read in as a single block.

@tlnagy
Copy link
Owner Author

tlnagy commented Feb 26, 2021

Checking for contiguous strips allows me to recover the performance of master.

julia> using TiffImages

julia> @time TiffImages.load("/home/tlnagy/Downloads/Gcamp6F_BSA_60px_3_stack.ome.tif");
Progress: 100%|█████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| Time: 0:00:01
  2.724760 seconds (4.75 M allocations: 738.599 MiB, 2.18% gc time)

julia> @time TiffImages.load("/home/tlnagy/Downloads/Gcamp6F_BSA_60px_3_stack.ome.tif");
  0.768867 seconds (286.85 k allocations: 513.183 MiB, 3.43% gc time)

julia> using ImageMagick

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

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

@tlnagy tlnagy force-pushed the tn/fix-type-piracy branch from d6b06c3 to f2b2506 Compare February 26, 2021 00:13
tlnagy and others added 3 commits March 1, 2021 12:17
fixes the issue brought up by @timholy in #36 (comment)
check if striped data is contiguous and then read in as block
tlnagy added a commit to tlnagy/exampletiffs that referenced this pull request Mar 1, 2021
this fills a gap in the test suite that led to
tlnagy/TiffImages.jl#38 (comment)
@tlnagy tlnagy force-pushed the tn/fix-type-piracy branch from f2b2506 to 5dfef35 Compare March 1, 2021 22:08
@tlnagy
Copy link
Owner Author

tlnagy commented Mar 1, 2021

I created an artificial image that has its stripes offset by a random amount that should catch any regressions in handling discontiguously striped uncompressed images.

@tlnagy tlnagy merged commit 31191cb into master Mar 1, 2021
@tlnagy tlnagy deleted the tn/fix-type-piracy branch March 1, 2021 22:16
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

Successfully merging this pull request may close these issues.

2 participants