Preference window slow to open

RESOLVED WONTFIX

Status

Camino Graveyard
Preferences
P3
normal
RESOLVED WONTFIX
11 years ago
3 years ago

People

(Reporter: philippe (part-time), Unassigned)

Tracking

({regression})

unspecified
PowerPC
Mac OS X
regression
Bug Flags:

Details

Attachments

(7 attachments)

(Reporter)

Description

11 years ago
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

11 years ago
Can you attach a sample?
(Reporter)

Comment 2

11 years ago
Created attachment 239289 [details]
sample

does this suffice ?

Updated

11 years ago
Keywords: regression
I see some slowness on the branch, too.  I'll sample when I next relaunch.

Comment 4

11 years ago
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

Comment 5

11 years ago
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

Comment 7

11 years ago
I don't see anything special from that sample. Might be LS running slow returning queries for "feed://" and "http://"...
Flags: camino1.1a1?

Comment 8

11 years ago
Created attachment 240206 [details]
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

11 years ago
Created attachment 240289 [details] [diff] [review]
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)?

Updated

11 years ago
Assignee: nobody → nick.kreeger

Comment 10

11 years ago
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

11 years ago
Created attachment 240338 [details] [diff] [review]
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.
Not blocking 1.1a1, but we'd definitely take a patch.
Flags: camino1.1a1? → camino1.1a1-
Flags: camino1.1?
Any thoughts on alternative approaches?  This remains very annoying ;)
Flags: camino1.1b1?

Comment 14

11 years ago
Created attachment 246246 [details] [diff] [review]
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.
Assignee: nick.kreeger → stuart.morgan
Status: NEW → ASSIGNED
Attachment #246246 - Flags: review?

Comment 15

11 years ago
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+

Updated

11 years ago
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 17

11 years ago
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)

Comment 18

11 years ago
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.

Comment 20

11 years ago
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

11 years ago
(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

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

Comment 23

11 years ago
(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

11 years ago
(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

11 years ago
(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

11 years ago
> 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

11 years ago
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).

Comment 29

11 years ago
(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

10 years ago
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

Comment 33

9 years ago
Created attachment 297177 [details] [diff] [review]
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.
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....
Created attachment 297308 [details]
crash log

Doesn't look terribly informative :(

Comment 36

9 years ago
(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 → ---

Updated

8 years ago
Assignee: stuart.morgan+bugzilla → nobody

Updated

8 years ago
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
Last Resolved: 3 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.