Move tiff JPEG compression outside the lock and thread it #3679

jcupitt · 2023-09-22T13:01:03Z

At the moment, libvips uses libtiff to write JPEG compressed tiles. Since compression is inside libtiff, it has to be single-threaded.

This PR moves JPEG compression into libvips and uses a threadpool to write JPEG and JP2K compressed tiles in parallel.

git master:

$ time vips copy CMU-1.svs[rgb] x.tif[tile,compression=jpeg,pyramid]
real	0m11.440s
user	1m10.627s
sys	0m4.162s

This PR:

$ time vips copy CMU-1.svs[rgb] x.tif[compression=jpeg,tile,pyramid]
real	0m4.972s
user	1m3.305s
sys	0m7.089s

euzada · 2023-09-22T20:01:55Z

Thank you @jcupitt for starting this PR. If I understand correctly, it still running on single thread even after move it inside libvips?

I failed to compile this version on ubuntu. There may be some issues compiling jpeg2vips.

Looking forward to test it.

jcupitt · 2023-09-23T14:17:42Z

jcupitt · 2023-09-23T14:21:24Z

Thank you @jcupitt for starting this PR. If I understand correctly, it still running on single thread even after move it inside libvips?

You're right, it still needs a threadpool to write each line of tiles. I'll put that in next.

jcupitt · 2023-09-24T12:33:40Z

I added threaded write (as well as compression by libvips).

Git master libvips:

$ time vips copy CMU-1.svs[rgb] x.tif[compression=jpeg,tile,pyramid]
real	0m11.408s
user	1m10.671s
sys	0m4.546s

This PR:

$ time vips copy CMU-1.svs[rgb] x.tif[compression=jpeg,tile,pyramid]
real	0m7.385s
user	1m5.646s
sys	0m6.712s

So about 50% faster on this PC, nice!

euzada · 2023-09-24T12:42:32Z

I added threaded write (as well as compression by libvips).

Git master libvips:
$ time vips copy CMU-1.svs[rgb] x.tif[compression=jpeg,tile,pyramid]
real	0m11.408s
user	1m10.671s
sys	0m4.546s
This PR:
$ time vips copy CMU-1.svs[rgb] x.tif[compression=jpeg,tile,pyramid]
real	0m7.385s
user	1m5.646s
sys	0m6.712s
So about 50% faster on this PC, nice!

That's pretty impressive. I am waiting for tomorrow to do some testing. Hope I can build it for windows successfully first. My test will be using a very large image where the old version took almost 1h to finish.

jcupitt · 2023-09-24T12:51:42Z

This is the largest test image I have to hand:

$ time vips copy C2023000589S1-0-1_743eee76-f53b-aca2-4da5-fa97a44d9933_125232.svs[rgb] x.tif[tile,compression=jpeg,pyramid,tile-width=256,tile-height=256] --vips-progress --vips-leak
vips temp-2: 211213 x 87581 pixels, 32 threads, 128 x 128 tiles, 640 lines in buffer
vips temp-2: done in 86.9s          
vips_threadset_free: peak of 64 threads
memory: high-water mark 1.44 GB

real	1m27.625s
user	12m44.819s
sys	0m47.753s

jcupitt · 2023-09-24T13:06:48Z

Ah jp2k compression sees more of a speedup. Before:

$ time vips copy CMU-1.svs[rgb] x.tif[compression=jp2k,tile,pyramid]
real	7m40.086s
user	9m17.533s
sys	9m3.294s

After:

$ time vips copy CMU-1.svs[rgb] x.tif[compression=jp2k,tile,pyramid]
real	2m25.948s
user	10m17.321s
sys	18m49.749s

euzada · 2023-09-24T14:55:06Z

Ah jp2k compression sees more of a speedup. Before:

$ time vips copy CMU-1.svs[rgb] x.tif[compression=jp2k,tile,pyramid]
real	7m40.086s
user	9m17.533s
sys	9m3.294s

After:

$ time vips copy CMU-1.svs[rgb] x.tif[compression=jp2k,tile,pyramid]
real	2m25.948s
user	10m17.321s
sys	18m49.749s

That again a very big improvement. So 70 % faster in case of jp2k and 35% faster in jpeg.

Silly question : what's the advantage of using jp2k over jpeg in tiff compression? The jpeg tiles are very small and in term of their sizes, they are probably smaller and faster to load.

