From 811d3f0be7cacdab6e412991e4ac4581ec74ecfc Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 9 Jan 2023 17:24:52 +0100 Subject: [PATCH] [api-minor] Add a new `takeOwnershipOfData` option to allow transferring more data to the worker-thread (bug 1809164) --- src/display/api.js | 18 +++++++++++------- test/unit/api_spec.js | 39 +++++++++++++++++++++++++++++++++++++++ web/app_options.js | 5 +++++ 3 files changed, 55 insertions(+), 7 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index 4efa0a72f72e4..2d9399f7481a8 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -192,6 +192,9 @@ if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("PRODUCTION")) { * @property {number} [maxImageSize] - The maximum allowed image size in total * pixels, i.e. width * height. Images above this value will not be rendered. * Use -1 for no limit, which is also the default value. + * @property {boolean} [takeOwnershipOfData] - Determines if we can transfer + * TypedArrays provided in e.g. the `data` and `initialData` options, which + * will help reduce main-thread memory usage. The default value is `false`. * @property {boolean} [isEvalSupported] - Determines if we can evaluate strings * as JavaScript. Primarily used to improve performance of font rendering, and * when parsing PDF functions. The default value is `true`. @@ -375,6 +378,9 @@ function getDocument(src) { isValidFetchUrl(params.cMapUrl, document.baseURI) && isValidFetchUrl(params.standardFontDataUrl, document.baseURI)); } + if (typeof params.takeOwnershipOfData !== "boolean") { + params.takeOwnershipOfData = false; + } if (typeof params.isEvalSupported !== "boolean") { params.isEvalSupported = true; } @@ -513,6 +519,9 @@ async function _fetchDocument(worker, source, pdfDataRangeTransport, docId) { source.contentDispositionFilename = pdfDataRangeTransport.contentDispositionFilename; } + const transfers = + source.takeOwnershipOfData && source.data ? [source.data.buffer] : null; + const workerId = await worker.messageHandler.sendWithPromise( "GetDocRequest", // Only send the required properties, and *not* the entire `source` object. @@ -542,15 +551,10 @@ async function _fetchDocument(worker, source, pdfDataRangeTransport, docId) { ? source.standardFontDataUrl : null, }, - } + }, + transfers ); - // Release the TypedArray data, when it exists, since it's no longer needed - // on the main-thread *after* it's been sent to the worker-thread. - if (source.data) { - source.data = null; - } - if (worker.destroyed) { throw new Error("Worker was destroyed"); } diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index 933cd6130e3cf..4a17679eb3143 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -193,6 +193,45 @@ describe("api", function () { expect(data[0] instanceof PDFDocumentProxy).toEqual(true); expect(data[1].loaded / data[1].total).toEqual(1); + // Check that the TypedArray wasn't transferred. + expect(typedArrayPdf.length).toEqual(basicApiFileLength); + + await loadingTask.destroy(); + }); + + it("creates pdf doc from TypedArray, with `takeOwnershipOfData` set", async function () { + if (isNodeJS) { + pending("Worker is not supported in Node.js."); + } + const typedArrayPdf = await DefaultFileReaderFactory.fetch({ + path: TEST_PDFS_PATH + basicApiFileName, + }); + + // Sanity check to make sure that we fetched the entire PDF file. + expect(typedArrayPdf instanceof Uint8Array).toEqual(true); + expect(typedArrayPdf.length).toEqual(basicApiFileLength); + + const loadingTask = getDocument({ + data: typedArrayPdf, + takeOwnershipOfData: true, + }); + expect(loadingTask instanceof PDFDocumentLoadingTask).toEqual(true); + + const progressReportedCapability = createPromiseCapability(); + loadingTask.onProgress = function (data) { + progressReportedCapability.resolve(data); + }; + + const data = await Promise.all([ + loadingTask.promise, + progressReportedCapability.promise, + ]); + expect(data[0] instanceof PDFDocumentProxy).toEqual(true); + expect(data[1].loaded / data[1].total).toEqual(1); + + // Check that the TypedArray was transferred. + expect(typedArrayPdf.length).toEqual(0); + await loadingTask.destroy(); }); diff --git a/web/app_options.js b/web/app_options.js index d0cafe2427538..5137aae101fcb 100644 --- a/web/app_options.js +++ b/web/app_options.js @@ -270,6 +270,11 @@ const defaultOptions = { : "../web/standard_fonts/", kind: OptionKind.API, }, + takeOwnershipOfData: { + /** @type {boolean} */ + value: typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL"), + kind: OptionKind.API, + }, verbosity: { /** @type {number} */ value: 1,