Closed Bug 353433 Opened 18 years ago Closed 10 years ago

Preference window slow to open

Categories

(Camino Graveyard :: Preferences, defect, P3)

PowerPC
macOS
defect

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: phiw2, Unassigned)

Details

(Keywords: regression)

Attachments

(7 files)

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.
Can you attach a sample?
Attached file sample
does this suffice ?
Keywords: regression
I see some slowness on the branch, too.  I'll sample when I next relaunch.
Not just trunk per Smokey, and I'm confirming, since I definitely see the slowness.

cl
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: [trunk] Preference window slow to open → Preference window slow to open
I don't see anything useful in the sample; I'll try sharking this if no-one else gets to it.
We should try to get this for a1.
Target Milestone: --- → Camino1.1
I don't see anything special from that sample. Might be LS running slow returning queries for "feed://" and "http://"...
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.
Attached patch Proposed Patch 1Splinter Review
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)?
Assignee: nobody → nick.kreeger
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.
The patch actually seems to slow things down a bit. identifierForBundle: now takes a bit more time to load.
Not blocking 1.1a1, but we'd definitely take a patch.
Flags: camino1.1a1? → camino1.1a1-
Any thoughts on alternative approaches?  This remains very annoying ;)
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.
Assignee: nick.kreeger → stuart.morgan
Status: NEW → ASSIGNED
Attachment #246246 - Flags: review?
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
Attachment #246246 - Flags: review? → review+
Attachment #246246 - Flags: superreview?(mikepinkerton)
Comment on attachment 246246 [details] [diff] [review]
ingore http (checked in)

still, less code is always better.

sr=pink
Attachment #246246 - Flags: superreview?(mikepinkerton) → superreview+
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.
Attachment #246246 - Attachment description: ingore http → ingore http (checked in)
So for those that were seeing this, how are recent nightlies doing?
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.
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.
(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...
Another thread is probably a good idea. How about doing it on [or slightly after] startup on a separate thread?
(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...)
(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

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

(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.
Pushing to 2.0 for now, but we'll take patches as they're written.
Flags: camino1.1b1?
Flags: camino1.1b1-
Flags: camino1.1?
Flags: camino1.1-
Target Milestone: Camino1.1 → Camino2.0
Is comment 20 et seq. something we can/should consider for 1.1 (or are there any other brilliant ideas)?
Setting priority per 1.6 roadmap.
Priority: -- → P3
Target Milestone: Camino2.0 → Camino1.6
Attached patch fixSplinter Review
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.
Attachment #297177 - Flags: review?(alqahira)
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....
Attached file crash log
Doesn't look terribly informative :(
(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 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.
Attachment #297177 - Flags: review?(alqahira)
And/or a General nib that was last touched in an unfriendly IB version....

Pushing off since the patch currently here doesn't work.
Target Milestone: Camino1.6 → ---
Assignee: stuart.morgan+bugzilla → nobody
Status: ASSIGNED → NEW
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]
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: