Concurrency queue should slice per output image, not input image

[ryanccn]

The global Image.concurrency option is set as the concurrency limit for Images and not their underlying processes. An Image can have multiple formats and widths, and all of them are processed with sharp simultaneously, which can be really inefficient, increase resource usage, and reduce performance (not to mention break builds on some platforms!).

IMHO Image.concurrency should be set as the maximum concurrency of the individual output processes, not the Images. I already have a working patch for this that simply uses the global PQueue for the sharp Promises instead of the Images. Another less optimal solution could be to introduce another option that controls actual processing concurrency.

[zachleat]

What’s the patch look like?

[ryanccn]

Should I open a PR with this patch?

diff --git a/img.js b/img.js
index 58868a750853c658fe182e2bda56f436b38e368d..daab08a8005a56cca78b6a89735a2c2d3a72236b 100644
--- a/img.js
+++ b/img.js
@@ -578,16 +578,16 @@ class Image {
 
           if(!this.options.dryRun && stat.outputPath) {
             // Should never write when dryRun is true
-            outputFilePromises.push(sharpInstance.toFile(stat.outputPath).then(info => {
+            outputFilePromises.push(processingQueue.add(async () => sharpInstance.toFile(stat.outputPath).then(info => {
               stat.size = info.size;
               return stat;
-            }));
+            })));
           } else {
-            outputFilePromises.push(sharpInstance.toBuffer({ resolveWithObject: true }).then(({ data, info }) => {
+            outputFilePromises.push(processingQueue.add(async () => sharpInstance.toBuffer({ resolveWithObject: true }).then(({ data, info }) => {
               stat.buffer = data;
               stat.size = info.size;
               return stat;
-            }));
+            })));
           }
         }
 
@@ -689,7 +689,7 @@ function queueImage(src, opts) {
 
   debug("In-memory cache miss for %o, options: %o", src, opts);
 
-  let promise = processingQueue.add(async () => {
+  let promise = (async () => {
     if(typeof src === "string" && opts && opts.statsOnly) {
       if(Util.isRemoteUrl(src)) {
         if(!opts.remoteImageMetadata || !opts.remoteImageMetadata.width || !opts.remoteImageMetadata.height) {
@@ -713,7 +713,7 @@ function queueImage(src, opts) {
 
     let input = await img.getInput();
     return img.resize(input);
-  });
+  })();
 
   if(img.options.useCache) {
     memCache.add(key, promise);

[zachleat]

Shipping with 5.0.0-alpha.1, thank you!

[zachleat]

I don’t believe this shipped in v5 due to some test failures—not yet diagnosed.