Merge pull request #17428 from Snuffleupagus/cacheGlobally-CopyImage

Attempt to further reduce re-parsing for globally cached images (PR 11912, 16108 follow-up)

src/core/evaluator.js

@@ -546,6 +546,12 @@ class PartialEvaluator {
   }
 
   _sendImgData(objId, imgData, cacheGlobally = false) {
+    if (
+      (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) &&
+      imgData
+    ) {
+      assert(Number.isInteger(imgData.dataLen), "Expected dataLen to be set.");
+    }
     const transfers = imgData ? [imgData.bitmap || imgData.data.buffer] : null;
 
     if (this.parsingType3Font || cacheGlobally) {
@@ -690,6 +696,10 @@ class PartialEvaluator {
 
       const objId = `mask_${this.idFactory.createObjId()}`;
       operatorList.addDependency(objId);
+
+      imgData.dataLen = imgData.bitmap
+        ? imgData.width * imgData.height * 4
+        : imgData.data.length;
       this._sendImgData(objId, imgData);
 
       args = [
@@ -761,13 +771,15 @@ class PartialEvaluator {
 
     if (this.parsingType3Font) {
       objId = `${this.idFactory.getDocId()}_type3_${objId}`;
-    } else if (imageRef) {
+    } else if (cacheKey && imageRef) {
       cacheGlobally = this.globalImageCache.shouldCache(
         imageRef,
         this.pageIndex
       );
 
       if (cacheGlobally) {
+        assert(!isInline, "Cannot cache an inline image globally.");
+
         objId = `${this.idFactory.getDocId()}_${objId}`;
       }
     }
@@ -775,6 +787,30 @@ class PartialEvaluator {
     // Ensure that the dependency is added before the image is decoded.
     operatorList.addDependency(objId);
     args = [objId, w, h];
+    operatorList.addImageOps(OPS.paintImageXObject, args, optionalContent);
+
+    // For large images, at least 500x500 in size, that we'll cache globally
+    // check if the image is still cached locally on the main-thread to avoid
+    // having to re-parse the image (since that can be slow).
+    if (cacheGlobally && w * h > 250000) {
+      const localLength = await this.handler.sendWithPromise("commonobj", [
+        objId,
+        "CopyLocalImage",
+        { imageRef },
+      ]);
+
+      if (localLength) {
+        this.globalImageCache.setData(imageRef, {
+          objId,
+          fn: OPS.paintImageXObject,
+          args,
+          optionalContent,
+          byteSize: 0, // Temporary entry, to avoid `setData` returning early.
+        });
+        this.globalImageCache.addByteSize(imageRef, localLength);
+        return;
+      }
+    }
 
     PDFImage.buildImage({
       xref: this.xref,
@@ -790,14 +826,14 @@ class PartialEvaluator {
           /* isOffscreenCanvasSupported = */ this.options
             .isOffscreenCanvasSupported
         );
+        imgData.dataLen = imgData.bitmap
+          ? imgData.width * imgData.height * 4
+          : imgData.data.length;
+        imgData.ref = imageRef;
 
-        if (cacheKey && imageRef && cacheGlobally) {
+        if (cacheGlobally) {
-          const length = imgData.bitmap
+          this.globalImageCache.addByteSize(imageRef, imgData.dataLen);
-            ? imgData.width * imgData.height * 4
-            : imgData.data.length;
-          this.globalImageCache.addByteSize(imageRef, length);
         }
-
         return this._sendImgData(objId, imgData, cacheGlobally);
       })
       .catch(reason => {
@@ -806,8 +842,6 @@ class PartialEvaluator {
         return this._sendImgData(objId, /* imgData = */ null, cacheGlobally);
       });
 
-    operatorList.addImageOps(OPS.paintImageXObject, args, optionalContent);
-
     if (cacheKey) {
       const cacheData = {
         fn: OPS.paintImageXObject,
@@ -820,8 +854,6 @@ class PartialEvaluator {
         this._regionalImageCache.set(/* name = */ null, imageRef, cacheData);
 
         if (cacheGlobally) {
-          assert(!isInline, "Cannot cache an inline image globally.");
-
           this.globalImageCache.setData(imageRef, {
             objId,
             fn: OPS.paintImageXObject,

src/display/api.js

@@ -2704,11 +2704,11 @@ class WorkerTransport {
 
     messageHandler.on("commonobj", ([id, type, exportedData]) => {
       if (this.destroyed) {
-        return; // Ignore any pending requests if the worker was terminated.
+        return null; // Ignore any pending requests if the worker was terminated.
       }
 
       if (this.commonObjs.has(id)) {
-        return;
+        return null;
       }
 
       switch (type) {
@@ -2750,6 +2750,23 @@ class WorkerTransport {
               this.commonObjs.resolve(id, font);
             });
           break;
+        case "CopyLocalImage":
+          const { imageRef } = exportedData;
+          assert(imageRef, "The imageRef must be defined.");
+
+          for (const pageProxy of this.#pageCache.values()) {
+            for (const [, data] of pageProxy.objs) {
+              if (data.ref !== imageRef) {
+                continue;
+              }
+              if (!data.dataLen) {
+                return null;
+              }
+              this.commonObjs.resolve(id, structuredClone(data));
+              return data.dataLen;
+            }
+          }
+          break;
         case "FontPath":
         case "Image":
         case "Pattern":
@@ -2758,6 +2775,8 @@ class WorkerTransport {
         default:
           throw new Error(`Got unknown common object type ${type}`);
       }
+
+      return null;
     });
 
     messageHandler.on("obj", ([id, pageIndex, type, imageData]) => {
@@ -2781,18 +2800,8 @@ class WorkerTransport {
           pageProxy.objs.resolve(id, imageData);
 
           // Heuristic that will allow us not to store large data.
-          if (imageData) {
+          if (imageData?.dataLen > MAX_IMAGE_SIZE_TO_CACHE) {
-            let length;
+            pageProxy._maybeCleanupAfterRender = true;
-            if (imageData.bitmap) {
-              const { width, height } = imageData;
-              length = width * height * 4;
-            } else {
-              length = imageData.data?.length || 0;
-            }
-
-            if (length > MAX_IMAGE_SIZE_TO_CACHE) {
-              pageProxy._maybeCleanupAfterRender = true;
-            }
           }
           break;
         case "Pattern":
@@ -3125,7 +3134,7 @@ class PDFObjects {
    */
   has(objId) {
     const obj = this.#objs[objId];
-    return obj?.capability.settled || false;
+    return obj?.capability.settled ?? false;
   }
 
   /**
@@ -3147,6 +3156,17 @@ class PDFObjects {
     }
     this.#objs = Object.create(null);
   }
+
+  *[Symbol.iterator]() {
+    for (const objId in this.#objs) {
+      const { capability, data } = this.#objs[objId];
+
+      if (!capability.settled) {
+        continue;
+      }
+      yield [objId, data];
+    }
+  }
 }
 
 /**
@@ -3165,6 +3185,15 @@ class RenderTask {
      * @type {function}
      */
     this.onContinue = null;
+
+    if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) {
+      // For testing purposes.
+      Object.defineProperty(this, "getOperatorList", {
+        value: () => {
+          return this.#internalRenderTask.operatorList;
+        },
+      });
+    }
   }
 
   /**

test/unit/api_spec.js

@@ -3811,14 +3811,36 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`)
       const loadingTask = getDocument(
         buildGetDocumentParams("issue11878.pdf", {
           isOffscreenCanvasSupported: false,
+          pdfBug: true,
         })
       );
       const pdfDoc = await loadingTask.promise;
-      let firstImgData = null;
+      let checkedCopyLocalImage = false,
+        firstImgData = null,
+        firstStatsOverall = null;
 
       for (let i = 1; i <= pdfDoc.numPages; i++) {
         const pdfPage = await pdfDoc.getPage(i);
-        const opList = await pdfPage.getOperatorList();
+        const viewport = pdfPage.getViewport({ scale: 1 });
+
+        const canvasAndCtx = CanvasFactory.create(
+          viewport.width,
+          viewport.height
+        );
+        const renderTask = pdfPage.render({
+          canvasContext: canvasAndCtx.context,
+          viewport,
+        });
+
+        await renderTask.promise;
+        const opList = renderTask.getOperatorList();
+        // The canvas is no longer necessary, since we only care about
+        // the image-data below.
+        CanvasFactory.destroy(canvasAndCtx);
+
+        const [statsOverall] = pdfPage.stats.times
+          .filter(time => time.name === "Overall")
+          .map(time => time.end - time.start);
 
         const { commonObjs, objs } = pdfPage;
         const imgIndex = opList.fnArray.indexOf(OPS.paintImageXObject);
@@ -3843,6 +3865,7 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`)
         // Ensure that the actual image data is identical for all pages.
         if (i === 1) {
           firstImgData = objs.get(objId);
+          firstStatsOverall = statsOverall;
 
           expect(firstImgData.width).toEqual(EXPECTED_WIDTH);
           expect(firstImgData.height).toEqual(EXPECTED_HEIGHT);
@@ -3854,6 +3877,8 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`)
           const objsPool = i >= NUM_PAGES_THRESHOLD ? commonObjs : objs;
           const currentImgData = objsPool.get(objId);
 
+          expect(currentImgData).not.toBe(firstImgData);
+
           expect(currentImgData.width).toEqual(firstImgData.width);
           expect(currentImgData.height).toEqual(firstImgData.height);
 
@@ -3866,11 +3891,20 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`)
               return value === firstImgData.data[index];
             })
           ).toEqual(true);
+
+          if (i === NUM_PAGES_THRESHOLD) {
+            checkedCopyLocalImage = true;
+            // Ensure that the image was copied in the main-thread, rather
+            // than being re-parsed in the worker-thread (which is slower).
+            expect(statsOverall).toBeLessThan(firstStatsOverall / 5);
+          }
         }
       }
+      expect(checkedCopyLocalImage).toBeTruthy();
 
       await loadingTask.destroy();
       firstImgData = null;
+      firstStatsOverall = null;
     });
 
     it("render for printing, with `printAnnotationStorage` set", async function () {