When using the bookmark.plist file attached in the next comment Camino will take minutes if not tens of minutes to display bookmarks in the bookmarks bar, bookmarks menu and bookmarks manager. Property List editor opens it in a couple of seconds. The user claims the Mozilla and Firefox are able to handle the original .xml file with out problems and wishes Camino would to. Please don't publisze the bookmarks file publically outside this bug. It's ment for test purposes only.
we need to profile this
Status: NEW → ASSIGNED
Target Milestone: --- → Camino0.9
Adding qawanted per comment 2. Mike, how does one profile? Is that something I could do?
*** Bug 243697 has been marked as a duplicate of this bug. ***
*** Bug 244628 has been marked as a duplicate of this bug. ***
Shark suggests that we are spending an inordinate amount of time in __CFXNotificationPost while loading the large bookmark file. I checked bit, and we are totally insane with posting notifications for bookmark updates during loading (this is all per bookmark): -Once when the title is set -Once when the status is set -Once when the number of visits is set -Once when the icon is set (done when URL is set) -Once when the status is set *again* (done when the URL is set) -Possibly more that I didn't see at a glance. So there are two things that would help. Simply skipping the setter functions, or alternately using a flag to supress notifications during the setters then posting one notification manually, would probably give us 5x or more improvement in bookmark loading speed. The better method might be to skip notifications entirely during the load process, then do some sort of full refresh of everything that depends on those changes all at once. I'll try option 1 and see how much that helps.
Just supressing the redundant notifications only helped by a factor of about 2 (the icon notification is distinct, so it was 5->2 not 5->1). However, I found out that there is already a system in place for all interested parties to learn that the bookmark manager is loaded, and update toolbar/bmview/menu/whatever appropriately. This patch adds two things: -One is a per-item way of temporarily suppressing updates then firing one off when done. This is used in the redundant setting to make them roughly twice as fast when using those functions after loading (importing bookmarks, etc.) -The other is a bookmark-wide suppression of *all* NSNotification events that would be sent, which is turned on during initial bookmark loading, then turned off when finished. My testing for just the bookmark loading function on the attached bookmark file: Before: 55-60 seconds After: ~8 seconds Unfortunatly, the subsequent filling of the bookmark menu takes 14-15 seconds, so there's still a pretty sizable delay on loading. We should leave the bug open for more work, but this is a substantial improvement.
I looked over the patch and it looks great. However, it should be generated using "diff -u" instead of whatever it is now. I'll test it more soon. Nice work.
Attachment #149544 - Flags: review?(qa-mozilla) → superreview?(pinkerton)
Comment on attachment 149544 [details] [diff] [review] prevent notifications during load sr=pink landed on trunk and branch. should this stay on the 0.8 list?
Unfortunately, I don't see any obvious way to do a similar thing for the menu constructions notifications, which is where we spend most of our time now. |setMenuChangedMessagesEnabled| sounded promising, but doesn't seem to apply to addItem notifications. Maybe it would be possible to create a custom NSNotificationQueue that would do batching automatically? Since nothing else is obvious, pushing to 0.9. Also lowering severity to normal, since it's significantly less painful now.
Severity: major → normal
Target Milestone: Camino0.8 → Camino0.9
Pushing to 1.0 since this is not a big problem any more and there are no plans to do more work.
Target Milestone: Camino0.9 → Camino1.0
Is this still an issue in recently nightlies?
(In reply to comment #12) > Is this still an issue in recently nightlies? Still an issue. I tested this with an optimized build from today and it took about 45 seconds (on a 1ghz G4) until I could do anything after launching. I even see this with a much smaller bookmarks file, mine is only 340kb but there is a period of 3-5 seconds after launch when I can't do anything with the browser (it beachballs). If I delete most of the bookmarks I can use the browser immediately on launch. Disabling favicons knocked a few seconds off the beachballing.. it used to be 10 seconds +. This was the first thing I noticed after switching over from Firefox.
(In reply to comment #13) > Still an issue. I tested this with an optimized build from today and it took > about 45 seconds (on a 1ghz G4) until I could do anything after launching. > You need to test on a non-optimized build for us to recognize it as still an issue. Thank you.
i think simon's investigating this one.
Assignee: pinkerton → sfraser_bugs
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
Depends on: 279994
Priority: -- → P2
(In reply to comment #14) > You need to test on a non-optimized build for us to recognize it as still an > issue. Thank you. OK. Re-tested with latest official nightly. I got the same result. Still an issue.
Yup, working on it. I have that 45 seconds down to about 10 or so...
In brief, the issues here are: 1. writing the spotlight metadata blocks the UI thread for ~12 seconds 2. when loading site icons, every bookmark gets notified of every site icon load (or failure to load) (this is O(N^2)) 3. we save the bookmarks file every time you go Back/Forward (because visit counts change)
also 4.We build the entire hierarchy of bookmarks menus up-front, which takes a few seconds. We should build all but the main Bookmarks menu lazily, on click.
(In reply to comment #19) > 2. when loading site icons, every bookmark gets notified of every site icon > load (or failure to load) (this is O(N^2)) We should probably have the BM manager do the listening and updating instead of individual bookmarks (I think this was mentioned earlier). > 3. we save the bookmarks file every time you go Back/Forward (because visit > counts change) Maybe we should just be incrementing things in memory and only saving the file periodically and on quit, like (IIRC) the user defaults database does? > 4.We build the entire hierarchy of bookmarks menus up-front, which takes a few > seconds. We should build all but the main Bookmarks menu lazily, on click. I agree, but if you mean building the submenus lazily, we may have to wait until we drop 10.2 support, as the Cocoa methods for lazy menu population don't exist until 10.3.
(In reply to comment #20) > > 4.We build the entire hierarchy of bookmarks menus up-front, which takes a few > > seconds. We should build all but the main Bookmarks menu lazily, on click. > > I agree, but if you mean building the submenus lazily, we may have to wait until > we drop 10.2 support, as the Cocoa methods for lazy menu population don't exist > until 10.3. We already have code to do this for history which works on 10.2.
I just checked in a large set of performance improvements here. Checkin comment: Optimize the performance of bookmarks loading in Camino so that we launch must faster with large bookmarks files. Changes include: * Change from having every bookmark register for page and favicon load notifications to having the BookmarksManger watch for loads, and then notify the relevant bookmarks. Requires the BookmarksManager to maintain two maps, one of page url -> bookmark, and another of favicon url -> bookmark. * Now we build the bookmarks menus lazily on display: BookmarkMenu is now an NSMenu subclass. * The writing of Spotlight Metadata for bookmarks now happens on a thread, after we've set up the rest of the bookmarks. * We no longer save some empty keys for bookmarks and folders to the bookmarks plist file, which reduces its size (and saving cost). Also changed some bookmarks members from being object types (NSNumber) to primitive types. * Delay the loading of bookmark icons until requested (rather than when the bookmarks url is set on bookmark loading). * Bookmark changed notifications now go out with a set of change flags, so we know what changed and can avoid saving bookmarks for trivial changes. * We no longer save the bookmarks when you tab between fields in the Info panel, if you didn't change anything. * Sundry bookmarks code cleanup for readability and leg-cocking. * We now save custom (i.e. <link>-based) favicon URLS with bookmarks, which allows us to avoid hitting the Necko cache for every bookmark up-front as we try to figure out the favicon url for a bookmark. * The site icon provider code now provides info in the "loaded" notification about whether the load came from the network. Now with that 3+Mb file, starting is much faster, with no pizza wheel.
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Summary: Camino takes ages to load a 3.1mb bookmarks.plist file. → Camino takes ages to load a 3.1mb bookmarks.plist file (huge bookmarks).
You need to log in before you can comment on or make changes to this bug.