jcupitt · 2023-09-24T15:41:51Z

jp2k doesn't really have any advantages imo, it's just there for compatibility with some deranged software that still uses it.

by copying the TIFFTAG_JPEGTABLES tag

jcupitt · 2023-09-24T16:14:12Z

Ah I found another speedup: we can copy the TIFFTAG_JPEGTABLES tag and skip decompress/recompress on level copy.

Git master:

$ time vips copy CMU-1.svs[rgb] x.tif[compression=jpeg,tile,pyramid]
real	0m11.408s
user	1m10.671s
sys	0m4.546s

This PR:

$ time vips copy CMU-1.svs[rgb] x.tif[compression=jpeg,tile,pyramid]
real	0m4.972s
user	1m3.305s
sys	0m7.089s

More than 2x faster now.

euzada · 2023-09-24T17:38:24Z

Ah I found another speedup: we can copy the TIFFTAG_JPEGTABLES tag and skip decompress/recompress on level copy.

Git master:
$ time vips copy CMU-1.svs[rgb] x.tif[compression=jpeg,tile,pyramid]
real	0m11.408s
user	1m10.671s
sys	0m4.546s
This PR:
$ time vips copy CMU-1.svs[rgb] x.tif[compression=jpeg,tile,pyramid]
real	0m4.972s
user	1m3.305s
sys	0m7.089s
More than 2x faster now.

Is this TIFFTAG_JPEGTABLES only optimized for Q=70 the default value? Tifftag_jpegtables

jcupitt · 2023-09-24T17:59:55Z

Is this TIFFTAG_JPEGTABLES only optimized for Q=70 the default value? Tifftag_jpegtables

It's created for each Q value.

jcupitt · 2023-09-24T18:03:48Z

OK, all done, I think.

euzada · 2023-09-24T18:17:56Z

OK, all done, I think.

That was very fast for >50% improvement. Now to test it, do I need to compile this branch in mxe for windows?

jcupitt · 2023-09-24T18:31:46Z

do I need to compile this branch in mxe for windows?

No idea, probably! I think it should work.

kleisauke · 2023-09-25T16:46:28Z

do I need to compile this branch in mxe for windows?

I just updated the 8.15-master branch within the build-win64-mxe repository, allowing you to build Windows binaries for this PR using:

$ ./build.sh --ref tiffsave-threaded-jpeg

euzada · 2023-09-25T17:00:16Z

./build.sh --ref tiffsave-threaded-jpeg

Thank you. Build successfully. Now I will test it using my large images.

update: Thank you @jcupitt for the excellent results; I did not believe it :)

**Before: vips 8.14.3**
Image C:\Users\euzada\Desktop\TA24b2-5X_2023-08-09_8p14p3.tif[tile,compression=jpeg,pyramid,bigtiff] is now saved.
Process size in memory: 5319 MB
save_image took 5956.271 sec to execute!
Elapsed time: 1h39m51s

vipsheader.exe -a C:\Users\euzada\Desktop\TA24b2-5X_2023-08-09_8p14p3.tif
C:\Users\euzada\Desktop\TA24b2-5X_2023-08-09_8p14p3.tif: 323520x315128 uchar, 3 bands, srgb, tiffload
width: 323520
height: 315128
bands: 3
format: uchar
coding: none
interpretation: srgb
xoffset: 0
yoffset: 0
xres: 3.77953
yres: 3.77953
filename: C:\Users\euzada\Desktop\TA24b2-5X_2023-08-09_8p14p3.tif
vips-loader: tiffload
n-pages: 13
resolution-unit: in
orientation: 1

file_size= 6.16GB

**After: vips 8.15.0**
Image C:\Users\euzada\Desktop\TA24b2-5X_2023-08-09_8p15.tif[tile,compression=jpeg,pyramid,bigtiff] is now saved.
Process size in memory: 14822.34 MB
save_image took 797.650 sec to execute!
Elapsed time: 13m37s

vipsheader.exe -a C:\Users\euzada\Desktop\TA24b2-5X_2023-08-09_8p15.tif
C:\Users\euzada\Desktop\TA24b2-5X_2023-08-09_8p15.tif: 323520x315128 uchar, 3 bands, srgb, tiffload
width: 323520
height: 315128
bands: 3
format: uchar
coding: none
interpretation: srgb
xoffset: 0
yoffset: 0
xres: 3.77953
yres: 3.77953
filename: C:\Users\euzada\Desktop\TA24b2-5X_2023-08-09_8p15.tif
vips-loader: tiffload
n-pages: 13
resolution-unit: in
orientation: 1

file_size = 7.67GB

edit:
the difference in the size file probably due to a corrupted file in case of Vips 8.14.3. I was not able to open it using QuPath. While the new file was opening with no issue. Incorrect statement: Both files opened in QuPath. The second is larger in size. So, I get improvement of > 86% faster with no issue opening the file. The CPU was most of the time close to 100% while before was close to 10% on average.

Thank you @jcupitt and @kleisauke for the improvement.

kleisauke · 2023-09-25T19:03:20Z

It looks like this PR would cause non-deterministic output. See for example this test script:

vips copy CMU-1.svs[rgb] x.tif[compression=jpeg,tile,pyramid]

expected_sha256=$(sha256sum "x.tif" | awk '{ print $1 }')
for run in {1..5}; do
  vips copy CMU-1.svs[rgb] x.tif[compression=jpeg,tile,pyramid]
  echo "$expected_sha256 x.tif" | sha256sum --check
done

Git master:

$ ./check-deterministic.sh
x.tif: OK
x.tif: OK
x.tif: OK
x.tif: OK
x.tif: OK

This PR:

$ ./check-deterministic.sh
x.tif: FAILED
sha256sum: WARNING: 1 computed checksum did NOT match
x.tif: FAILED
sha256sum: WARNING: 1 computed checksum did NOT match
x.tif: FAILED
sha256sum: WARNING: 1 computed checksum did NOT match
x.tif: FAILED
sha256sum: WARNING: 1 computed checksum did NOT match
x.tif: FAILED
sha256sum: WARNING: 1 computed checksum did NOT match

euzada · 2023-09-25T19:07:06Z

It looks like this PR would cause non-deterministic output. See for example this test script:

vips copy CMU-1.svs[rgb] x.tif[compression=jpeg,tile,pyramid]

expected_sha256=$(sha256sum "x.tif" | awk '{ print $1 }')
for run in {1..5}; do
  vips copy CMU-1.svs[rgb] x.tif[compression=jpeg,tile,pyramid]
  echo "$expected_sha256 x.tif" | sha256sum --check
done

Git master:

$ ./check-deterministic.sh
x.tif: OK
x.tif: OK
x.tif: OK
x.tif: OK
x.tif: OK

This PR:

$ ./check-deterministic.sh
x.tif: FAILED
sha256sum: WARNING: 1 computed checksum did NOT match
x.tif: FAILED
sha256sum: WARNING: 1 computed checksum did NOT match
x.tif: FAILED
sha256sum: WARNING: 1 computed checksum did NOT match
x.tif: FAILED
sha256sum: WARNING: 1 computed checksum did NOT match
x.tif: FAILED
sha256sum: WARNING: 1 computed checksum did NOT match

Is this test will verify the output hash? The size of the output is changing from run to run.

libvips/foreign/vips2jpeg.c

libvips/foreign/vips2tiff.c

jcupitt · 2023-09-25T21:03:23Z

It looks like this PR would cause non-deterministic output.

Certainly -- it's writing the tiles across a row in parallel, so the tile order is set by which thread finishes first.

We could buffer all the compressed tiles in the row, then sort and write them sequentially, but it would obviously raise memory use.

Co-authored-by: Kleis Auke Wolthuizen <github@kleisauke.nl>

jcupitt · 2023-09-25T21:07:07Z

Thinking about it, you're right, non-deterministic output is pretty bad. I'll add gather-sort-write. Hopefully memory use won't increase too much.

jcupitt · 2023-09-26T14:46:17Z

It should be sorted now.

