Drawing performance regression after merging bug 1464032

RESOLVED FIXED in Firefox 69

Status

()

defect
--
major
RESOLVED FIXED
Last month
2 days ago

People

(Reporter: kasper93, Assigned: bobowen)

Tracking

(Regression, {regression})

69 Branch
mozilla69
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox67 unaffected, firefox68 unaffected, firefox69 fixed)

Details

Attachments

(2 attachments)

Posted file about_support.txt

Hi,

Since few day Firefox is unusable slow to draw. Here is profile https://perfht.ml/2RkocNR (from few seconds on vsynctester.com) I am attaching also about_support.txt with information about my config. I am currently on a trip so can't really test with other platforms, but on this Intel UHD 620 it is slideshow... instead of motion. I checked also on some very low end NVIDIA OEM GPU and result was similar. But I didn't compare to higher end hardware.

I bisected recent builds and here is offending change:
Bug 1464032 Part 15: Spread the playback of canvas recordings across multiple threads in the GPU process. r=mattwoodrow

I also had some GPU driver crashes while using FF recently, but I will report it in another bug. Need to reproduce first.

Regressed by: canvas-ipc

I'm fairly sure this is because I am getting a DataSourceSurface from the snapshot, when I shouldn't be.

Could you test the build at:
https://queue.taskcluster.net/v1/task/IW5RSWLKRZW72cH5mzGtLw/runs/0/artifacts/public/build/install/sea/target.installer.exe
to see if it fixes the problem, thanks.

You might want to run that with the following to create a profile for testing:
firefox -no-remote -P

Flags: needinfo?(kasper93)

Yes, your new build resolves the main issue. Here is the profile if you are interested https://perfht.ml/2RhF1cc

Do you have some reference benchmark you would like me to test against? To see if there are maybe more areas to improve?

Flags: needinfo?(kasper93)
Duplicate of this bug: 1559676

(In reply to Kacper Michajłow [:kasper93] from comment #2)

Yes, your new build resolves the main issue. Here is the profile if you are interested https://perfht.ml/2RhF1cc

Do you have some reference benchmark you would like me to test against? To see if there are maybe more areas to improve?

Thanks for testing this, it's also been confirmed in another bug.

I don't have anything specific benchmark-wise, we have some of our own benchmark testing, but they didn't seem to pick this up.

The changes for bug 1464032 are actually for moving the accelerated canvas drawing to the GPU thread and are mainly behind a pref.
Obviously they do touch some existing code-paths and I mistakenly introduced this bug in a fairly late refactor during reviews.
Part of that was to avoid getting data back from the GPU process off the content main-thread, so I'll have to come up with a different way of doing that.

Assignee: nobody → bobowencode
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Pushed by bobowencode@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/5fc7054fc05d
Don't explicitly return DataSourceSurface from CanvasRenderingContext2D::GetSurfaceSnapshot. r=mattwoodrow

we have some of our own benchmark testing, but they didn't seem to pick this up.

This bothers me, I would expect a test to track this performance. I know there are a lot of automated tests, but it seems that none is triggered by this workload which make it easy to sneak regressions. Severe regression can easily be detected manually like this bug, but if it weren't so obvious it would probably be not detected for long time.

Anyhow, Thanks for the fix.

Status: ASSIGNED → RESOLVED
Closed: Last month
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
See Also: → 1560085
Duplicate of this bug: 1560085
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.