1904655 - 389.5 - 2.01% pdfpaint bug1260585.pdf / pdfpaint issue15577.pdf + 427 more (Linux, OSX, Windows) regression on Sat June 15 2024

Status: RESOLVED FIXED

(Firefox :: PDF Viewer, defect)

Description

[Andrej (:aglavic)]

Perfherder has detected a talos performance regression from push 717431608191ac4e40c83e2c4f1e4af9e367cb86. As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
390%	pdfpaint bug1260585.pdf	linux1804-64-shippable-qr	e10s fission stylo webrender	868.04 -> 4,249.10
318%	pdfpaint bug1260585.pdf	macosx1015-64-shippable-qr	e10s fission stylo webrender	840.16 -> 3,515.05
135%	pdfpaint wnv_chinese.pdf	linux1804-64-shippable-qr	e10s fission stylo webrender	360.43 -> 848.23
93%	pdfpaint issue10717.pdf	macosx1015-64-shippable-qr	e10s fission stylo webrender	1,247.76 -> 2,412.40
68%	pdfpaint issue12714.pdf	macosx1015-64-shippable-qr	e10s fission stylo webrender	290.93 -> 489.47
61%	pdfpaint bug1755507.pdf	macosx1015-64-shippable-qr	e10s fission stylo webrender	470.23 -> 758.13
55%	pdfpaint bug1354114.pdf	macosx1015-64-shippable-qr	e10s fission stylo webrender	281.39 -> 435.45
52%	pdfpaint ThuluthFeatures.pdf	macosx1015-64-shippable-qr	e10s fission stylo webrender	240.35 -> 364.22
48%	pdfpaint ohkubo-SS04.pdf	macosx1015-64-shippable-qr	e10s fission stylo webrender	276.75 -> 409.30
47%	pdfpaint bug1755507.pdf	linux1804-64-shippable-qr	e10s fission stylo webrender	613.18 -> 899.75
...	...	...	...	...
2%	pdfpaint issue17215.pdf	macosx1015-64-shippable-qr	e10s fission stylo webrender	202.52 -> 207.48
2%	pdfpaint issue12750.pdf	macosx1015-64-shippable-qr	e10s fission stylo webrender	200.75 -> 205.40
2%	pdfpaint issue7200.pdf	macosx1015-64-shippable-qr	e10s fission stylo webrender	200.53 -> 205.18
2%	pdfpaint issue1905.pdf	linux1804-64-shippable-qr	e10s fission stylo webrender	1,021.06 -> 1,043.35
2%	pdfpaint issue15577.pdf	macosx1015-64-shippable-qr	e10s fission stylo webrender	209.03 -> 213.24

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
29%	pdfpaint bug1795263.pdf	macosx1015-64-shippable-qr	e10s fission stylo webrender	770.62 -> 546.34
8%	pdfpaint issue12295.pdf	linux1804-64-shippable-qr	e10s fission stylo webrender	1,273.02 -> 1,166.90

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests. Please follow our guide to handling regression bugs and let us know your plans within 3 business days, or the patch(es) may be backed out in accordance with our regression policy.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run these tests on try with ./mach try perf --alert 828

For more information on performance sheriffing please see our FAQ.

This is a result of the pdf benchmark update, and more of an FYI rather than a regression you can dis-regard the backout by 3 days, this is as mentioned an FYI

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1902649

Comment 2

[Calixte Denizet (:calixte)]

Most of (probably all) these regressions are due to the fix for bug 1902012.
So disabling accelerated canvas isn't the solution to improve overall performances, enabling it neither...
:jrmuizel, would you have any ideas to try to improve the situation here ?

Comment 3

[Jeff Muizelaar [:jrmuizel]]

How does pdfpaint measure the time?

Comment 4

[Calixte Denizet (:calixte)]

For now it's a bit basic, we just wait for the "pagerendered" event:
https://searchfox.org/mozilla-central/source/testing/talos/talos/pageloader/chrome/lh_pdfpaint.js
and it's dispatched once the rendering is done:
https://github.com/mozilla/pdf.js/blob/7f586182bada2a531bce1ce584d00dadab7ceba7/web/pdf_page_view.js#L866

It includes the time to load the lib, ui, ... so it isn't super accurate and so the number itself doesn't mean anything but the order of magnitude of the difference is something.
That said the differences under 5% are probably meaningless, between 5 and 20%, probably it's a kinda imperceptible, and above 20% it becomes to be a bit annoying.
For sure we must improve the measurement itself, but it won't change that much the difference itself.

Comment 5

[Jonas Jenwald [:Snuffleupagus]]

(In reply to Calixte Denizet (:calixte) from comment #4)

For now it's a bit basic, we just wait for the "pagerendered" event:
[...]
It includes the time to load the lib, ui, ... so it isn't super accurate and so the number itself doesn't mean anything but the order of magnitude of the difference is something.

I don't know how much it'd help overall, but we could include a timestamp: performance.now(), entry when dispatching the "pagerender" event and then compute the effective rendering time as the difference between "pagerendered" and "pagerender". That way we'd remove any variation caused by library/viewer initialization.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The bug is marked as tracked for firefox129 (nightly). We have limited time to fix this, the soft freeze is in 8 days. However, the bug still isn't assigned.

:marco, could you please find an assignee for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 7

[Jeff Muizelaar [:jrmuizel]]

(In reply to Calixte Denizet (:calixte) from comment #4)

For now it's a bit basic, we just wait for the "pagerendered" event:
https://searchfox.org/mozilla-central/source/testing/talos/talos/pageloader/chrome/lh_pdfpaint.js
and it's dispatched once the rendering is done:
https://github.com/mozilla/pdf.js/blob/7f586182bada2a531bce1ce584d00dadab7ceba7/web/pdf_page_view.js#L866

Accelerated canvas is async and so the time to send the canvas commands won't include the actual painting time. Unfortunately, I don't have any great suggestions for how to get better timing info. We should probably do a better recording and exposing this time somehow.

Comment 8

[Calixte Denizet (:calixte)]

I don't really know what to do here... We can fix this regression in setting pdfjs.enableHWA to true but the performances issues we had will come back... I've only the choice between two bad options and I don't know what's the worst, :jrmuizel, do you have any suggestions ?

Comment 9

[Jeff Muizelaar [:jrmuizel]]

I'd recommend setting enableHWA to false for now. The worse case scenario's with hardware acceleration turned on are probably worse than the worse case with it off. We can always try re-enabling it in the future as the accelerated backed matures. Does that seem reasonable Lee?

Comment 10

[Lee Salzman [:lsalzman]]

It's probably worth setting acceleration on on Windows, at least, since that was just the previous status quo before AC2D.

Acceleration is not really a sledgehammer, there are always going to be some tasks it is not good at. While in the future we can try to get better at when and how to fall back in scenarios where acceleration is a net loss, that is not unfortunately going to be a quick project.

Disabling accel will give you guarantees about not falling off an acceleration cliff in the worst case, but it by no means will make the common case faster either.

That is the situation we face. Which is the lesser evil? The cliff or the common case perf?

Comment 11

[Calixte Denizet (:calixte)]

I'll keep the pref pdfjs.enableHWA set to false by default except on Windows.
It induces some perf regressions in some cases but it fixes some rendering issues in some cases so I guess that users would prefer reading something which has been maybe slowly rendered instead of having nothing to read.
I'll take a look on each regression to see if there's something we can do on the pdf.js side to improve the situation.

Comment 12

[Calixte Denizet (:calixte)]

Attachment: Bug 1904655 - Enable hardware acceleration in pdf.js on Windows r=#pdfjs-reviewers

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/78619a70d21e
Enable hardware acceleration in pdf.js on Windows r=pdfjs-reviewers,robwu

Comment 14

[tszentpeteri]

https://hg.mozilla.org/mozilla-central/rev/78619a70d21e

Comment 15

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1902649