vips2tiff.c is getting quite big :( but maybe we can move parts of it out it later.

kleisauke

LGTM!

euzada · 2023-09-26T18:27:52Z

LGTM!

Test it and it works well on windows. Thank you.

jcupitt · 2023-09-27T04:32:14Z

Great! Thanks for testing @euzada

try moving jpeg compress outside the lock

e2fb8ba

jcupitt marked this pull request as draft September 22, 2023 13:01

jcupitt changed the title ~~Move tiff JPEG compression outside the lock~~ WiP: Move tiff JPEG compression outside the lock Sep 22, 2023

Merge branch 'master' into tiffsave-threaded-jpeg

ebb63e9

set JPEG tables for direct write

764f465

jcupitt added 5 commits September 23, 2023 20:22

pad edge tiles

f291341

Merge branch 'master' into tiffsave-threaded-jpeg

5af34bb

Merge branch 'master' into tiffsave-threaded-jpeg

79b8667

fix test suite

035775c

use a threadpool for tiled tiff compress

8c1f26e

Merge branch 'master' into tiffsave-threaded-jpeg

ad419d2

don't decompress/recompress on tiff copy

0c4bdf8

by copying the TIFFTAG_JPEGTABLES tag

jcupitt added 2 commits September 24, 2023 17:16

fix build without openjp2

e47408d

add error handling for vips2tiff jpeg tile write

9da89f8

small polish

eca68d5

jcupitt changed the title ~~WiP: Move tiff JPEG compression outside the lock~~ Move tiff JPEG compression outside the lock and thread it Sep 24, 2023

jcupitt requested a review from kleisauke September 24, 2023 18:03

jcupitt marked this pull request as ready for review September 24, 2023 18:03

fix memleak

550dea7

kleisauke reviewed Sep 25, 2023

View reviewed changes

libvips/foreign/vips2jpeg.c Outdated Show resolved Hide resolved

libvips/foreign/vips2jpeg.c Outdated Show resolved Hide resolved

libvips/foreign/vips2tiff.c Show resolved Hide resolved

jcupitt and others added 3 commits September 25, 2023 22:04

Update libvips/foreign/vips2jpeg.c

5866f80

Co-authored-by: Kleis Auke Wolthuizen <github@kleisauke.nl>

Update libvips/foreign/vips2jpeg.c

a8184c0

Co-authored-by: Kleis Auke Wolthuizen <github@kleisauke.nl>

Update libvips/foreign/vips2tiff.c

1a03595

Co-authored-by: Kleis Auke Wolthuizen <github@kleisauke.nl>

make tiffsave deterministic

383c56a

Merge branch 'master' into tiffsave-threaded-jpeg

2e4dded

kleisauke approved these changes Sep 26, 2023

View reviewed changes

kleisauke merged commit 1131b7d into master Sep 26, 2023

Uh oh!

Move tiff JPEG compression outside the lock and thread it #3679

Move tiff JPEG compression outside the lock and thread it #3679

Uh oh!

Conversation

jcupitt commented Sep 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

euzada commented Sep 22, 2023

Uh oh!

jcupitt commented Sep 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jcupitt commented Sep 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jcupitt commented Sep 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

euzada commented Sep 24, 2023

Uh oh!

jcupitt commented Sep 24, 2023

Uh oh!

jcupitt commented Sep 24, 2023

Uh oh!

euzada commented Sep 24, 2023

Uh oh!

jcupitt commented Sep 24, 2023

Uh oh!

jcupitt commented Sep 24, 2023

Uh oh!

euzada commented Sep 24, 2023

Uh oh!

jcupitt commented Sep 24, 2023

Uh oh!

jcupitt commented Sep 24, 2023

Uh oh!

euzada commented Sep 24, 2023

Uh oh!

jcupitt commented Sep 24, 2023

Uh oh!

kleisauke commented Sep 25, 2023

Uh oh!

euzada commented Sep 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kleisauke commented Sep 25, 2023

Uh oh!

euzada commented Sep 25, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jcupitt commented Sep 25, 2023

Uh oh!

jcupitt commented Sep 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jcupitt commented Sep 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kleisauke left a comment

Choose a reason for hiding this comment

Uh oh!

euzada commented Sep 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jcupitt commented Sep 27, 2023

Uh oh!

Uh oh!

jcupitt commented Sep 22, 2023 •

edited

Loading

jcupitt commented Sep 23, 2023 •

edited

Loading

jcupitt commented Sep 23, 2023 •

edited

Loading

jcupitt commented Sep 24, 2023 •

edited

Loading

euzada commented Sep 25, 2023 •

edited

Loading

jcupitt commented Sep 25, 2023 •

edited

Loading

jcupitt commented Sep 26, 2023 •

edited

Loading

euzada commented Sep 26, 2023 •

edited

Loading