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

Slow rotate on this order rotate().resize().toBuffer() #3725

Closed
3 tasks done
rnavarroz opened this issue Jul 8, 2023 · 6 comments
Closed
3 tasks done

Slow rotate on this order rotate().resize().toBuffer() #3725

rnavarroz opened this issue Jul 8, 2023 · 6 comments

Comments

@rnavarroz
Copy link

Possible bug

Is this a possible bug in a feature of sharp, unrelated to installation?

  • Running npm install sharp completes without error.
  • Running node -e "require('sharp')" completes without error.

If you cannot confirm both of these, please open an installation issue instead.

Are you using the latest version of sharp?

  • I am using the latest version of sharp as reported by npm view sharp dist-tags.latest.

If you cannot confirm this, please upgrade to the latest version and try again before opening an issue.

If you are using another package which depends on a version of sharp that is not the latest, please open an issue against that package instead.

What is the output of running npx envinfo --binaries --system --npmPackages=sharp --npmGlobalPackages=sharp?

System:
    OS: Linux 5.15 Ubuntu 22.04.2 LTS 22.04.2 LTS (Jammy Jellyfish)
    CPU: (8) x64 Intel(R) Core(TM) i5-10210U CPU @ 1.60GHz
    Memory: 356.21 MB / 15.26 GB
    Container: Yes
    Shell: 5.1.16 - /bin/bash
  Binaries:
    Node: 16.14.0 - ~/.config/nvm/versions/node/v16.14.0/bin/node
    npm: 8.3.1 - ~/.config/nvm/versions/node/v16.14.0/bin/npm
  npmPackages:
    sharp: ^0.32.0 => 0.32.1 

What are the steps to reproduce?

  1. Took a large image with an orientation RightTop (like image attached in this report) which need to be rotated first before applying a resizing.
  2. Use the example of code below to note that there is a big difference of time when you change the order between rotate and resize and also this does not happen if rotate is run first before an extract method.

What is the expected behaviour?

The usage of rotate().resize().toBuffer() should not be too long, it should be close to the time taken when rotate().extract().toBuffer() is executed.

Please provide a minimal, standalone code sample, without other dependencies, that demonstrates this problem

const optionsResize = { width: 1297, height: 975, withoutEnlargement: true, fit: 'inside' };
  const optionsExtract = { width: 1297, height: 975, left: 2819, top: 4136 };
  const file = 'test.jpg';
  console.time('rotateResize');
  await sharp(file)
    .rotate()
    .resize(optionsResize)
    .toBuffer();
  console.timeEnd('rotateResize');

  console.time('rotateExtract');
  await sharp(file)
    .rotate()
    .extract(optionsExtract)
    .toBuffer();
  console.timeEnd('rotateExtract');

  console.time('resizeRotate');
  await sharp(file)
    .resize(optionsResize)
    .rotate()
    .toBuffer();
  console.timeEnd('resizeRotate');

output

rotateResize: 5.161s  <----- very high compared to  the other actions.
rotateExtract: 28.781ms
resizeRotate: 147.8ms

Please provide sample image(s) that help explain this problem

test.tar.gz

@lovell
Copy link
Owner

lovell commented Jul 9, 2023

Hi, thanks for reporting, this was a relatively easy thing to improve by switching to libvips' sequential read optimisation for EXIF-based auto-orientation - see commit 2f67823

For the test image provided, this provides at least a 3x performance gain. This improvement will be in v0.32.2

@rnavarroz
Copy link
Author

Great to hear it was easy to improve it. Thanks for include it. Just curious this new version when would be released it? :)

@lovell
Copy link
Owner

lovell commented Jul 11, 2023

v0.32.2 now available.

@lovell lovell closed this as completed Jul 11, 2023
@rnavarroz
Copy link
Author

thanks @lovell, I just tested it and I am not sure why now I am getting the following Error testing the code above, this is the new output:

rotateResize: 976.65ms
rotateExtract: 587.464ms
node:internal/process/promises:265
            triggerUncaughtException(err, true /* fromPromise */);
            ^

[Error: VipsJpeg: out of order read at line 867]

I can not do resize().rotate().toBuffer():

console.time('resizeRotate');
  await sharp(file)
    .resize(optionsResize)
    .rotate()
    .toBuffer();
  console.timeEnd('resizeRotate');

@lovell
Copy link
Owner

lovell commented Jul 12, 2023

Argh, not sure why I didn't spot that, sorry.

Commit bcd865c refactors the pipeline logic to ensure decoding can remain sequential for all operations, which should help avoid this situation. This will be in v0.32.3.

@lovell lovell reopened this Jul 12, 2023
@lovell lovell modified the milestones: v0.32.2, v0.32.3 Jul 12, 2023
@lovell
Copy link
Owner

lovell commented Jul 14, 2023

v0.32.3 now available.

@lovell lovell closed this as completed Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants