Closed
Bug 1155998
Opened 10 years ago
Closed 1 year ago
[APZ] Performance regression in CSS-animation-heavy interactive text
Categories
(Core :: Layout, defect, P3)
Tracking
()
RESOLVED
WORKSFORME
Tracking | Status | |
---|---|---|
e10s | - | --- |
firefox47 | --- | wontfix |
firefox48 | --- | fix-optional |
firefox49 | + | wontfix |
firefox50 | --- | fix-optional |
People
(Reporter: kael, Unassigned)
References
(Blocks 1 open bug, )
Details
(Keywords: perf, regression, Whiteboard: [gfx-noted])
Attachments
(6 files)
I revisited one of my old experiments in the latest rev of Developer Edition and the performance is catastrophically bad compared to how it was before. I previously filed a bug about an issue that affected it - bug #1046055 - and had implemented a workaround for that. With the workaround (and with the fix) performance was great.
Now, performance is generally kind of stuttery, and if you scroll down rapidly the whole browser hangs. This didn't happen before and *shouldn't* happen; the code makes attempts to ensure that it doesn't. Scrolling works as intended in IE (perfectly, very smooth) and just fine in Chrome.
The intended behavior is that scrolling instantly skips through the document to the point of your scroll then resumes animating, without judder or perf degradation.
Comment 1•10 years ago
|
||
I think this might be more of a layout/layerization issue. What do you think Matt?
Flags: needinfo?(matt.woodrow)
Whiteboard: [gfx-noted]
Comment 2•8 years ago
|
||
Not sure if this is an issue with both JavaScript mixed with CSS styles but in my work environment our network monitoring software (Science Logic EM7) runs decent on first login. But degrades very rapidly until pages stutter pop windows open slower and saves take forever.
Comment 3•8 years ago
|
||
Comment 4•8 years ago
|
||
Comment 5•8 years ago
|
||
[Tracking Requested - why for this release]:
Comment 6•8 years ago
|
||
Tracking 49+ because of the nature of this regression - this bug still needs an owner - Matt are you the correct person to investigate? Thanks.
Comment 7•8 years ago
|
||
(In reply to Marcia Knous [:marcia - use ni] from comment #6)
> Tracking 49+ because of the nature of this regression - this bug still needs
> an owner - Matt are you the correct person to investigate? Thanks.
I created a new bug for my issue before I found this. I am not 100% sure my bug is related but its sounds very similar performance wise. Same performance profiles attached. https://bugzilla.mozilla.org/show_bug.cgi?id=1277206
Updated•8 years ago
|
Comment 8•8 years ago
|
||
I can't reproduce this on OSX on Nightly, performance is really good there, this might be Windows specific.
Flags: needinfo?(matt.woodrow)
Keywords: regressionwindow-wanted
Reporter | ||
Comment 9•8 years ago
|
||
It's fine on Windows for me in x64 dev edition, so it may've been fixed recently?
Updated•8 years ago
|
Comment 10•8 years ago
|
||
I can't reproduce either on Win10. However, scrolling in my Ubuntu VM (Basic Compositor) is still pretty terrible.
Comment 11•8 years ago
|
||
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0
Firefox: 50.0a1, Build ID: 20160701030235
I have tested this issue on Ubuntu 14.04 x64 and Windows 8.1 x64 on the latest Nightly(50.0a1) build.
I've managed to reproduce it and performed a regression on both OSs.
This are the results on Ubuntu 14.04:
Last good revision: 077a6b1a1ba5abd927990e01740806c76f899f9b
First bad revision: 30742281c223e2a7abb5931ee5383d890b5ce0e8
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=077a6b1a1ba5abd927990e01740806c76f899f9b&tochange=30742281c223e2a7abb5931ee5383d890b5ce0e8
This are the results on Windows 8.1 x64:
Last good revision: 738d8ee106fe760c44f54b5f250d0b36a84818c8
First bad revision: af67b5f73381f154f41ded5926d823709a0ebd08
INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=738d8ee106fe760c44f54b5f250d0b36a84818c8&tochange=af67b5f73381f154f41ded5926d823709a0ebd08
Keywords: regressionwindow-wanted → regression
Updated•8 years ago
|
Blocks: apz-windows, apz-linux
Comment 13•8 years ago
|
||
(In reply to Emil Pasca from comment #11)
> This are the results on Windows 8.1 x64:
>
> Last good revision: 738d8ee106fe760c44f54b5f250d0b36a84818c8
> First bad revision: af67b5f73381f154f41ded5926d823709a0ebd08
> INFO: Pushlog:
> https://hg.mozilla.org/integration/mozilla-inbound/
> pushloghtml?fromchange=738d8ee106fe760c44f54b5f250d0b36a84818c8&tochange=af67
> b5f73381f154f41ded5926d823709a0ebd08
Can you attach your about:support information from the windows build that reproduces this problem? On my Windows tablet I can't repro any bad behaviour while scrolling down on the page in the URL field.
Flags: needinfo?(bugmail)
Comment 15•8 years ago
|
||
Attached windows_8.1_troubleshooting_information.
Flags: needinfo?(emil.pasca)
Comment 16•8 years ago
|
||
Nothing there looks out of the ordinary. Do you have access to a Windows 10 machine that you can try reproducing this on? I wonder if it's just a Windows 8 vs Windows 10 issue.
Flags: needinfo?(emil.pasca)
Comment 17•8 years ago
|
||
Build Identifier:
https://hg.mozilla.org/mozilla-central/rev/c9a70b64f2faa264296f0cc90d68a2ee2bac6ac5
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0 ID:20160705030222
I can reproduce the performance regression in Nightly50.0a1 Windows10 if enabled APZ.
This performance regression appears regardless of HWA enabled/disabled.
Summary: Catastrophic performance regression in CSS-animation-heavy interactive text → [APZ] Catastrophic performance regression in CSS-animation-heavy interactive text
Comment 18•8 years ago
|
||
Thanks. I am still unable to reproduce on my Windows 10 device with APZ enabled. Just to confirm, the STR I'm using is:
1. Click on the link in the URL field of this bug
2. Scroll down a bunch (I've tried scrolling down to different places in the page)
In all cases, when I stop scrolling, the text starts animating in one letter at a time, and it looks pretty nice (although I also get the quotation marks always visible, which looks kind of odd). After it fills the visible screen area it pauses the animation and I have to scroll down more to get it continue animating. After I scroll down to the very bottom everything is done and I can scroll around no problem.
Flags: needinfo?(emil.pasca)
Comment 19•8 years ago
|
||
STR:
1. OPEN URL field of this bug
2. Press [SPACE] key to start drawing texts
Actual results:
Rendering is slow down about 40-50% if APZ is enabled
See screen capture: https://youtu.be/LZkZaNSiWr8
Performance comparison between APZ on and APZ off.
LEFT: APZ on / RIGHT:APZ off
Comment 20•8 years ago
|
||
Ah, thanks! I was thinking there would be stuttery performance / browser hangs as described in comment 0. I can do some profiling on this tomorrow and see what's causing the slowness. ni myself so I don't forget.
Flags: needinfo?(bugmail)
Comment 21•8 years ago
|
||
Based on a profile I got, I suspect we are creating a lot of ContainerLayers when APZ is enabled.
https://cleopatra.io/#report=635340cada1ef09b628cbd346b471b91349ffdb8
Comment 22•8 years ago
|
||
Ok, so from what I can make out, each character that animates in is initially set to opacity:0, but when event-regions are enabled, they get a nsDisplayOpacity which contains a event regions item and a text item. These all end up creating inactive layer managers during processing and then it all gets flattened back into a relatively simple final layer tree.
With APZ enabled, the displayport area is of course larger and so it takes even more time. I'm attaching a display list dumps with APZ on, APZ off but event regions on, and APZ and event regions off which illustrate the problem.
Flags: needinfo?(bugmail)
Comment 23•8 years ago
|
||
Comment 24•8 years ago
|
||
Comment 25•8 years ago
|
||
So in the end I think this is basically expected behaviour given how the page is constructed and how we handle opacity. Because opacity:0 stops elements from painting but keeps them participating in event dispatch, we need to build the event regions to ship over to the compositor. If the opacity:0 were accompanied by pointer-events:none then we wouldn't have to do this and things would behave much better.
Our other option is to try to optimize the "opacityItemForEventsOnly" codepath at [1], which is probably going to be difficult.
Given that the user-visible impact here is just slower animation, and not browser hanging/crashing as described in comment 0, I don't think this is super urgent to fix. Authors should be able to pointer-events:none on the hidden characters as a workaround.
[1] http://searchfox.org/mozilla-central/rev/a7c8e9f3cc323fd707659175a46826ad12899cd1/layout/generic/nsFrame.cpp#2193
Comment 26•8 years ago
|
||
I think we could fix this if we wanted to.
When we get to BuildDisplayListForStackingContext for an opacity:0 with event regions, we could skip creating the new event regions item (so all added regions go into the existing regions item). We'd also have to block recursive calls to this and BuildDisplayListForChild creating event regions items.
Once we finish building the child display list (which should only contain real items, not event regions), we can just discard it instead of wrapping it in an nsDisplayOpacity.
Reporter | ||
Comment 27•8 years ago
|
||
pointer-events: none being necessary for good rendering performance is really surprising to me. I've never seen it messaged to anyone in documentation or as developer best practices wisdom. If it's a new problem that's been introduced recently, an effort to communicate it to people seems necessary, at the very least.
I don't think I've ever encountered pointer-events: none in the wild except in cases where people specifically wanted to block mouse events from hitting an element.
I do think the bad behavior here is less severe than it was when I reported the issue though. It makes the behavior unpleasant but doesn't cause the browser to severely degrade or hang (unless you count mobile, where this sort of stuff is unusably bad anyway no matter which browser you use). If the pointer events hack is messaged to developers they'll eventually pick up on it.
Updated•8 years ago
|
status-firefox48:
--- → fix-optional
status-firefox50:
--- → affected
Summary: [APZ] Catastrophic performance regression in CSS-animation-heavy interactive text → [APZ] Performance regression in CSS-animation-heavy interactive text
Updated•8 years ago
|
status-firefox47:
--- → wontfix
Comment 28•8 years ago
|
||
Hi :milan,
Do you think is there anyone in your team can help to investigate this issue or take this while kats is on PTO?
Flags: needinfo?(milan)
(In reply to Gerry Chang [:gchang] from comment #28)
> Hi :milan,
> Do you think is there anyone in your team can help to investigate this issue
> or take this while kats is on PTO?
kats wouldn't likely be doing it; we would deal with this in layout - see comment 26 - but sitting on the outside, it isn't clear to me how urgent that work would be. Maybe Jet has a feeling about the priority.
Flags: needinfo?(milan) → needinfo?(bugs)
Comment 30•8 years ago
|
||
(In reply to K. Gadd (:kael) from comment #27)
> pointer-events: none being necessary for good rendering performance is
> really surprising to me.
We agree that this is incorrectly functioning, but offer an easy workaround (pointer-events: none) if your use-case needs it. The fix proposed in comment 26 may take more time to put together, but the workaround should help you get good performance in Firefox right away.
Flags: needinfo?(bugs)
We are heading into beta 7 now so it's a bit late. We could still take a low risk fix. Seems ok to aim this at 50 or 51 given we have a workaround.
Updated•8 years ago
|
Updated•7 years ago
|
Updated•2 years ago
|
Severity: normal → S3
Comment 32•2 years ago
|
||
Can anyone still reproduce this bug?
Flags: needinfo?(kg)
Flags: needinfo?(alice0775)
Updated•2 years ago
|
Comment 33•2 years ago
|
||
(In reply to Gregory Pappas [:gregp] from comment #32)
Can anyone still reproduce this bug?
The problem no longer reproduces on Nightly112.0a1 as well as the bad build in Comment#17.
Flags: needinfo?(alice0775)
Comment 34•1 year ago
|
||
Closing per comment 33, thanks!
Status: NEW → RESOLVED
Closed: 1 year ago
Flags: needinfo?(kg)
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•