Convert the remaining functions in src/core/primitives.js to use st…

…andard classes This patch was tested using the PDF file from issue 2618, i.e. https://bug570667.bugzilla-attachments.gnome.org/attachment.cgi?id=226471, with the following manifest file: ``` [ { "id": "issue2618", "file": "../web/pdfs/issue2618.pdf", "md5": "", "rounds": 50, "type": "eq" } ] ``` which gave the following results when comparing this patch against the `master` branch: ``` -- Grouped By browser, stat -- browser | stat | Count | Baseline(ms) | Current(ms) | +/- | % | Result(P<.05) ------- | ------------ | ----- | ------------ | ----------- | --- | ---- | ------------- firefox | Overall | 50 | 3417 | 3426 | 9 | 0.27 | firefox | Page Request | 50 | 1 | 1 | 0 | 5.41 | firefox | Rendering | 50 | 3416 | 3426 | 9 | 0.27 | ``` Based on these results, there's not significant performance regression from using standard classes and this patch should thus be OK.
mozilla · May 12, 2021 · 81b6913 · 81b6913
1 parent dda1a9a
commit 81b6913
Show file tree

Hide file tree

Showing 2 changed files with 139 additions and 145 deletions.
diff --git a/src/core/primitives.js b/src/core/primitives.js
@@ -13,7 +13,7 @@
  * limitations under the License.
  */
 
-import { assert, unreachable } from "../shared/util.js";
+import { assert, shadow, unreachable } from "../shared/util.js";
 import { BaseStream } from "./base_stream.js";
 
 const EOF = {};
@@ -22,21 +22,21 @@ const Name = (function NameClosure() {
   let nameCache = Object.create(null);
 
   // eslint-disable-next-line no-shadow
-  function Name(name) {
-    this.name = name;
-  }
-
-  Name.prototype = {};
+  class Name {
+    constructor(name) {
+      this.name = name;
+    }
 
-  Name.get = function Name_get(name) {
-    const nameValue = nameCache[name];
-    // eslint-disable-next-line no-restricted-syntax
-    return nameValue ? nameValue : (nameCache[name] = new Name(name));
-  };
+    static get(name) {
+      const nameValue = nameCache[name];
+      // eslint-disable-next-line no-restricted-syntax
+      return nameValue ? nameValue : (nameCache[name] = new Name(name));
+    }
 
-  Name._clearCache = function () {
-    nameCache = Object.create(null);
-  };
+    static _clearCache() {
+      nameCache = Object.create(null);
+    }
+  }
 
   return Name;
 })();
@@ -45,142 +45,138 @@ const Cmd = (function CmdClosure() {
   let cmdCache = Object.create(null);
 
   // eslint-disable-next-line no-shadow
-  function Cmd(cmd) {
-    this.cmd = cmd;
-  }
-
-  Cmd.prototype = {};
+  class Cmd {
+    constructor(cmd) {
+      this.cmd = cmd;
+    }
 
-  Cmd.get = function Cmd_get(cmd) {
-    const cmdValue = cmdCache[cmd];
-    // eslint-disable-next-line no-restricted-syntax
-    return cmdValue ? cmdValue : (cmdCache[cmd] = new Cmd(cmd));
-  };
+    static get(cmd) {
+      const cmdValue = cmdCache[cmd];
+      // eslint-disable-next-line no-restricted-syntax
+      return cmdValue ? cmdValue : (cmdCache[cmd] = new Cmd(cmd));
+    }
 
-  Cmd._clearCache = function () {
-    cmdCache = Object.create(null);
-  };
+    static _clearCache() {
+      cmdCache = Object.create(null);
+    }
+  }
 
   return Cmd;
 })();
 
