353433 - Preference window slow to open

Status: RESOLVED WONTFIX

(Camino Graveyard :: Preferences, defect, P3)

Description

[philippe (part-time)]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en; rv:1.9a1) Gecko/20060919 Camino/1.2+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en; rv:1.9a1) Gecko/20060919 Camino/1.2+

Summary says it all. This happens only the first time after starting up the browser.

Reproducible: Always

Actual Results:  
delay of 4 to 5 seconds before the Pref window opens.

Expected Results:  
opens immediately.

Seen this for the past 10 days or so, possibly since the RSS stuff landed.

Comment 1

[Stuart Morgan]

Can you attach a sample?

Comment 2

[philippe (part-time)]

Attachment: sample

does this suffice ?

Comment 3

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I see some slowness on the branch, too.  I'll sample when I next relaunch.

Comment 4

[Chris Lawson (gone)]

Not just trunk per Smokey, and I'm confirming, since I definitely see the slowness.

cl

Comment 5

[Stuart Morgan]

I don't see anything useful in the sample; I'll try sharking this if no-one else gets to it.

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

We should try to get this for a1.

Comment 7

[Nick Kreeger]

I don't see anything special from that sample. Might be LS running slow returning queries for "feed://" and "http://"...

Comment 8

[Håkan Waara]

Attachment: Pretty-print of top of Shark profile

I sharked this. In total, the feedviewer-popupmenu and webbrowser-popupmenu account for around ~10% of the first startup of the pref pane.

Most of it is searching for browsers. I noticed that our category method |installedBrowserIdentifiers| seems to first check for apps that support the "http:" protocol, and then, in addition the "https:" protocol. Especially the second check is very expensive.

Anyway, I'll attach a pretty print of the interesting data here.

Comment 9

[Nick Kreeger]

Attachment: Proposed Patch 1

Here is a patch that cleans up |installedBrowserIdentifiers:|. From my testing it seems to speed up things a bit. However, if you find a .html (another good example is a .jpeg) and right click on the file and choose "Open With ->" you can see LS chug to build that list.

The solution that I have in my patch might be a cheaper solution since it doesn't call |LSCanURLAcceptURL| for every URL we get back for the "http://" query.

Anyone want to take a look at this (smorgan, hwaara, smfr)?

Comment 10

[Håkan Waara]

Comment on attachment 240289 [details] [diff] [review]
Proposed Patch 1

Depending on how expensive NSArray's |containsObject:| and |isEqualToArray:| messages are, you might want to use a mutable set instead, and get the union of http+https browsers.

I'll try to shark again with this patch applied later tonight.

Comment 11

[Håkan Waara]

Attachment: Top of shark report after patch

The patch actually seems to slow things down a bit. identifierForBundle: now takes a bit more time to load.

Comment 12

[Samuel Sidler (old account; do not CC)]

Not blocking 1.1a1, but we'd definitely take a patch.

Comment 13

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Any thoughts on alternative approaches?  This remains very annoying ;)

Comment 14

[Stuart Morgan]

Attachment: ingore http (checked in)

Here's another option: ignore http entirely. We only use things that implement https anyway, and it seems pretty unlikely that there are apps out there that handle https but not http.

That said, since we've had the browser code for a while and this only happened after the rss stuff landed, this may not make much of a difference (it seems like either we must be calling it more somehow, or feed: is really slow to find for some reason).  But it certainly wouldn't hurt to do this even if there are other speedups to be had.

Comment 15

[Nick Kreeger]

Comment on attachment 246246 [details] [diff] [review]
ingore http (checked in)

This looks like a better option, perhaps the only *real* one we have atm. I can't sense any increase in speed here, I still think LS is still thinking to hard.

r=me

Comment 16

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 246246 [details] [diff] [review]
ingore http (checked in)

still, less code is always better.

sr=pink

Comment 17

[Stuart Morgan]

Comment on attachment 246246 [details] [diff] [review]
ingore http (checked in)

Checked in on trunk and MOZILLA_1_8_BRANCH.  We'll leave this open for now to see if we need to try to do more.

Comment 18

[Stuart Morgan]

So for those that were seeing this, how are recent nightlies doing?

Comment 19

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

