76865 - sServiceManager->GetService is an expensive call [perf]

Status: RESOLVED WONTFIX

(Core Graveyard :: Embedding: GTK Widget, defect, P3)

Description

[Pete Collins (MDG)]

In nsEmbedAPI.cpp 

sServiceManager->GetService is an expensive call. 
I guess this is another threading blocker issue. 
Once the chrome is registered this should not block.
There is a hiccup right at this call. 

This is a performance issue.

--pete

Comment 1

[Pete Collins (MDG)]

Link to call:

http://lxr.mozilla.org/seamonkey/source/embedding/base/nsEmbedAPI.cpp#152

--pete

Comment 2

[Pete Collins (MDG)]

I am proposing this patch to give a speed boost to mozilla embedded.

These calls seem to not be needed. They are covered later on.

What this does is gives you a base window very quickly whicl everything is
registered in the background.

Try it out is works nice.

--pete

Comment 3

[Pete Collins (MDG)]

Attachment: Patch to get a UI window up quicker

Comment 4

[Pete Collins (MDG)]

Now if i do:

$ ./run-mozilla.sh ./gtkEmbed

I get a base window up in under a second.

--pete

Comment 5

[Pete Collins (MDG)]

So now, perhaps if someone can beat some sense into me and tell me why this is
not a good thing. 

;-)

--pete

Comment 6

[Conrad Carlen (not reading bugmail)]

Adam, since you discovered the need to create the string bundle service on the
main thread, what were the symptoms of it *not* being created on the main
thread? Also, Pete, since you're doing this on Unix, is it possible that the
problem Adam found is not a problem for Unix but is for Windows?

Comment 7

[Pete Collins (MDG)]

FYI Some time stats:

0-1 sec: gtk window appears
1-5 sec: url www.mozdev.org is finished loading.

So i see a 5 second load time for that single web page.

If i use: Netscape® Communicator 4.73

I see the base window in just slightly over a second.
The url www.mozdev.org is loaded in under 2 seconds.

Granted, i am using a *debug* version of mozilla so . . .

--pete

Comment 8

[Pete Collins (MDG)]

Woha ..  `Mid Air Colision'  heh . . .
 
Conrad, maybe If Adam has a minute he can apply the patch on windows and see
what breaks. Does it work for you on mac?

--pete

Comment 9

[Adam Lock]

The symptoms were that as soon as you made an HTTP request you got thread 
assertions everytime necko tried to access the string bundle to send you a 
notification string.

If everything works with the hack commented out, I think it is no longer 
needed. You could also remove the #define HACK_AROUND_THREADING_ISSUES at the 
top of the file too.
 
I notice that the patch also removes some chrome-registry code. I don't know why 
it is there, but it's possible that it's necessary to ensure newly added chrome 
is correctly registered.

Comment 10

[Boris Zbarsky [:bzbarsky]]

get thee off the unconfirmed radar.

Comment 11

[Pete Collins (MDG)]

Ok, i see different behavior using embed that is built in the bin dir and using
embed in the dist/Embed dir. I do see some asserts if i run in dist/Embed.


--pete

Comment 12

[Christopher Blizzard (:blizzard)]

I think that the call to CheckForNewChrome() should be in an ifdef DEBUG since
we should autoregister in that case.

Comment 13

[Pete Collins (MDG)]

Yea, things look pretty smooth until we get to here:

Source File: tests/gtkEmbed/main.cpp:
Function: OpenWebPage():
IMethod: webNav->LoadURI(NS_ConvertASCIItoUCS2(url).GetUnicode(),
nsIWebNavigation::LOAD_FLAGS_NONE);

--pete

Comment 14

[Pete Collins (MDG)]

Execution of LoadURI is fine.
It looks like this is the point that it can't fire off until the chromeRegistry
thread is finished it's auto reg.

So this particular perf choke is the chrome registration. 

(This is all being tested from dist/bin/gtkEmbed)

--pete

Comment 15

[Christopher Blizzard (:blizzard)]

Over to pete since he's been working on it.

Comment 16

[Pete Collins (MDG)]

Marking for 0.9.5.

--pete

Comment 17

[Pete Collins (MDG)]

pushing back

Comment 18

[Sylvestre Ledru [:Sylvestre]]

Embedding: GTK Widget isn't a thing, closing.