From 2de03a7d9179eff7d622408411cff43b6006228a Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 4 Feb 2023 10:01:16 +0100 Subject: [PATCH] Improve how we cache Promises in `WorkerTransport` A number of methods have their Promises cached, to avoid repeated worker round-trips, since they're expected to be called more than once from the default viewer. The way that the caching is currently implemented means that we need to remember to manually clear these Promises on document cleanup/destruction, and it'd be nice to avoid that. With this patch the relevant Promises are now instead placed in just one `Map`, which is easy to clear, and a new helper method is also introduced to reduce duplication for *simple* `WorkerTransport` methods. --- src/display/api.js | 44 +++++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index b935f4f4c..7e896792d 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -2332,12 +2332,12 @@ class PDFWorker { * @ignore */ class WorkerTransport { + #methodPromises = new Map(); + #pageCache = new Map(); #pagePromises = new Map(); - #metadataPromise = null; - constructor(messageHandler, loadingTask, networkStream, params) { this.messageHandler = messageHandler; this.loadingTask = loadingTask; @@ -2371,6 +2371,17 @@ class WorkerTransport { this.setupMessageHandler(); } + #cacheSimpleMethod(name, data = null) { + const cachedPromise = this.#methodPromises.get(name); + if (cachedPromise) { + return cachedPromise; + } + const promise = this.messageHandler.sendWithPromise(name, data); + + this.#methodPromises.set(name, promise); + return promise; + } + get annotationStorage() { return shadow(this, "annotationStorage", new AnnotationStorage()); } @@ -2467,9 +2478,7 @@ class WorkerTransport { Promise.all(waitOn).then(() => { this.commonObjs.clear(); this.fontLoader.clear(); - this.#metadataPromise = null; - this._getFieldObjectsPromise = null; - this._hasJSActionsPromise = null; + this.#methodPromises.clear(); if (this._networkStream) { this._networkStream.cancelAllRequests( @@ -2934,15 +2943,11 @@ class WorkerTransport { } getFieldObjects() { - return (this._getFieldObjectsPromise ||= - this.messageHandler.sendWithPromise("GetFieldObjects", null)); + return this.#cacheSimpleMethod("GetFieldObjects"); } hasJSActions() { - return (this._hasJSActionsPromise ||= this.messageHandler.sendWithPromise( - "HasJSActions", - null - )); + return this.#cacheSimpleMethod("HasJSActions"); } getCalculationOrderIds() { @@ -3023,8 +3028,13 @@ class WorkerTransport { } getMetadata() { - return (this.#metadataPromise ||= this.messageHandler - .sendWithPromise("GetMetadata", null) + const name = "GetMetadata", + cachedPromise = this.#methodPromises.get(name); + if (cachedPromise) { + return cachedPromise; + } + const promise = this.messageHandler + .sendWithPromise(name, null) .then(results => { return { info: results[0], @@ -3032,7 +3042,9 @@ class WorkerTransport { contentDispositionFilename: this._fullReader?.filename ?? null, contentLength: this._fullReader?.contentLength ?? null, }; - })); + }); + this.#methodPromises.set(name, promise); + return promise; } getMarkInfo() { @@ -3058,9 +3070,7 @@ class WorkerTransport { if (!keepLoadedFonts) { this.fontLoader.clear(); } - this.#metadataPromise = null; - this._getFieldObjectsPromise = null; - this._hasJSActionsPromise = null; + this.#methodPromises.clear(); } get loadingParams() {