Skip to content

Commit

Permalink
[api-minor] Add a new takeOwnershipOfData option to allow transferr…
Browse files Browse the repository at this point in the history
…ing more data to the worker-thread (bug 1809164)
  • Loading branch information
Snuffleupagus committed Jan 9, 2023
1 parent d6f63b5 commit 811d3f0
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 7 deletions.
18 changes: 11 additions & 7 deletions src/display/api.js
Expand Up @@ -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`.
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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");
}
Expand Down
39 changes: 39 additions & 0 deletions test/unit/api_spec.js
Expand Up @@ -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();
});

Expand Down
5 changes: 5 additions & 0 deletions web/app_options.js
Expand Up @@ -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,
Expand Down

0 comments on commit 811d3f0

Please sign in to comment.