-const Dict = (function DictClosure() {
-  const nonSerializable = function nonSerializableClosure() {
-    return nonSerializable; // creating closure on some variable
-  };
+const nonSerializable = function nonSerializableClosure() {
+  return nonSerializable; // Creating closure on some variable.
+};
 
-  // xref is optional
-  // eslint-disable-next-line no-shadow
-  function Dict(xref) {
+class Dict {
+  constructor(xref = null) {
     // Map should only be used internally, use functions below to access.
     this._map = Object.create(null);
     this.xref = xref;
     this.objId = null;
     this.suppressEncryption = false;
-    this.__nonSerializable__ = nonSerializable; // disable cloning of the Dict
+    this.__nonSerializable__ = nonSerializable; // Disable cloning of the Dict.
   }
 
-  Dict.prototype = {
-    assignXref: function Dict_assignXref(newXref) {
-      this.xref = newXref;
-    },
+  assignXref(newXref) {
+    this.xref = newXref;
+  }
 
-    get size() {
-      return Object.keys(this._map).length;
-    },
+  get size() {
+    return Object.keys(this._map).length;
+  }
 
-    // automatically dereferences Ref objects
-    get(key1, key2, key3) {
-      let value = this._map[key1];
-      if (value === undefined && key2 !== undefined) {
-        value = this._map[key2];
-        if (value === undefined && key3 !== undefined) {
-          value = this._map[key3];
-        }
+  // Automatically dereferences Ref objects.
+  get(key1, key2, key3) {
+    let value = this._map[key1];
+    if (value === undefined && key2 !== undefined) {
+      value = this._map[key2];
+      if (value === undefined && key3 !== undefined) {
+        value = this._map[key3];
       }
-      if (value instanceof Ref && this.xref) {
-        return this.xref.fetch(value, this.suppressEncryption);
-      }
-      return value;
-    },
-
-    // Same as get(), but returns a promise and uses fetchIfRefAsync().
-    async getAsync(key1, key2, key3) {
-      let value = this._map[key1];
-      if (value === undefined && key2 !== undefined) {
-        value = this._map[key2];
-        if (value === undefined && key3 !== undefined) {
-          value = this._map[key3];
-        }
-      }
-      if (value instanceof Ref && this.xref) {
-        return this.xref.fetchAsync(value, this.suppressEncryption);
-      }
-      return value;
-    },
+    }
+    if (value instanceof Ref && this.xref) {
+      return this.xref.fetch(value, this.suppressEncryption);
+    }
+    return value;
+  }
 
-    // Same as get(), but dereferences all elements if the result is an Array.
-    getArray(key1, key2, key3) {
-      let value = this.get(key1, key2, key3);
-      if (!Array.isArray(value) || !this.xref) {
-        return value;
-      }
-      value = value.slice(); // Ensure that we don't modify the Dict data.
-      for (let i = 0, ii = value.length; i < ii; i++) {
-        if (!(value[i] instanceof Ref)) {
-          continue;
-        }
-        value[i] = this.xref.fetch(value[i], this.suppressEncryption);
+  // Same as get(), but returns a promise and uses fetchIfRefAsync().
+  async getAsync(key1, key2, key3) {
+    let value = this._map[key1];
+    if (value === undefined && key2 !== undefined) {
+      value = this._map[key2];
+      if (value === undefined && key3 !== undefined) {
+        value = this._map[key3];
       }
+    }
+    if (value instanceof Ref && this.xref) {
+      return this.xref.fetchAsync(value, this.suppressEncryption);
+    }
+    return value;
+  }
+
+  // Same as get(), but dereferences all elements if the result is an Array.
+  getArray(key1, key2, key3) {
+    let value = this.get(key1, key2, key3);
+    if (!Array.isArray(value) || !this.xref) {
       return value;
-    },
-
-    // no dereferencing
-    getRaw: function Dict_getRaw(key) {
-      return this._map[key];
-    },
-
-    getKeys: function Dict_getKeys() {
-      return Object.keys(this._map);
-    },
-
-    // no dereferencing
-    getRawValues: function Dict_getRawValues() {
-      return Object.values(this._map);
-    },
-
-    set: function Dict_set(key, value) {
-      if (
-        (typeof PDFJSDev === "undefined" ||
-          PDFJSDev.test("!PRODUCTION || TESTING")) &&
-        value === undefined
-      ) {
-        unreachable('Dict.set: The "value" cannot be undefined.');
+    }
+    value = value.slice(); // Ensure that we don't modify the Dict data.
+    for (let i = 0, ii = value.length; i < ii; i++) {
+      if (!(value[i] instanceof Ref)) {
+        continue;
       }
-      this._map[key] = value;
-    },
+      value[i] = this.xref.fetch(value[i], this.suppressEncryption);
+    }
+    return value;
+  }
 
-    has: function Dict_has(key) {
-      return this._map[key] !== undefined;
-    },
+  // No dereferencing.
+  getRaw(key) {
+    return this._map[key];
+  }
 
-    forEach: function Dict_forEach(callback) {
-      for (const key in this._map) {
-        callback(key, this.get(key));
-      }
-    },
-  };
+  getKeys() {
+    return Object.keys(this._map);
+  }
+
+  // No dereferencing.
+  getRawValues() {
+    return Object.values(this._map);
+  }
+
+  set(key, value) {
+    if (
+      (typeof PDFJSDev === "undefined" ||
+        PDFJSDev.test("!PRODUCTION || TESTING")) &&
+      value === undefined
+    ) {
+      unreachable('Dict.set: The "value" cannot be undefined.');
+    }
+    this._map[key] = value;
+  }
+
+  has(key) {
+    return this._map[key] !== undefined;
+  }
 
-  Dict.empty = (function () {
+  forEach(callback) {
+    for (const key in this._map) {
+      callback(key, this.get(key));
+    }
+  }
+
+  static get empty() {
     const emptyDict = new Dict(null);
 
     emptyDict.set = (key, value) => {
       unreachable("Should not call `set` on the empty dictionary.");
     };
-    return emptyDict;
-  })();
+    return shadow(this, "empty", emptyDict);
+  }
 
-  Dict.merge = function ({ xref, dictArray, mergeSubDicts = false }) {
+  static merge({ xref, dictArray, mergeSubDicts = false }) {
     const mergedDict = new Dict(xref);
 
     if (!mergeSubDicts) {
@@ -235,41 +231,39 @@ const Dict = (function DictClosure() {
     properties.clear();
 
     return mergedDict.size > 0 ? mergedDict : Dict.empty;
-  };
-
-  return Dict;
-})();
+  }
+}
 
 const Ref = (function RefClosure() {
   let refCache = Object.create(null);
 
   // eslint-disable-next-line no-shadow
-  function Ref(num, gen) {
-    this.num = num;
-    this.gen = gen;
-  }
+  class Ref {
+    constructor(num, gen) {
+      this.num = num;
+      this.gen = gen;
+    }
 
-  Ref.prototype = {
-    toString: function Ref_toString() {
+    toString() {
       // This function is hot, so we make the string as compact as possible.
       // |this.gen| is almost always zero, so we treat that case specially.
       if (this.gen === 0) {
         return `${this.num}R`;
       }
       return `${this.num}R${this.gen}`;
-    },
-  };
-
-  Ref.get = function (num, gen) {
-    const key = gen === 0 ? `${num}R` : `${num}R${gen}`;
-    const refValue = refCache[key];
-    // eslint-disable-next-line no-restricted-syntax
-    return refValue ? refValue : (refCache[key] = new Ref(num, gen));
-  };
-
-  Ref._clearCache = function () {
-    refCache = Object.create(null);
-  };
+    }
+
+    static get(num, gen) {
+      const key = gen === 0 ? `${num}R` : `${num}R${gen}`;
+      const refValue = refCache[key];
+      // eslint-disable-next-line no-restricted-syntax
+      return refValue ? refValue : (refCache[key] = new Ref(num, gen));
+    }
+
+    static _clearCache() {
+      refCache = Object.create(null);
+    }
+  }
 
   return Ref;
 })();

diff --git a/test/unit/annotation_spec.js b/test/unit/annotation_spec.js
@@ -1192,8 +1192,8 @@ describe("annotation", function () {
       expect(data.url).toBeUndefined();
       expect(data.unsafeUrl).toBeUndefined();
       expect(data.dest).toEqual([
-        { num: 17, gen: 0 },
-        { name: "XYZ" },
+        Ref.get(17, 0),
+        Name.get("XYZ"),
         0,
         841.89,
         null,