1366874 - [meta] WindowConstructor is expensive during startup

Status: RESOLVED FIXED

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

Description

[Florian Quèze [:florian]]

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?

Comment 1

[Jim Mathies [:jimm]]

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

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

(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.

Comment 3

[(No longer employed by Mozilla) Aaron Klotz]

...though I guess it probably does some initialization overtop of just COM initialization?

Comment 4

[Jim Mathies [:jimm]]

Yeah, it inits a host of random COM based windows services like clipboard and drag and drop. We need this call.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(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.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I fixed all bugs of TSFTextStore which can be fixed easy. How did they affect the score?

Comment 7

[Florian Quèze [: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 :-).

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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.

Comment 9

[Florian Quèze [:florian]]

Another cold profile captured today on the reference hardware: http://bit.ly/2tVuJ5A

Comment 10

[Andrew Overholt [:overholt]]

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.

Comment 11

[Florian Quèze [: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.

Comment 12

[Andrew Overholt [:overholt]]

(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?