Trigger cleanup, once rendering has finished, in PDFThumbnailView.draw

This patch will help reduce memory usage, especially for longer documents, when the user scrolls around in the thumbnailView (in the sidebar).

Note how the `PDFPageProxy.cleanup` method will, assuming it's safe to do so, release main-thread resources associated with the page. These include things such as e.g. image data (which can be arbitrarily large), and also the operatorList (which can also be quite large).
Hence when pages are evicted from the `PDFPageViewBuffer`, on the `BaseViewer`-instance, the `PDFPageView.destroy` method is invoked which will (among other things) call `PDFPageProxy.cleanup` in the API.

However, looking at the `PDFThumbnailViewer`/`PDFThumbnailView` classes you'll notice that there's no attempt to ever call `PDFPageProxy.cleanup`, which implies that in certain circumstances we'll essentially keep all resources allocated permanently on the `PDFPageProxy`-instances in the API.
In particular, this happens when the users opens the sidebar and starts scrolling around in the thumbnails. Generally speaking you obviously need to keep all thumbnail *images* around, since otherwise the thumbnailView is useless, but there's still room for improvement here.

Please note that the case where a *rendered page* is used to create the thumbnail is (obviously) completely unaffected by the issues described above, and this rather only applies to thumbnails being explicitly rendered by the `PDFThumbnailView.draw` method.
For the latter case, we can fix these issues simply by calling `PDFPageProxy.cleanup` once rendering has finished. To prevent *accidentally* pulling the rug out from under `PDFPageViewBuffer` in the viewer, which expects data to be available, this required adding a couple of new methods[1] to enable checking that it's indeed safe to call `PDFPageProxy.cleanup` from the `PDFThumbnailView.draw` method.

It's really quite fascinating that no one has noticed this issue before, since it's been around since basically "forever".

---
[1] While it should be *very* rare for `PDFThumbnailView.draw` to be called for a pageView that's also in the `PDFPageViewBuffer`, given that pages are rendered before thumbnails and that the *rendered page* is used to create the thumbnail, it can still happen since rendering is asynchronous.
Furthermore, it's also possible for `PDFThumbnailView.setImage` to be disabled, in which case checking the `PDFPageViewBuffer` for active pageViews *really* matters.
This commit is contained in:
Jonas Jenwald 2020-11-12 15:49:29 +01:00
parent 8b5bc8d7f9
commit 4a9994b54c
4 changed files with 59 additions and 0 deletions

View File

@ -115,6 +115,10 @@ function PDFPageViewBuffer(size) {
data.shift().destroy();
}
};
this.has = function (view) {
return data.includes(view);
};
}
function isSameScale(oldScale, newScale) {
@ -1113,6 +1117,32 @@ class BaseViewer {
});
}
/**
* @param {number} pageNumber
*/
isPageCached(pageNumber) {
if (!this.pdfDocument || !this._buffer) {
return false;
}
if (
!(
Number.isInteger(pageNumber) &&
pageNumber > 0 &&
pageNumber <= this.pagesCount
)
) {
console.error(
`${this._name}.isPageCached: "${pageNumber}" is not a valid page.`
);
return false;
}
const pageView = this._pages[pageNumber - 1];
if (!pageView) {
return false;
}
return this._buffer.has(pageView);
}
cleanup() {
for (let i = 0, ii = this._pages.length; i < ii; i++) {
if (

View File

@ -95,6 +95,11 @@ class IPDFLinkService {
* @param {number} pageNumber
*/
isPageVisible(pageNumber) {}
/**
* @param {number} pageNumber
*/
isPageCached(pageNumber) {}
}
/**

View File

@ -458,6 +458,13 @@ class PDFLinkService {
isPageVisible(pageNumber) {
return this.pdfViewer.isPageVisible(pageNumber);
}
/**
* @param {number} pageNumber
*/
isPageCached(pageNumber) {
return this.pdfViewer.isPageCached(pageNumber);
}
}
function isValidExplicitDestination(dest) {
@ -609,6 +616,13 @@ class SimpleLinkService {
isPageVisible(pageNumber) {
return true;
}
/**
* @param {number} pageNumber
*/
isPageCached(pageNumber) {
return true;
}
}
export { PDFLinkService, SimpleLinkService };

View File

@ -363,6 +363,16 @@ class PDFThumbnailView {
finishRenderTask(error);
}
);
// Only trigger cleanup, once rendering has finished, when the current
// pageView is *not* cached on the `BaseViewer`-instance.
resultPromise.finally(() => {
const pageCached = this.linkService.isPageCached(this.id);
if (pageCached) {
return;
}
this.pdfPage?.cleanup();
});
return resultPromise;
}