From 757636d519a3bab8996538e3395a2bf49d2e5357 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 11 May 2021 17:08:26 +0200 Subject: [PATCH] Convert the remaining functions in `src/core/primitives.js` to use standard 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 no significant performance regression from using standard classes and this patch should thus be OK. --- src/core/primitives.js | 274 +++++++++++++++++------------------ test/unit/annotation_spec.js | 4 +- 2 files changed, 136 insertions(+), 142 deletions(-) diff --git a/src/core/primitives.js b/src/core/primitives.js index e58e513f8..ca8e59bdb 100644 --- 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,22 +22,22 @@ const Name = (function NameClosure() { let nameCache = Object.create(null); // eslint-disable-next-line no-shadow - function Name(name) { - this.name = name; + class Name { + constructor(name) { + this.name = name; + } + + static get(name) { + const nameValue = nameCache[name]; + // eslint-disable-next-line no-restricted-syntax + return nameValue ? nameValue : (nameCache[name] = new Name(name)); + } + + static _clearCache() { + nameCache = Object.create(null); + } } - Name.prototype = {}; - - 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)); - }; - - Name._clearCache = function () { - 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; + class Cmd { + constructor(cmd) { + this.cmd = cmd; + } + + static get(cmd) { + const cmdValue = cmdCache[cmd]; + // eslint-disable-next-line no-restricted-syntax + return cmdValue ? cmdValue : (cmdCache[cmd] = new Cmd(cmd)); + } + + static _clearCache() { + cmdCache = Object.create(null); + } } - Cmd.prototype = {}; - - 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)); - }; - - Cmd._clearCache = function () { - 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); + } + 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; + } + + // 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; - }, - - // 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]; - } + } + 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; } - if (value instanceof Ref && this.xref) { - return this.xref.fetchAsync(value, this.suppressEncryption); - } - return value; - }, + value[i] = this.xref.fetch(value[i], 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); - } - return value; - }, + // No dereferencing. + getRaw(key) { + return this._map[key]; + } - // no dereferencing - getRaw: function Dict_getRaw(key) { - return this._map[key]; - }, + getKeys() { + return Object.keys(this._map); + } - getKeys: function Dict_getKeys() { - return Object.keys(this._map); - }, + // No dereferencing. + getRawValues() { + return Object.values(this._map); + } - // no dereferencing - getRawValues: function Dict_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; + } - set: function Dict_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; + } - has: function Dict_has(key) { - return this._map[key] !== undefined; - }, + forEach(callback) { + for (const key in this._map) { + callback(key, this.get(key)); + } + } - forEach: function Dict_forEach(callback) { - for (const key in this._map) { - callback(key, this.get(key)); - } - }, - }; - - Dict.empty = (function () { + 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)); - }; + 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)); + } - Ref._clearCache = function () { - refCache = Object.create(null); - }; + static _clearCache() { + refCache = Object.create(null); + } + } return Ref; })(); diff --git a/test/unit/annotation_spec.js b/test/unit/annotation_spec.js index db94cb54c..b4bbc5f6f 100644 --- 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,