Skip to content

Commit

Permalink
Transfer, rather than copy, CMap data to the worker-thread
Browse files Browse the repository at this point in the history
It recently occurred to me that the CMap data should be an excellent candidate for transfering.
This will help reduce peak memory usage for PDF documents using CMaps, since transfering of data avoids duplicating it on both the main- and worker-threads.

Unfortunately it's not possible to actually transfer data when *returning* data through `sendWithPromise`, and another solution had to be used.
Initially I looked at using one message for requesting the data, and another message for returning the actual CMap data. While that should have worked, it would have meant adding a lot more complexity particularly on the worker-thread.
Hence the simplest solution, at least in my opinion, is to utilize `sendWithStream` since that makes it *really* easy to transfer the CMap data. (This required PR 11115 to land first, since otherwise CMap fetch errors won't propagate correctly to the worker-thread.)

Please note that the patch *purposely* only changes the API to Worker communication, and not the API *itself* since changing the interface of `CMapReaderFactory` would be a breaking change.
Furthermore, given the relatively small size of the `.bcmap` files (the largest one is smaller than the default range-request size) streaming doesn't really seem necessary either.
  • Loading branch information
Snuffleupagus committed Sep 4, 2019
1 parent 7e37eb4 commit f11a4ba
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 5 deletions.
20 changes: 18 additions & 2 deletions src/core/evaluator.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,24 @@ var PartialEvaluator = (function PartialEvaluatorClosure() {
if (this.builtInCMapCache.has(name)) {
return this.builtInCMapCache.get(name);
}
const data = await this.handler.sendWithPromise('FetchBuiltInCMap',
{ name, });
const readableStream = this.handler.sendWithStream('FetchBuiltInCMap', {
name,
});
const reader = readableStream.getReader();

const data = await new Promise(function(resolve, reject) {
function pump() {
reader.read().then(function({ value, done, }) {
if (done) {
return;
}
resolve(value);
pump();
}, reject);
}
pump();
});

if (data.compressionType !== CMapCompressionType.NONE) {
// Given the size of uncompressed CMaps, only cache compressed ones.
this.builtInCMapCache.set(name, data);
Expand Down
21 changes: 18 additions & 3 deletions src/display/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -2230,11 +2230,26 @@ class WorkerTransport {
});
});

messageHandler.on('FetchBuiltInCMap', (data) => {
messageHandler.on('FetchBuiltInCMap', (data, sink) => {
if (this.destroyed) {
return Promise.reject(new Error('Worker was destroyed'));
sink.error(new Error('Worker was destroyed'));
return;
}
return this.CMapReaderFactory.fetch(data);
let fetched = false;

sink.onPull = () => {
if (fetched) {
sink.close();
return;
}
fetched = true;

this.CMapReaderFactory.fetch(data).then(function(builtInCMap) {
sink.enqueue(builtInCMap, 1, [builtInCMap.cMapData.buffer]);
}).catch(function(reason) {
sink.error(reason);
});
};
});
}

Expand Down

0 comments on commit f11a4ba

Please sign in to comment.