-
Notifications
You must be signed in to change notification settings - Fork 10k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Don't store page-level data, in the API, after cleanup has run (bug 1854145) #17106
Conversation
This is based on the nice idea in https://bugzilla.mozilla.org/show_bug.cgi?id=1854145#c12, however please note that I've not (yet) run any tests locally and I've also not really spent a lot of time thinking through these changes. |
3d73a9a
to
1540579
Compare
I just tested locally on a mac and it works pretty well. |
/botio test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you.
…854145) For large/complex images it's possible that the image-data arrives in the API *after* the page has been scrolled out-of-view and thus been cleaned-up. In this case we obviously shouldn't cache such page-level data, since it'll first of all be unused and secondly can increase memory usage *a lot*. Also, ensure that we *immediately* release any `ImageBitmap` data in this case to help reclaim memory faster.
1540579
to
0238cf1
Compare
Given that we abort worker-thread parsing with a delay, to avoid a bunch of re-parsing on zooming and rotation in the viewer, it occurred to me that we shouldn't just check if rendering is "ongoing" but actually ensure that cleanup has successfully run when we decide to ignore any image-data here. /botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/73b4f7f6d027bf2/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/70ae3b870b97db9/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/73b4f7f6d027bf2/output.txt Total script time: 24.52 mins
Image differences available at: http://54.241.84.105:8877/73b4f7f6d027bf2/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/70ae3b870b97db9/output.txt Total script time: 36.93 mins
Image differences available at: http://54.193.163.58:8877/70ae3b870b97db9/reftest-analyzer.html#web=eq.log |
On Windows 11, I scrolled around page 120 in keeping pressed PageDown. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you.
…has run. r=calixte, a=RyanVM Backport of upstream PR: mozilla/pdf.js#17106 Differential Revision: https://phabricator.services.mozilla.com/D195741
…has run. r=calixte, a=RyanVM Backport of upstream PR: mozilla/pdf.js#17106 Differential Revision: https://phabricator.services.mozilla.com/D195741
…has run. r=calixte, a=RyanVM Backport of upstream PR: mozilla/pdf.js#17106 Differential Revision: https://phabricator.services.mozilla.com/D195741
For large/complex images it's possible that the image-data arrives in the API after the page has been scrolled out-of-view and thus been cleaned-up. In this case we obviously shouldn't cache such page-level data, since it'll first of all be unused and secondly can increase memory usage a lot.
Also, ensure that we immediately release any
ImageBitmap
data in this case to help reclaim memory faster.