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 c3b5209
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 9 deletions.
17 changes: 10 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 @@ -342,6 +345,7 @@ function getDocument(src) {
params.StandardFontDataFactory =
params.StandardFontDataFactory || DefaultStandardFontDataFactory;
params.ignoreErrors = params.stopAtErrors !== true;
params.takeOwnershipOfData = params.takeOwnershipOfData === true;
params.fontExtraProperties = params.fontExtraProperties === true;
params.pdfBug = params.pdfBug === true;
params.enableXfa = params.enableXfa === true;
Expand Down Expand Up @@ -439,6 +443,7 @@ function getDocument(src) {
{
length: params.length,
initialData: params.initialData,
takeOwnershipOfData: params.takeOwnershipOfData,
progressiveDone: params.progressiveDone,
contentDispositionFilename: params.contentDispositionFilename,
disableRange: params.disableRange,
Expand Down Expand Up @@ -513,6 +518,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 +550,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
6 changes: 4 additions & 2 deletions src/display/transport_stream.js
Expand Up @@ -29,9 +29,11 @@ class PDFDataTransportStream {
this._contentDispositionFilename =
params.contentDispositionFilename || null;

const initialData = params.initialData;
const { initialData } = params;
if (initialData?.length > 0) {
const buffer = new Uint8Array(initialData).buffer;
const buffer = params.takeOwnershipOfData
? initialData.buffer
: new Uint8Array(initialData).buffer;
this._queuedChunks.push(buffer);
}

Expand Down
77 changes: 77 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 Expand Up @@ -3257,6 +3296,44 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`)
expect(pdfPage.rotate).toEqual(0);
expect(fetches).toBeGreaterThan(2);

// Check that the TypedArray wasn't transferred.
expect(initialData.length).toEqual(initialDataLength);

await loadingTask.destroy();
});

it("should fetch document info and page using ranges, with `takeOwnershipOfData` set", async function () {
if (isNodeJS) {
pending("Worker is not supported in Node.js.");
}
const initialDataLength = 4000;
let fetches = 0;

const data = await dataPromise;
const initialData = new Uint8Array(data.subarray(0, initialDataLength));
const transport = new PDFDataRangeTransport(data.length, initialData);
transport.requestDataRange = function (begin, end) {
fetches++;
waitSome(function () {
transport.onDataProgress(4000);
transport.onDataRange(begin, data.subarray(begin, end));
});
};

const loadingTask = getDocument({
range: transport,
takeOwnershipOfData: true,
});
const pdfDocument = await loadingTask.promise;
expect(pdfDocument.numPages).toEqual(14);

const pdfPage = await pdfDocument.getPage(10);
expect(pdfPage.rotate).toEqual(0);
expect(fetches).toBeGreaterThan(2);

// Check that the TypedArray was transferred.
expect(initialData.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 c3b5209

Please sign in to comment.