Closed Bug 1366874 Opened 7 years ago Closed 7 years ago

[meta] WindowConstructor is expensive during startup

Categories

(Core :: Widget: Win32, defect, P2)

x86_64
Windows 10
defect

Tracking

()

RESOLVED FIXED
Performance Impact high

People

(Reporter: florian, Unassigned)

References

Details

(Keywords: meta, Whiteboard: [tpi:+])

See this profile: https://perfht.ml/2qdobMq This is when creating the hidden window. mozilla::widget::IMEHandler::Initialize() (and more specifically mozilla::widget::TSFTextStore::Initialize()) takes a while here, with a very expensive CoCreateInstance call (I'm not familiar enough with the Windows APIs to figure out exactly which service we are creating an instance of). It looks like it does main thread IO in CAssemblyCacheFile::Open(unsigned char * *) and CAssemblyCacheFile::~CAssemblyCacheFile(). Could this be done off main thread? Then OleInitialize takes 21ms. Again, could this be done off main thread? Finally we have 88ms spent in nsUXThemeData::InitTitlebarInfo() calling GetSystemMetrics. This seems to be to get data that will be overwritten very soon when the first actual browser window will be opened, with a call to nsUXThemeData::UpdateTitlebarInfo (that spends another 12ms in NtUserGetWindowCompositionAttribute). Could we initialize this placeholder data lazily so that we don't pay this cost if the data isn't going to be used? Jimm, could you please figure out what's actionable in this?
Flags: needinfo?(jmathies)
We should break this up I think. For starters we spend a lot of time doing initialization in TSFTextStore::Initialize() [1]. Masayuki, do you think any of this can be delayed or done on demand in some way? http://searchfox.org/mozilla-central/source/widget/windows/TSFTextStore.cpp#5989
Flags: needinfo?(jmathies) → needinfo?(masayuki)
Depends on: 1369508
(In reply to Florian Quèze [:florian] [:flo] from comment #0) > Then OleInitialize takes 21ms. Again, could this be done off main thread? No, it has to be run on the thread that is instantiating and accessing the COM object. But one thing that we probably *can* do is completely eliminate that call; The main thread is already initialized for COM via mscom::MainThreadRuntime.
...though I guess it probably does some initialization overtop of just COM initialization?
Whiteboard: [qf] → [qf:p1]
Yeah, it inits a host of random COM based windows services like clipboard and drag and drop. We need this call.
(In reply to Florian Quèze [:florian] [:flo] from comment #0) > See this profile: https://perfht.ml/2qdobMq > > This is when creating the hidden window. > > mozilla::widget::IMEHandler::Initialize() (and more specifically > mozilla::widget::TSFTextStore::Initialize()) takes a while here, with a very > expensive CoCreateInstance call (I'm not familiar enough with the Windows > APIs to figure out exactly which service we are creating an instance of). It > looks like it does main thread IO in CAssemblyCacheFile::Open(unsigned char > * *) and CAssemblyCacheFile::~CAssemblyCacheFile(). Could this be done off > main thread? No, TSF is managed every thread (according to MSDN): https://msdn.microsoft.com/en-us/library/windows/desktop/ms628979(v=vs.85).aspx Even if we create them from another thread, how do we activate it in the main thread before any input of users? I guess that it's impossible. On the other hand, some of the initialization could be put off. I think that initializing each pref cache can be delayed until first use: http://searchfox.org/mozilla-central/rev/972b7a5149bbb2eaab48aafa87678c33d5f2f045/widget/windows/TSFTextStore.cpp#6130-6163 sInputProcessorProfiles is used only by TSFStaticSink and debug code. TSFStaticSink is used for logging and hacking TSF bugs. So, we could put off to initialize it until first use: http://searchfox.org/mozilla-central/rev/972b7a5149bbb2eaab48aafa87678c33d5f2f045/widget/windows/TSFTextStore.cpp#6008-6017 QIed interfaces could be put off until first use: http://searchfox.org/mozilla-central/rev/972b7a5149bbb2eaab48aafa87678c33d5f2f045/widget/windows/TSFTextStore.cpp#6036-6054 And two COM objects are necessary only when user uses composition, so, their initialization could be put off: http://searchfox.org/mozilla-central/rev/972b7a5149bbb2eaab48aafa87678c33d5f2f045/widget/windows/TSFTextStore.cpp#6063-6083 I think that creating and initializing disabled context couldn't be put off because it will be used immediately after that. http://searchfox.org/mozilla-central/rev/972b7a5149bbb2eaab48aafa87678c33d5f2f045/widget/windows/TSFTextStore.cpp#6085-6104 Additionally, these initialization must be good to test TSF enough work on the environment. In other words, after we put off some initialization until first use, we cannot test if TSF completely work on the environment and fallback to IMM mode forcibly. However, there shouldn't be such environment actually. So, I guess that this won't be a matter. I'll file follow up bugs to fix each issue.
Flags: needinfo?(masayuki)
Priority: -- → P2
Summary: WindowConstructor is expensive during startup → [meta] WindowConstructor is expensive during startup
Whiteboard: [qf:p1] → [qf:p1][tpi:+]
I fixed all bugs of TSFTextStore which can be fixed easy. How did they affect the score?
Flags: needinfo?(florian)
Here are 2 cold startup profiles of WindowConstructor with the current (June 20) nightly on the quantum reference hardware: http://bit.ly/2ryURBB (10ms) http://bit.ly/2rzeY2L (15ms) Note that IO cost is random, so I can't really give a 'score'. Overall, WindowConstructor is still visible, but seems to have improved significantly :-).
Flags: needinfo?(florian)
Thank you. Looks like that ITfThreadMgr::Activate() is sometimes slow (taking 5ms in the former, but only 1ms in the latter). For risk management, we shouldn't touch this anymore (although I guess it can be put off). Otherwise, there are no points which I can work on.
Another cold profile captured today on the reference hardware: http://bit.ly/2tVuJ5A
This is marked as [meta] but has no more open dependencies. Is there work left here? Masayuki's comment 8 indicates there's nothing in his realm.
Flags: needinfo?(florian)
Someone who understands this code should look at the profile in comment 9 to see if there are any additional possible wins. If there are none, then we should resolve this bug.
Flags: needinfo?(florian)
(In reply to Florian Quèze [:florian] [:flo] from comment #11) > Someone who understands this code should look at the profile in comment 9 to > see if there are any additional possible wins. If there are none, then we > should resolve this bug. Thanks, Florian. Jim, do you know who could do that?
Flags: needinfo?(jmathies)
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(jmathies)
Resolution: --- → FIXED
Performance Impact: --- → P1
Whiteboard: [qf:p1][tpi:+] → [tpi:+]
You need to log in before you can comment on or make changes to this bug.