Walter Chen[:ypwalter][:wachen] Reporter
	Description • 7 years ago

STR:
0. Enable Keyboard Shortcuts in Settings
1. Open GMail
2. Press "c"
3. Wait for the new mail window to open

Reports: https://tinyurl.com/knp6swp
Notes: https://docs.google.com/spreadsheets/d/1MsTK1FW88wuLd25A18HG2KqpjYJnvrxGM9a6MeZp35w/edit#gid=256706414

Firefox Profile and Chrome Tracing will be attached in the following.

	Walter Chen[:ypwalter][:wachen] Reporter
	Comment 1 • 7 years ago

Firefox Profile:
https://perfht.ml/2pVhaQ9	https://perfht.ml/2pVfmqF	https://perfht.ml/2pV3hSn
Chrome Tracing:
https://tinyurl.com/lubdbtc

	MarkYen
	Comment 2 • 7 years ago

Short Gecko Profile(cover only Input Lantency action):
https://perfht.ml/2qNdvof

	Cervantes Yu [:cyu] [:cervantes]
	Comment 3 • 7 years ago

(In reply to myen from comment #2)
> Short Gecko Profile(cover only Input Lantency action):
> https://perfht.ml/2qNdvof

In this part of the profile after the key event is handled in the content process:
https://perf-html.io/public/07f6e6a057f7328212062d8a199193d0d16fed8f/calltree/?implementation=cpp&range=1.3871_2.3175&search=Flush&thread=5

We can see 2 expensive operations:
mozilla::dom::CSS2PropertiesBinding::get_direction() triggers a restyle of 42 ms.
mozilla::dom::ElementBinding::get_clientWidth() triggers a reflow of 42 ms.

	Bobby Chien
	Comment 4 • 7 years ago

Andrea, could you help to profile this bug?

	Kan-Ru Chen [:kanru] (UTC+9)
	Comment 5 • 7 years ago

(In reply to Cervantes Yu [:cyu] [:cervantes] from comment #3)
> (In reply to myen from comment #2)
> > Short Gecko Profile(cover only Input Lantency action):
> > https://perfht.ml/2qNdvof
> 
> In this part of the profile after the key event is handled in the content
> process:
> https://perf-html.io/public/07f6e6a057f7328212062d8a199193d0d16fed8f/
> calltree/?implementation=cpp&range=1.3871_2.3175&search=Flush&thread=5
> 
> We can see 2 expensive operations:
> mozilla::dom::CSS2PropertiesBinding::get_direction() triggers a restyle of 42 ms.
> mozilla::dom::ElementBinding::get_clientWidth() triggers a reflow of 42 ms.

A bunch sync restyle/reflows, maybe Daniel can also take a look.

	Daniel Holbert [:dholbert]
	Comment 6 • 7 years ago

(In reply to Cervantes Yu [:cyu] [:cervantes] from comment #3)
> We can see 2 expensive operations:
> mozilla::dom::CSS2PropertiesBinding::get_direction() triggers a restyle of 42 ms.
> mozilla::dom::ElementBinding::get_clientWidth() triggers a reflow of 42 ms.

Minor correction: the latter 42ms for get_clientWidth is mostly not reflow (though it is prompted by a layout flush).

It's really a 26ms restyle (ProcessPendingRestyles, which in this case is mostly nsCSSFrameConstructor), and then there's 15ms of reflow (ProcessReflowCommands).

	Jean Gong :jgong
	Comment 7 • 7 years ago

Daniel, Do you think there is a bug that needs to surface from this? If not I can close this bug.

	Daniel Holbert [:dholbert]
	Comment 8 • 7 years ago

Sorry for the delay here.

(In reply to Jean Gong from comment #7)
> Daniel, Do you think there is a bug that needs to surface from this?

I don't think so -- not a layout/restyle one, at least.

The 15ms reflow inside of get_clientWidth seems to be for extremely-deeply-nested block reflow.

The one thing that stood out to me there is that we're spending 3 samples (3ms of that 15ms reflow) inside of nsNativeThemeWin::GetWidgetBorder:
https://perfht.ml/2uNeWcF

Fortunately, that code has been optimized recently, to cache results inside of that function! That happened in bug 1373079. So I expect this has improved a bit (shaving off a few ms) as a result of that bug.

Beyond that, nothing about the reflow jumps out at me as obviously expensive or optimizable -- and for the restyles, we can hope that stylo will help there.

> If not I can close this bug.

That makes sense to me, I think.

	Daniel Holbert [:dholbert]
	Comment 9 • 7 years ago

Er sorry, I meant to say 4 samples (4ms of that 15ms reflow).  (The profiler shows 3ms with one inverted callstack, and 1ms with a different inverted callstack - but they're all inside of that same reflow [same call to ProcessReflowCommands].)