Change WorkerTransport.pageCache from an Array to a Map

Given that not all pages necessarily are being accessed, or that the pages may be accessed out of order, using a `Map` seems like a more appropriate data-structure here.
For one thing, this simplifies iteration since we no longer have to worry about/check if `pageCache`-entries are undefined (which will happen for *sparse* `Array`s).

Of particular note is that we're no longer attempting to "null" the `pageCache`-entry from within the `PDFPageProxy._destroy`-method. Given that *synchronous* JavaScript will always run to completion[1] and that we're looping through all pages in `WorkerTransport.destroy` and immediately clear the cache afterwards, that code did/does not really make a lot of sense (as far as I can tell).

Finally, also changes the `pageCache` to a *private* property since it's not supposed to be accessed from the "outside".

---
[1] Unless there are errors, of course.
This commit is contained in:
Jonas Jenwald 2021-12-09 11:40:14 +01:00
parent 97dc048e56
commit c5525dcb69

View File

@ -1666,7 +1666,6 @@ class PDFPageProxy {
*/
_destroy() {
this.destroyed = true;
this._transport.pageCache[this._pageIndex] = null;
const waitOn = [];
for (const intentState of this._intentStates.values()) {
@ -2403,6 +2402,8 @@ if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("GENERIC")) {
class WorkerTransport {
#docStats = null;
#pageCache = new Map();
constructor(messageHandler, loadingTask, networkStream, params) {
this.messageHandler = messageHandler;
this.loadingTask = loadingTask;
@ -2433,7 +2434,6 @@ class WorkerTransport {
this._fullReader = null;
this._lastProgress = null;
this.pageCache = [];
this.pagePromises = [];
this.downloadInfoCapability = createPromiseCapability();
@ -2514,12 +2514,10 @@ class WorkerTransport {
const waitOn = [];
// We need to wait for all renderings to be completed, e.g.
// timeout/rAF can take a long time.
for (const page of this.pageCache) {
if (page) {
waitOn.push(page._destroy());
}
for (const page of this.#pageCache.values()) {
waitOn.push(page._destroy());
}
this.pageCache.length = 0;
this.#pageCache.clear();
this.pagePromises.length = 0;
// Allow `AnnotationStorage`-related clean-up when destroying the document.
if (this.hasOwnProperty("annotationStorage")) {
@ -2751,16 +2749,15 @@ class WorkerTransport {
return; // Ignore any pending requests if the worker was terminated.
}
const page = this.pageCache[data.pageIndex];
const page = this.#pageCache.get(data.pageIndex);
page._startRenderPage(data.transparency, data.cacheKey);
});
messageHandler.on("commonobj", data => {
messageHandler.on("commonobj", ([id, type, exportedData]) => {
if (this.destroyed) {
return; // Ignore any pending requests if the worker was terminated.
}
const [id, type, exportedData] = data;
if (this.commonObjs.has(id)) {
return;
}
@ -2818,14 +2815,13 @@ class WorkerTransport {
}
});
messageHandler.on("obj", data => {
messageHandler.on("obj", ([id, pageIndex, type, imageData]) => {
if (this.destroyed) {
// Ignore any pending requests if the worker was terminated.
return;
}
const [id, pageIndex, type, imageData] = data;
const pageProxy = this.pageCache[pageIndex];
const pageProxy = this.#pageCache.get(pageIndex);
if (pageProxy.objs.has(id)) {
return;
}
@ -2943,7 +2939,7 @@ class WorkerTransport {
this._params.ownerDocument,
this._params.pdfBug
);
this.pageCache[pageIndex] = page;
this.#pageCache.set(pageIndex, page);
return page;
});
this.pagePromises[pageIndex] = promise;
@ -3088,15 +3084,13 @@ class WorkerTransport {
if (this.destroyed) {
return; // No need to manually clean-up when destruction has started.
}
for (let i = 0, ii = this.pageCache.length; i < ii; i++) {
const page = this.pageCache[i];
if (!page) {
continue;
}
for (const page of this.#pageCache.values()) {
const cleanupSuccessful = page.cleanup();
if (!cleanupSuccessful) {
throw new Error(`startCleanup: Page ${i + 1} is currently rendering.`);
throw new Error(
`startCleanup: Page ${page.pageNumber} is currently rendering.`
);
}
}
this.commonObjs.clear();