They were slightly better right after you checked in a fix, but recently they seem as bad as ever :(

I'll try to test a pre-fix build, a post-fix build, and a current build in succession, though, to get some more reliable data.

Comment 20

[Stuart Morgan]

We may have to do something like fire off the LS requests on a background thread when the prefs are opened, and then block briefly if the user interacts with one of the drop-downs before the results have come back.

Comment 21

[Nick Kreeger]

(In reply to comment #20)
> We may have to do something like fire off the LS requests on a background
> thread when the prefs are opened, and then block briefly if the user interacts
> with one of the drop-downs before the results have come back.
> 

I agree, sounds like queries just for "https://" is about as expensive...

Comment 22

[Håkan Waara]

Another thread is probably a good idea. How about doing it on [or slightly after] startup on a separate thread?

Comment 23

[Nick Kreeger]

(In reply to comment #22)
> Another thread is probably a good idea. How about doing it on [or slightly
> after] startup on a separate thread?
> 

The only problem I see with that is why use up resources on startup that _might_ not be used at all (i.e. I might only my pref panes once in a blue moon...)

Comment 24

[Nick Kreeger]

(In reply to comment #23)
> (In reply to comment #22)
> > Another thread is probably a good idea. How about doing it on [or slightly
> > after] startup on a separate thread?
> > 
> 
> The only problem I see with that is why use up resources on startup that
> _might_ not be used at all (i.e. I might only my pref panes once in a blue
> moon...)
> 

s/only/open

Comment 25

[Håkan Waara]

(In reply to comment #24)
> (In reply to comment #23)
> > (In reply to comment #22)
> > > Another thread is probably a good idea. How about doing it on [or slightly
> > > after] startup on a separate thread?
> > > 
> > 
> > The only problem I see with that is why use up resources on startup that
> > _might_ not be used at all (i.e. I might only my pref panes once in a blue
> > moon...)
> > 

Then probably Stuart's idea of doing it on a bg thread when the prefs window opens is probably the best idea.   The popupmenu could change its selected item to "Loading ..." or something like it?

Comment 26

[Nick Kreeger]

> Then probably Stuart's idea of doing it on a bg thread when the prefs window
> opens is probably the best idea.   The popupmenu could change its selected item
> to "Loading ..." or something like it?
> 

Agreed, we can probably set the "Loading..." item in the nib and then let the menu building code replace the select button's menu once at the end of the thread.

Comment 27

[Stuart Morgan]

I would assume that the LS query to get the default handler is substantially less expensive than getting all the handlers. Since what's shown when the menus aren't open is the default handlers, they could still look normal while the rest is being figured out; the only user-visible effect would be a slight delay when bringing up one of those menus if they did it almost immediately after opening prefs for the first time.

Comment 28

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #19)
> I'll try to test a pre-fix build, a post-fix build, and a current build in
> succession, though, to get some more reliable data.

I looked at the CaminoBranch-2006-11-27-04, CaminoBranch-2006-11-29-02, and CaminoBranch-2006-12-18-02 builds with a fresh profile.

Empirically it seems to be a wash with the tools/test mechanism I have handy (but the times also seemed faster than what it seemed like when I launch a new Camino every morning, so who knows).

Comment 29

[Stuart Morgan]

(In reply to comment #28)
> Empirically it seems to be a wash with the tools/test mechanism I have handy
> (but the times also seemed faster than what it seemed like when I launch a new
> Camino every morning, so who knows).

LS may very well be keeping a cache that's warm in your testing.

Comment 30

[Chris Lawson (gone)]

Pushing to 2.0 for now, but we'll take patches as they're written.

Comment 31

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Is comment 20 et seq. something we can/should consider for 1.1 (or are there any other brilliant ideas)?

Comment 32

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Setting priority per 1.6 roadmap.

Comment 33

[Stuart Morgan]

Attachment: fix

Implements comment 20. Smokey, could you test this (cold, ideally) on the machine where you were seeing issues? The expected behavior is that the prefs open more or less instantly, and if you very quickly click on one of the menus it will block for as long as necessary, then show the menu.

Comment 34

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 297177 [details] [diff] [review]
fix

This seemed to improve prefs-open speed slightly, but clicking on one of the menus (or at least the feed menu) while the slow LS queries are going on causes a crash.  Log to follow....

Comment 35

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: crash log

Doesn't look terribly informative :(

Comment 36

[Stuart Morgan]

(In reply to comment #34)
> This seemed to improve prefs-open speed slightly

Only slightly? That's really surprising, since I expected getting the default handlers to be very fast.

Could you repro the crash with zombies enabled (NSZombieEnabled=YES) and see if anything useful is logged? I didn't see any crashes in testing that on my machine, so I probably can't repro it here.

Comment 37

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 297177 [details] [diff] [review]
fix

Clearing the review request here since Stuart and I debugged this last week and found that some part of that code hates him on 10.3.9.

I wonder also if part of the delay we're seeing, and perhaps what makes it extra-bad, is the prefs window itself being cold; I definitely see a delay even on Intel when opening prefs  (focused on another pane) when they haven't been opened in a while.

Comment 38

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

And/or a General nib that was last touched in an unfriendly IB version....

Pushing off since the patch currently here doesn't work.

Comment 39

[Gordon P. Hemsley [:GPHemsley]]

This bug has been buried in the graveyard and has not been updated in over 5 years. It is probably safe to assume that it will never be fixed, so resolving as WONTFIX.

[Mass-change filter: graveyard-wontfix-2014-09-24]