User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0 Build ID: 20171115094908 Steps to reproduce: Me and a friend use firefox with the tab bar hidden in userChrome.css, using: #TabsToolbar { visibility: collapse; } We had already noticed that the throbber both in Tree Style Tabs tabs and in normal tabs consumes a lot of CPU, but in addition we also noticed that _more_ CPU is consumed when the tab bar is hidden. You can find more detailed information here, the impact on battery life is particularly disconcerting: https://github.com/piroor/treestyletab/issues/1384#issuecomment-345475606 . Actual results: High CPU and energy consumption when throbber active, and even more CPU when throbber active and hidden Expected results: The throbber should have limited impact on the CPU and should definitely not consume more CPU when the tab bar is hidden.

We only track one issue per bug. Can you please clarify whether you want this bug to be about "throbber consumes a lot of CPU" or "throbber consumes more CPU when hidden"? FWIW, visibility: collapse works like visibility: hidden under the hood, not like display: none. You shouldn't expect a performance win from this.

(In reply to Dão Gottwald [::dao] from comment #1) > We only track one issue per bug. Can you please clarify whether you want > this bug to be about "throbber consumes a lot of CPU" or "throbber consumes > more CPU when hidden"? It does not make a difference to me, I can create a separate ticket if you like. > FWIW, visibility: collapse works like visibility: hidden under the hood, not > like display: none. You shouldn't expect a performance win from this. The same problem exists with display: none, see <https://github.com/piroor/treestyletab/issues/1384#issuecomment-339512601>.

jaws, can you make sense of this bug?

Feels like having 2 performance profiles (see https://perf-html.io/ ), one with the indicators visible and one with the indicators hidden (probably best to check display:none) would help make sense of this.

This is because of bug 1237454. (In reply to Francesco Mazzoli from comment #2) > (In reply to Dão Gottwald [::dao] from comment #1) > > We only track one issue per bug. Can you please clarify whether you want > > this bug to be about "throbber consumes a lot of CPU" or "throbber consumes > > more CPU when hidden"? > > It does not make a difference to me, I can create a separate ticket if you > like. > > > FWIW, visibility: collapse works like visibility: hidden under the hood, not > > like display: none. You shouldn't expect a performance win from this. > > The same problem exists with display: none, see > <https://github.com/piroor/treestyletab/issues/1384#issuecomment-339512601>. CSS animations on elements in display:none subtree, so they shouldn't consume CPU. But... The throbber animation isn't a CSS animations? It's script animation?

https://github.com/piroor/treestyletab/blob/2.2.11/webextensions/sidebar/styles/throbber.css#L24-L30 uses @keyframes.

Thank you, Simon. Then the animation should stop when the element is in display:none subtree. odd..

I don't see anything special in both profiles, but I've noticed the throbber animation in Tree Style Tabs does not specify 0% keyframe value, so if it could specify the 0% value, the high CPU usage should be fixed. I did leave a comment in their issue, https://github.com/piroor/treestyletab/issues/1384#issuecomment-347713777 .

If I am seeing the correct @keyframes for the throbber animation, we don't specify 0% value either [1] Jared, can we specify 0% value there? transform: translateX(0px) is a good value, (we shouldn't use transform:none there, you can read the reason in bug 1419079 comment 35.) Anyway, I think the high CPU usage is caused by bug 1419079. :/ Sorry for the inconvenience. [1] https://hg.mozilla.org/mozilla-central/file/cb9092a90f6e/browser/themes/shared/tabs.inc.css#l188

Yes, patch on the way. Though we didn't specify 0% value before because animations worked fine without it being specified as it would use the default value. We will now have to investigate our other @keyframes to make sure we have a 0% and 100% value explicitly specified.

I filed bug 1421672 to fix the other instances of this in the tree.

Comment on attachment 8932937 [details] Bug 1419096 - Explicitly declare 0% value in @keyframes animation because start/end values are needed to calculate cumulative change hints. https://reviewboard.mozilla.org/r/203942/#review209420 ::: commit-message-cad9c:3 (Diff revision 1) > +Bug 1419096 - Explicitly declare 0% value in @keyframes animation because start/end values are needed to calculate cumulative change hints. r?dao > + > +See https://bugzilla.mozilla.org/show_bug.cgi?id=1419079#c22 for more information. I'm not sure how to read this. This must be a bug, right? Will this go away with bug 1419079?

From my understanding, this optimization requires us to specify the 0% and 100% values and won't go away with bug 1419079. Hiro, can you confirm?

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #16) > From my understanding, this optimization requires us to specify the 0% and > 100% values and won't go away with bug 1419079. Hiro, can you confirm? I can't answer whether bug 1419079 will fix this case or not as of now, especially for transform animation cases. Currently I have the only one awkward approach in my mind to fix the transform animation cases, and I am not sure we take the approach in bug 1419079.

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #16) > From my understanding, this optimization requires us to specify the 0% and > 100% values Yes, specifying 0% and 100% values is a prerequisite for offscreen animation optimizations. Though there are many CSS properties that can't be optimized even if 0% and 100% values are specified, e.g. width, height, or some others which might affect layout.

ni?ing myself to make sure this gets added to https://developer.mozilla.org/en-US/Firefox/Performance_best_practices_for_Firefox_fe_engineers and the other front-end engineers are alerted to it.

Comment on attachment 8932937 [details] Bug 1419096 - Explicitly declare 0% value in @keyframes animation because start/end values are needed to calculate cumulative change hints. https://reviewboard.mozilla.org/r/203942/#review209702 I still don't understand the optimization and why the style system couldn't infer that information itself, but r+ for now

Comment on attachment 8932937 [details] Bug 1419096 - Explicitly declare 0% value in @keyframes animation because start/end values are needed to calculate cumulative change hints. https://reviewboard.mozilla.org/r/203942/#review209702 I agree.

Pushed by jwein@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8fcdfe8b0a97 Explicitly declare 0% value in @keyframes animation because start/end values are needed to calculate cumulative change hints. r=dao

Should we uplift this to 58, since it's a really simple fix?

(In reply to Marco Castelluccio [:marco] from comment #24) > Should we uplift this to 58, since it's a really simple fix? yes

Comment on attachment 8932937 [details] Bug 1419096 - Explicitly declare 0% value in @keyframes animation because start/end values are needed to calculate cumulative change hints. Approval Request Comment [Feature/Bug causing the regression]: performance regression noticed since optimizations require start/end values and we only specified one of the two [User impact if declined]: extra CPU usage when loading pages [Is this code covered by automated tests?]: no [Has the fix been verified in Nightly?]: no [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: very simple CSS addition that explicitly specifies the default values [String changes made/needed]: none

Comment on attachment 8932937 [details] Bug 1419096 - Explicitly declare 0% value in @keyframes animation because start/end values are needed to calculate cumulative change hints. Fix a performance regression. Beta58+.

Redirecting ni? to jaws to get something added to https://developer.mozilla.org/en-US/Firefox/Performance_best_practices_for_Firefox_fe_engineers about this optimization.

(In reply to Mike Conley (:mconley) (:⚙️) - Backlogged on reviews and needinfos from comment #29) > Redirecting ni? to jaws to get something added to > https://developer.mozilla.org/en-US/Firefox/ > Performance_best_practices_for_Firefox_fe_engineers about this optimization. Added to the doc at https://developer.mozilla.org/en-US/Firefox/Performance_best_practices_for_Firefox_fe_engineers#Explicitly_define_start_and_end_animation_values