Merge pull request #20726 from Snuffleupagus/getOrInsertComputed-fewer-functions

Reduce allocations and function creation when using `getOrInsert` and `getOrInsertComputed`

src/core/core_utils.js

@@ -18,6 +18,7 @@ import {
   assert,
   BaseException,
   hexNumbers,
+  makeArr,
   objectSize,
   stringToPDFString,
   Util,
@@ -669,7 +670,9 @@ function getNewAnnotationsMap(annotationStorage) {
     if (!key.startsWith(AnnotationEditorPrefix)) {
       continue;
     }
-    newAnnotationsByPage.getOrInsert(value.pageIndex, []).push(value);
+    newAnnotationsByPage
+      .getOrInsertComputed(value.pageIndex, makeArr)
+      .push(value);
   }
   return newAnnotationsByPage.size > 0 ? newAnnotationsByPage : null;
 }

src/core/document.js

@@ -20,6 +20,7 @@ import {
   info,
   InvalidPDFException,
   isArrayEqual,
+  makeArr,
   objectSize,
   PageActionEventType,
   RenderingIntentFlag,
@@ -1894,7 +1895,7 @@ class PDFDocument {
       orphanFields.put(fieldRef, parentRef);
     }
 
-    promises.getOrInsert(name, []).push(
+    promises.getOrInsertComputed(name, makeArr).push(
       AnnotationFactory.create(
         xref,
         fieldRef,

src/core/struct_tree.js

@@ -15,6 +15,7 @@
 
 import {
   AnnotationPrefix,
+  makeArr,
   stringToPDFString,
   stringToUTF8String,
   warn,
@@ -450,7 +451,7 @@ class StructTreeRoot {
     for (const element of elements) {
       if (element.structTreeParentId) {
         const id = parseInt(element.structTreeParentId.split("_mc")[1], 10);
-        idToElements.getOrInsert(id, []).push(element);
+        idToElements.getOrInsertComputed(id, makeArr).push(element);
       }
     }
 

src/core/xfa/builder.js

@@ -24,10 +24,10 @@ import {
   $resolvePrototypes,
   $root,
 } from "./symbol_utils.js";
+import { makeArr, warn } from "../../shared/util.js";
 import { NamespaceSetUp } from "./setup.js";
 import { Template } from "./template.js";
 import { UnknownNamespace } from "./unknown.js";
-import { warn } from "../../shared/util.js";
 import { XFAObject } from "./xfa_object.js";
 
 class Root extends XFAObject {
@@ -166,12 +166,9 @@ class Builder {
   _addNamespacePrefix(prefixes) {
     for (const { prefix, value } of prefixes) {
       const namespace = this._searchNamespace(value);
-      let prefixStack = this._namespacePrefixes.get(prefix);
-      if (!prefixStack) {
-        prefixStack = [];
-        this._namespacePrefixes.set(prefix, prefixStack);
-      }
-      prefixStack.push(namespace);
+      this._namespacePrefixes
+        .getOrInsertComputed(prefix, makeArr)
+        .push(namespace);
     }
   }
 

src/core/xfa/fonts.js

@@ -13,9 +13,9 @@
  * limitations under the License.
  */
 
+import { makeObj, warn } from "../../shared/util.js";
 import { $globalData } from "./symbol_utils.js";
 import { stripQuotes } from "./utils.js";
-import { warn } from "../../shared/util.js";
 
 class FontFinder {
   constructor(pdfFonts) {
@@ -48,14 +48,9 @@ class FontFinder {
   addPdfFont(pdfFont) {
     const cssFontInfo = pdfFont.cssFontInfo;
     const name = cssFontInfo.fontFamily;
-    let font = this.fonts.get(name);
-    if (!font) {
-      font = Object.create(null);
-      this.fonts.set(name, font);
-      if (!this.defaultFont) {
-        this.defaultFont = font;
-      }
-    }
+    const font = this.fonts.getOrInsertComputed(name, makeObj);
+    this.defaultFont ??= font;
+
     let property = "";
     const fontWeight = parseFloat(cssFontInfo.fontWeight);
     if (parseFloat(cssFontInfo.italicAngle) !== 0) {

src/core/xfa/som.js

@@ -19,7 +19,7 @@ import {
   $getChildrenByName,
   $getParent,
 } from "./symbol_utils.js";
-import { warn } from "../../shared/util.js";
+import { makeMap, warn } from "../../shared/util.js";
 
 const namePattern = /^[^.[]+/;
 const indexPattern = /^[^\]]+/;
@@ -193,11 +193,7 @@ function searchNode(
       let children, cached;
 
       if (useCache) {
-        cached = somCache.get(node);
-        if (!cached) {
-          cached = new Map();
-          somCache.set(node, cached);
-        }
+        cached = somCache.getOrInsertComputed(node, makeMap);
         children = cached.get(cacheName);
       }
 

src/display/annotation_layer.js

@@ -36,6 +36,7 @@ import {
   AnnotationType,
   FeatureTest,
   LINE_FACTOR,
+  makeArr,
   shadow,
   unreachable,
   Util,
@@ -3877,7 +3878,9 @@ class AnnotationLayer {
         this.#elements.push(element);
 
         if (data.popupRef) {
-          popupToElements.getOrInsert(data.popupRef, []).push(element);
+          popupToElements
+            .getOrInsertComputed(data.popupRef, makeArr)
+            .push(element);
         }
       }
 

src/display/annotation_storage.js

@@ -13,7 +13,7 @@
  * limitations under the License.
  */
 
-import { shadow, unreachable } from "../shared/util.js";
+import { makeMap, shadow, unreachable } from "../shared/util.js";
 import { AnnotationEditor } from "./editor/editor.js";
 import { MurmurHash3_64 } from "../shared/murmurhash3.js";
 
@@ -260,7 +260,7 @@ class AnnotationStorage {
         if (key === "type") {
           continue;
         }
-        const counters = map.getOrInsertComputed(key, () => new Map());
+        const counters = map.getOrInsertComputed(key, makeMap);
         counters.set(val, (counters.get(val) ?? 0) + 1);
       }
     }

src/display/api.js

@@ -25,6 +25,7 @@ import {
   getVerbosityLevel,
   info,
   isNodeJS,
+  makeObj,
   MathClamp,
   RenderingIntentFlag,
   setVerbosityLevel,
@@ -1502,8 +1503,9 @@ class PDFPageProxy {
     optionalContentConfigPromise ||=
       this._transport.getOptionalContentConfig(renderingIntent);
 
-    const intentState = this._intentStates.getOrInsertComputed(cacheKey, () =>
-      Object.create(null)
+    const intentState = this._intentStates.getOrInsertComputed(
+      cacheKey,
+      makeObj
     );
     // Ensure that a pending `streamReader` cancel timeout is always aborted.
     if (intentState.streamReaderCancelTimeout) {
@@ -1675,7 +1677,7 @@ class PDFPageProxy {
     );
     const intentState = this._intentStates.getOrInsertComputed(
       intentArgs.cacheKey,
-      () => Object.create(null)
+      makeObj
     );
     let opListTask;
 

src/display/canvas.js

@@ -22,6 +22,7 @@ import {
   FONT_IDENTITY_MATRIX,
   ImageKind,
   info,
+  makeMap,
   OPS,
   shadow,
   TextRenderingMode,
@@ -989,10 +990,7 @@ class CanvasGraphics {
           : [currentTransform.slice(0, 4), fillColor]
       );
 
-      cache = this._cachedBitmapsMap.getOrInsertComputed(
-        mainKey,
-        () => new Map()
-      );
+      cache = this._cachedBitmapsMap.getOrInsertComputed(mainKey, makeMap);
       const cachedImage = cache.get(cacheKey);
       if (cachedImage && !isPatternFill) {
         const offsetX = Math.round(

src/display/pdf_objects.js

@@ -15,6 +15,11 @@
 
 const INITIAL_DATA = Symbol("INITIAL_DATA");
 
+const dataObj = () => ({
+  ...Promise.withResolvers(),
+  data: INITIAL_DATA,
+});
+
 /**
  * A PDF document and page is built of many objects. E.g. there are objects for
  * fonts, images, rendering code, etc. These objects may get processed inside of
@@ -30,10 +35,7 @@ class PDFObjects {
    * @returns {Object}
    */
   #ensureObj(objId) {
-    return this.#objs.getOrInsertComputed(objId, () => ({
-      ...Promise.withResolvers(),
-      data: INITIAL_DATA,
-    }));
+    return this.#objs.getOrInsertComputed(objId, dataObj);
   }
 
   /**

src/pdf.js

@@ -33,6 +33,9 @@ import {
   getUuid,
   ImageKind,
   InvalidPDFException,
+  makeArr,
+  makeMap,
+  makeObj,
   MathClamp,
   normalizeUnicode,
   OPS,
@@ -123,6 +126,9 @@ globalThis.pdfjsLib = {
   isDataScheme,
   isPdfFile,
   isValidExplicitDest,
+  makeArr,
+  makeMap,
+  makeObj,
   MathClamp,
   noContextMenu,
   normalizeUnicode,
@@ -182,6 +188,9 @@ export {
   isDataScheme,
   isPdfFile,
   isValidExplicitDest,
+  makeArr,
+  makeMap,
+  makeObj,
   MathClamp,
   noContextMenu,
   normalizeUnicode,

src/shared/util.js

@@ -1234,6 +1234,12 @@ function _isValidExplicitDest(validRef, validName, dest) {
   return true;
 }
 
+// Helpers for simple `Map.prototype.getOrInsertComputed()` invocations,
+// to avoid duplicate function creation.
+const makeArr = () => [];
+const makeMap = () => new Map();
+const makeObj = () => Object.create(null);
+
 // TODO: Replace all occurrences of this function with `Math.clamp` once
 //       https://github.com/tc39/proposal-math-clamp/ is generally available.
 function MathClamp(v, min, max) {
@@ -1331,6 +1337,9 @@ export {
   isNodeJS,
   LINE_DESCENT_FACTOR,
   LINE_FACTOR,
+  makeArr,
+  makeMap,
+  makeObj,
   MathClamp,
   MeshFigureType,
   normalizeUnicode,

test/unit/pdf_spec.js

@@ -24,6 +24,9 @@ import {
   getUuid,
   ImageKind,
   InvalidPDFException,
+  makeArr,
+  makeMap,
+  makeObj,
   MathClamp,
   normalizeUnicode,
   OPS,
@@ -107,6 +110,9 @@ const expectedAPI = Object.freeze({
   isDataScheme,
   isPdfFile,
   isValidExplicitDest,
+  makeArr,
+  makeMap,
+  makeObj,
   MathClamp,
   noContextMenu,
   normalizeUnicode,

web/pdf_viewer.js

@@ -33,6 +33,7 @@ import {
   AnnotationEditorType,
   AnnotationEditorUIManager,
   AnnotationMode,
+  makeArr,
   MathClamp,
   PermissionFlag,
   PixelsPerInch,
@@ -2281,7 +2282,7 @@ class PDFViewer {
           if (percent === 0 || widthPercent < 100) {
             continue;
           }
-          pageLayout.getOrInsert(y, []).push(id);
+          pageLayout.getOrInsertComputed(y, makeArr).push(id);
         }
         // Find the row of the current page.
         for (const yArray of pageLayout.values()) {

web/pdfjs.js

@@ -44,6 +44,9 @@ const {
   isDataScheme,
   isPdfFile,
   isValidExplicitDest,
+  makeArr,
+  makeMap,
+  makeObj,
   MathClamp,
   noContextMenu,
   normalizeUnicode,
@@ -103,6 +106,9 @@ export {
   isDataScheme,
   isPdfFile,
   isValidExplicitDest,
+  makeArr,
+  makeMap,
+  makeObj,
   MathClamp,
   noContextMenu,
   normalizeUnicode,

web/struct_tree_layer_builder.js

@@ -15,7 +15,7 @@
 
 /** @typedef {import("../src/display/api").PDFPageProxy} PDFPageProxy */
 
-import { FeatureTest, shadow } from "pdfjs-lib";
+import { FeatureTest, makeMap, shadow } from "pdfjs-lib";
 import { removeNullCharacters } from "./ui_utils.js";
 
 const PDF_ROLE_TO_HTML_ROLE = {
@@ -251,12 +251,9 @@ class StructTreeLayerBuilder {
       const label = removeNullCharacters(alt);
       for (const child of structElement.children) {
         if (child.type === "annotation") {
-          let attrs = this.#elementAttributes.get(child.id);
-          if (!attrs) {
-            attrs = new Map();
-            this.#elementAttributes.set(child.id, attrs);
-          }
-          attrs.set("aria-label", label);
+          this.#elementAttributes
+            .getOrInsertComputed(child.id, makeMap)
+            .set("aria-label", label);
           added = true;
         }
       }