From e7a7f02f4c1a4f650c3fae1c933a8641bef4eef4 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 4 Mar 2023 09:42:32 +0100 Subject: [PATCH 1/3] Convert a couple of fields/methods into properly private ones in `PDFPageProxy` These were always intended to be *private*, so let's use modern JS features to actually enforce that. --- src/display/api.js | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index 72054e91b..66c22c437 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -1262,6 +1262,8 @@ class PDFDocumentProxy { * Proxy to a `PDFPage` in the worker thread. */ class PDFPageProxy { + #pendingCleanup = false; + constructor(pageIndex, pageInfo, transport, pdfBug = false) { this._pageIndex = pageIndex; this._pageInfo = pageInfo; @@ -1273,7 +1275,6 @@ class PDFPageProxy { this.objs = new PDFObjects(); this.cleanupAfterRender = false; - this.pendingCleanup = false; this._intentStates = new Map(); this.destroyed = false; } @@ -1414,7 +1415,7 @@ class PDFPageProxy { ); // If there was a pending destroy, cancel it so no cleanup happens during // this call to render. - this.pendingCleanup = false; + this.#pendingCleanup = false; if (!optionalContentConfigPromise) { optionalContentConfigPromise = this._transport.getOptionalContentConfig(); @@ -1457,9 +1458,9 @@ class PDFPageProxy { // Attempt to reduce memory usage during *printing*, by always running // cleanup once rendering has finished (regardless of cleanupAfterRender). if (this.cleanupAfterRender || intentPrint) { - this.pendingCleanup = true; + this.#pendingCleanup = true; } - this._tryCleanup(); + this.#tryCleanup(); if (error) { internalRenderTask.capability.reject(error); @@ -1509,7 +1510,7 @@ class PDFPageProxy { { transparency, isOffscreenCanvasSupported }, optionalContentConfig, ]) => { - if (this.pendingCleanup) { + if (this.#pendingCleanup) { complete(); return; } @@ -1681,7 +1682,7 @@ class PDFPageProxy { } } this.objs.clear(); - this.pendingCleanup = false; + this.#pendingCleanup = false; return Promise.all(waitOn); } @@ -1693,16 +1694,15 @@ class PDFPageProxy { * @returns {boolean} Indicates if clean-up was successfully run. */ cleanup(resetStats = false) { - this.pendingCleanup = true; - return this._tryCleanup(resetStats); + this.#pendingCleanup = true; + return this.#tryCleanup(resetStats); } /** * Attempts to clean up if rendering is in a state where that's possible. - * @private */ - _tryCleanup(resetStats = false) { - if (!this.pendingCleanup) { + #tryCleanup(resetStats = false) { + if (!this.#pendingCleanup) { return false; } for (const { renderTasks, operatorList } of this._intentStates.values()) { @@ -1716,7 +1716,7 @@ class PDFPageProxy { if (resetStats && this._stats) { this._stats = new StatTimer(); } - this.pendingCleanup = false; + this.#pendingCleanup = false; return true; } @@ -1756,7 +1756,7 @@ class PDFPageProxy { } if (operatorListChunk.lastChunk) { - this._tryCleanup(); + this.#tryCleanup(); } } @@ -1814,7 +1814,7 @@ class PDFPageProxy { for (const internalRenderTask of intentState.renderTasks) { internalRenderTask.operatorListChanged(); } - this._tryCleanup(); + this.#tryCleanup(); } if (intentState.displayReadyCapability) { From 15d9faba57a1d8994e39dbba0e3b41da5abf7ef4 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 4 Mar 2023 11:05:52 +0100 Subject: [PATCH 2/3] Slightly delay cleanup, after rendering, in documents with large images Currently in PDF documents with large images we immediately cleanup once rendering has finished, in order to reduce memory-usage. Normally that shouldn't be a big problem, however when e.g. repeated zooming happens in the viewer that could easily lead to a lot of wasted resources (and waiting). Hence this patch, which introduces a new `PDFPageProxy` method that will slightly delay cleanup after rendering. --- src/display/api.js | 57 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 43 insertions(+), 14 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index 66c22c437..e44dd78ba 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -66,6 +66,7 @@ import { XfaText } from "./xfa_text.js"; const DEFAULT_RANGE_CHUNK_SIZE = 65536; // 2^16 = 65536 const RENDERING_CANCELLED_TIMEOUT = 100; // ms +const DELAYED_CLEANUP_TIMEOUT = 5000; // ms let DefaultCanvasFactory = DOMCanvasFactory; let DefaultCMapReaderFactory = DOMCMapReaderFactory; @@ -1262,6 +1263,8 @@ class PDFDocumentProxy { * Proxy to a `PDFPage` in the worker thread. */ class PDFPageProxy { + #delayedCleanupTimeout = null; + #pendingCleanup = false; constructor(pageIndex, pageInfo, transport, pdfBug = false) { @@ -1274,7 +1277,7 @@ class PDFPageProxy { this.commonObjs = transport.commonObjs; this.objs = new PDFObjects(); - this.cleanupAfterRender = false; + this._maybeCleanupAfterRender = false; this._intentStates = new Map(); this.destroyed = false; } @@ -1414,8 +1417,10 @@ class PDFPageProxy { printAnnotationStorage ); // If there was a pending destroy, cancel it so no cleanup happens during - // this call to render. + // this call to render... this.#pendingCleanup = false; + // ... and ensure that a delayed cleanup is always aborted. + this.#abortDelayedCleanup(); if (!optionalContentConfigPromise) { optionalContentConfigPromise = this._transport.getOptionalContentConfig(); @@ -1456,11 +1461,11 @@ class PDFPageProxy { intentState.renderTasks.delete(internalRenderTask); // Attempt to reduce memory usage during *printing*, by always running - // cleanup once rendering has finished (regardless of cleanupAfterRender). - if (this.cleanupAfterRender || intentPrint) { + // cleanup immediately once rendering has finished. + if (this._maybeCleanupAfterRender || intentPrint) { this.#pendingCleanup = true; } - this.#tryCleanup(); + this.#tryCleanup(/* delayed = */ !intentPrint); if (error) { internalRenderTask.capability.reject(error); @@ -1683,6 +1688,8 @@ class PDFPageProxy { } this.objs.clear(); this.#pendingCleanup = false; + this.#abortDelayedCleanup(); + return Promise.all(waitOn); } @@ -1695,31 +1702,53 @@ class PDFPageProxy { */ cleanup(resetStats = false) { this.#pendingCleanup = true; - return this.#tryCleanup(resetStats); + const success = this.#tryCleanup(/* delayed = */ false); + + if (resetStats && success) { + this._stats &&= new StatTimer(); + } + return success; } /** * Attempts to clean up if rendering is in a state where that's possible. + * @param {boolean} [delayed] - Delay the cleanup, to e.g. improve zooming + * performance in documents with large images. + * The default value is `false`. + * @returns {boolean} Indicates if clean-up was successfully run. */ - #tryCleanup(resetStats = false) { + #tryCleanup(delayed = false) { + this.#abortDelayedCleanup(); + if (!this.#pendingCleanup) { return false; } + if (delayed) { + this.#delayedCleanupTimeout = setTimeout(() => { + this.#delayedCleanupTimeout = null; + this.#tryCleanup(/* delayed = */ false); + }, DELAYED_CLEANUP_TIMEOUT); + + return false; + } for (const { renderTasks, operatorList } of this._intentStates.values()) { if (renderTasks.size > 0 || !operatorList.lastChunk) { return false; } } - this._intentStates.clear(); this.objs.clear(); - if (resetStats && this._stats) { - this._stats = new StatTimer(); - } this.#pendingCleanup = false; return true; } + #abortDelayedCleanup() { + if (this.#delayedCleanupTimeout) { + clearTimeout(this.#delayedCleanupTimeout); + this.#delayedCleanupTimeout = null; + } + } + /** * @private */ @@ -1756,7 +1785,7 @@ class PDFPageProxy { } if (operatorListChunk.lastChunk) { - this.#tryCleanup(); + this.#tryCleanup(/* delayed = */ true); } } @@ -1814,7 +1843,7 @@ class PDFPageProxy { for (const internalRenderTask of intentState.renderTasks) { internalRenderTask.operatorListChanged(); } - this.#tryCleanup(); + this.#tryCleanup(/* delayed = */ true); } if (intentState.displayReadyCapability) { @@ -2793,7 +2822,7 @@ class WorkerTransport { } if (length > MAX_IMAGE_SIZE_TO_STORE) { - pageProxy.cleanupAfterRender = true; + pageProxy._maybeCleanupAfterRender = true; } } break; From c0671ac13341dde2b0378771dc846cad84352a9c Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 4 Mar 2023 11:22:59 +0100 Subject: [PATCH 3/3] Slightly increase the maximum image sizes that we'll cache The current value originated in PR 2317, and in the decade that have passed the amount of RAM available in (most) devices should have increased a fair bit. Nowadays we also do a much better job of detecting repeated images at both the page- and document-level, which helps reduce overall memory-usage in many documents. Finally the constant is also moved into the `src/shared/util.js` file, since it was implicitly used on both the main- and worker-thread previously. --- src/core/image_utils.js | 10 ++++++++-- src/display/api.js | 4 ++-- src/shared/util.js | 3 +++ 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/core/image_utils.js b/src/core/image_utils.js index 8cef91dc0..1dfb5b740 100644 --- a/src/core/image_utils.js +++ b/src/core/image_utils.js @@ -13,7 +13,13 @@ * limitations under the License. */ -import { assert, shadow, unreachable, warn } from "../shared/util.js"; +import { + assert, + MAX_IMAGE_SIZE_TO_CACHE, + shadow, + unreachable, + warn, +} from "../shared/util.js"; import { RefSetCache } from "./primitives.js"; class BaseLocalCache { @@ -160,7 +166,7 @@ class GlobalImageCache { } static get MAX_BYTE_SIZE() { - return shadow(this, "MAX_BYTE_SIZE", /* Forty megabytes = */ 40e6); + return shadow(this, "MAX_BYTE_SIZE", 5 * MAX_IMAGE_SIZE_TO_CACHE); } constructor() { diff --git a/src/display/api.js b/src/display/api.js index e44dd78ba..00adb9044 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -26,6 +26,7 @@ import { info, InvalidPDFException, isArrayBuffer, + MAX_IMAGE_SIZE_TO_CACHE, MissingPDFException, PasswordException, RenderingIntentFlag, @@ -2811,7 +2812,6 @@ class WorkerTransport { pageProxy.objs.resolve(id, imageData); // Heuristic that will allow us not to store large data. - const MAX_IMAGE_SIZE_TO_STORE = 8000000; if (imageData) { let length; if (imageData.bitmap) { @@ -2821,7 +2821,7 @@ class WorkerTransport { length = imageData.data?.length || 0; } - if (length > MAX_IMAGE_SIZE_TO_STORE) { + if (length > MAX_IMAGE_SIZE_TO_CACHE) { pageProxy._maybeCleanupAfterRender = true; } } diff --git a/src/shared/util.js b/src/shared/util.js index d500aa9a7..873e26e0c 100644 --- a/src/shared/util.js +++ b/src/shared/util.js @@ -26,6 +26,8 @@ if ( const IDENTITY_MATRIX = [1, 0, 0, 1, 0, 0]; const FONT_IDENTITY_MATRIX = [0.001, 0, 0, 0.001, 0, 0]; +const MAX_IMAGE_SIZE_TO_CACHE = 10e6; // Ten megabytes. + // Represent the percentage of the height of a single-line field over // the font size. Acrobat seems to use this value. const LINE_FACTOR = 1.35; @@ -1060,6 +1062,7 @@ export { isArrayEqual, LINE_DESCENT_FACTOR, LINE_FACTOR, + MAX_IMAGE_SIZE_TO_CACHE, MissingPDFException, objectFromMap, objectSize,