11ty / eleventy-img

Utility to perform build-time image transformations.
https://www.11ty.dev/docs/plugins/image/
436 stars 54 forks source link

Image.concurrency option is confusing #205

Open ryanccn opened 8 months ago

ryanccn commented 8 months ago

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 commented 7 months ago

What’s the patch look like?

ryanccn commented 7 months ago

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 commented 5 months ago

Shipping with 5.0.0-alpha.1, thank you!