From 7273795eb63962912ad45b84b8e05a1074f90335 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 16 Mar 2019 12:09:14 +0100 Subject: [PATCH 1/2] Actually transfer eligible ImageMask data, rather than always copying it By transfering `ArrayBuffer`s you can avoid having two copies of the same data, i.e. one copy on each of the worker/main-thread, for data that's used only *once* on the worker-thread. Note how the code in [`PDFImage.createMask`](https://github.com/mozilla/pdf.js/blob/80135378cadd98b55a835446f0857e4bc30524e0/src/core/image.js#L284-L285) goes to great lengths to actually enable tranfering of the image data. However in [`PartialEvaluator.buildPaintImageXObject`](https://github.com/mozilla/pdf.js/blob/80135378cadd98b55a835446f0857e4bc30524e0/src/core/evaluator.js#L336) the `cached` property is always set to `true`, which disqualifies the image data from being transfered; see [`getTransfers`](https://github.com/mozilla/pdf.js/blob/80135378cadd98b55a835446f0857e4bc30524e0/src/core/operator_list.js#L552-L554). For most ImageMask data this patch won't matter, since images found in the `/Resources -> /XObject` dictionary will always be indexed by name. However for *inline* images which contains ImageMask data, where only "small" images are cached (in both `parser.js` and `evaluator.js`), the current code will result in some unnecessary memory usage. --- src/core/evaluator.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/evaluator.js b/src/core/evaluator.js index dd13aeb64..06951cdee 100644 --- a/src/core/evaluator.js +++ b/src/core/evaluator.js @@ -333,8 +333,9 @@ var PartialEvaluator = (function PartialEvaluatorClosure() { imageIsFromDecodeStream: image instanceof DecodeStream, inverseDecode: (!!decode && decode[0] > 0), }); - imgData.cached = true; + imgData.cached = !!cacheKey; args = [imgData]; + operatorList.addOp(OPS.paintImageMaskXObject, args); if (cacheKey) { imageCache[cacheKey] = { From 56eeeea1dca9b9a4ee4a43e74614e91e0a518695 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 16 Mar 2019 12:15:22 +0100 Subject: [PATCH 2/2] Re-factor the `getTransfers` helper function into a "private" getter method on the `OperatorList` This function is currently called with the `OperatorList` instance as its argument, hence I cannot think of any good reason for not just moving it into the `OperatorList` properly. (This will also help with other planned changes regarding the `ImageCache` functionality.) --- src/core/operator_list.js | 56 +++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/src/core/operator_list.js b/src/core/operator_list.js index 3128ff6fb..b5690843d 100644 --- a/src/core/operator_list.js +++ b/src/core/operator_list.js @@ -534,30 +534,6 @@ var OperatorList = (function OperatorListClosure() { var CHUNK_SIZE = 1000; var CHUNK_SIZE_ABOUT = CHUNK_SIZE - 5; // close to chunk size - function getTransfers(queue) { - var transfers = []; - var fnArray = queue.fnArray, argsArray = queue.argsArray; - for (var i = 0, ii = queue.length; i < ii; i++) { - switch (fnArray[i]) { - case OPS.paintInlineImageXObject: - case OPS.paintInlineImageXObjectGroup: - case OPS.paintImageMaskXObject: - var arg = argsArray[i][0]; // first param in imgData - - if (typeof PDFJSDev === 'undefined' || - PDFJSDev.test('!PRODUCTION || TESTING')) { - assert(arg.data instanceof Uint8ClampedArray, - 'OperatorList - getTransfers: Unsupported "arg.data" type.'); - } - if (!arg.cached) { - transfers.push(arg.data.buffer); - } - break; - } - } - return transfers; - } - function OperatorList(intent, messageHandler, pageIndex) { this.messageHandler = messageHandler; this.fnArray = []; @@ -630,10 +606,33 @@ var OperatorList = (function OperatorListClosure() { }; }, - flush(lastChunk) { + get _transfers() { + const transfers = []; + const { fnArray, argsArray, length, } = this; + for (let i = 0; i < length; i++) { + switch (fnArray[i]) { + case OPS.paintInlineImageXObject: + case OPS.paintInlineImageXObjectGroup: + case OPS.paintImageMaskXObject: + const arg = argsArray[i][0]; // first param in imgData + + if (typeof PDFJSDev === 'undefined' || + PDFJSDev.test('!PRODUCTION || TESTING')) { + assert(arg.data instanceof Uint8ClampedArray, + 'OperatorList._transfers: Unsupported "arg.data" type.'); + } + if (!arg.cached) { + transfers.push(arg.data.buffer); + } + break; + } + } + return transfers; + }, + + flush(lastChunk = false) { this.optimizer.flush(); - var transfers = getTransfers(this); - var length = this.length; + const length = this.length; this._totalLength += length; this.messageHandler.send('RenderPageChunk', { @@ -645,7 +644,8 @@ var OperatorList = (function OperatorListClosure() { }, pageIndex: this.pageIndex, intent: this.intent, - }, transfers); + }, this._transfers); + this.dependencies = Object.create(null); this.fnArray.length = 0; this.argsArray.length = 0;