on first time migration of a fx2 profile with livemarks, are we starting up the livemark service? I think the answer is yes (from a simple LOG() in nsLivemarkService.js) , but I need to get a stack trace to prove it. I'm seeing us start it up even before the first browser window comes up. If possible, we should delay instantating the livemark service on startup like we are attempting to do in normal fx3 startup.

(In reply to comment #0) > on first time migration of a fx2 profile with livemarks, are we starting up the > livemark service? Yes, I don't think we can avoid it in the initial import case. We call the livemark servce in order to create the livemark folders. Is there evidence this is causing performance problems that users actually notice?

kind of related to bug 363634, which is more about http traffic, than about service initiation. this bug (if it is one) could also apply to the microsummary service, which we also kick off during import.

well, we probably need the livemark service in nsPlacesImportExportService.cpp on import, see HandleLinkEnd(). but note, in nsPlacesImportExportService::nsPlacesImportExportService(), we instantiate the livemark service, and that will set up a 15 second (seems aggressively fast) that when it fires will update all the livemarks. also note, there are bugs about making this less aggressive. but I still think we should avoid doing this on import / startup. I believe sayrer (or mano) have done this in other parts of our code. perhaps we want to move the code the code that starts the timer out of the livemark service constructor, and into a method of the livemark service. then we can start the updates of livemarks (which we will make less aggressive) from the browser later, once we've started up and shown UI, etc. comments?

updating summary. to answer robert's question, I don't have numbers to show this is a real perf problem. but you know how we have your fix for bug #363636 in toolbar.xml that duplicates LivemarkService.getSiteURI to avoid instantiating the service on startup? http://lxr.mozilla.org/seamonkey/source/browser/components/places/content/toolbar.xml#252 Was that for a perf gain on startup? If so, this would be related (first time migration perf startup gain). but dietrich is right, I am trying to avoid the http traffic which I want to avoid on first time migration startup, for both livemarks and microsummaries. (which sounds like bug #363634)

(In reply to comment #4) > > but you know how we have your fix for bug #363636 in toolbar.xml that > duplicates LivemarkService.getSiteURI to avoid instantiating the service on > startup? Yes, it seems like the right thing to avoid instantiating the service and kicking of traffic for a normal browser startup. > > http://lxr.mozilla.org/seamonkey/source/browser/components/places/content/toolbar.xml#252 > > Was that for a perf gain on startup? If so, this would be related (first time > migration perf startup gain). iirc, it was, though I admit I didn't spend as much time measuring as I should have. My question is about the value of a "first time migration perf startup gain". I don't understand why we're focusing on that... but I admit I am totally clueless there.

"iirc, it was, though I admit I didn't spend as much time measuring as I should have." separate from your change, we should determine if this fix is worth the cost. >My question is about the value of a "first time migration perf startup > gain". I don't understand why we're focusing on that... We have bugs about how first time launching trunk is really slow for people with lots of bookmarks and history. we've got bugs on that, and I think it's bad enough where we want to make it better for the <n> millions of folks who will get upgraded to fx 3. but note, you can run into this same issue on first start up after a schema change. (Although eventually, we have plans not to use bookmarks.html for that, so that may not be a problem.) I think it makes more sense to focus on making the livemark service less aggressive first, so I'll start there, before coming back to this bug.

This is wanted, as that first time trying out the new Firefox will be important, but not blocking.

here's one approach: manually start up refresh at a specified time.

(In reply to comment #7) > This is wanted, as that first time trying out the new Firefox will be > important, but not blocking. > Hm, marked as blocking. Doesn't matter, as this effectively fixes 363634 as well (which is another blocker).

this is causing problems locally when running xpcshell tests, making it difficult to test bookmarks import. i think that import starts up the livemark service update timer, and which gets shut down before fully initialized, because the test is very short.

Comment on attachment 294138 [details] [diff] [review] unrotted chatting with dietrich over irc, this has a few issues which he is addresing.

now doesn't restart timer w/ each window load.

Comment on attachment 294165 [details] [diff] [review] v2 r=sspitzer I bet we could do some fancy memoization to avoid needing the the _started boolean, but I think this is fine. one request, can you elaborate in the comment in browser.js on why we are waiting 15 seconds? other thoughts: maybe we should be keeping track of the timer, and then canceling it in _shutdown()? this._livemarkTimer = new G_Alarm(); and we could use this._livemarkTimer as our boolean, instead of _started and then in _shutdown(), we would do: if (this._livemarkTimer) { this._livemarkTimer.cancel() this._livemarkTimer = null; } do you think we'll ever have a need to stop the refresh timer, like when going offline? all of this can be spun off, if you prefer.

(In reply to comment #13) > (From update of attachment 294165 [details] [diff] [review]) > r=sspitzer > > I bet we could do some fancy memoization to avoid needing the the _started > boolean, but I think this is fine. to do that, i'd have to make it a write-able attribute in the interface, which would be confusing IMO. > > one request, can you elaborate in the comment in browser.js on why we are > waiting 15 seconds? done. also, i changed to 5 seconds and moved above the DM. 15 secs was far too long. > > other thoughts: > > maybe we should be keeping track of the timer, and then canceling it in > _shutdown()? > done > do you think we'll ever have a need to stop the refresh timer, like when going > offline? > i tried using ioService.offline, but it did not ever properly reflect my offline status. filed bug 410101 to follow this up.

Comment on attachment 294776 [details] [diff] [review] v3 r=sspitzer

i should've noted this earlier: i haven't checked this in because i think i might need to fix the livemark tests to kick off the update loop.

(In reply to comment #17) > i should've noted this earlier: i haven't checked this in because i think i > might need to fix the livemark tests to kick off the update loop. > they explicitly reload specific livemark folders, so we should be ok.

Checking in browser/base/content/browser.js; /cvsroot/mozilla/browser/base/content/browser.js,v <-- browser.js new revision: 1.927; previous revision: 1.926 done Checking in toolkit/components/places/public/nsILivemarkService.idl; /cvsroot/mozilla/toolkit/components/places/public/nsILivemarkService.idl,v <-- nsILivemarkService.idl new revision: 1.9; previous revision: 1.8 done Checking in toolkit/components/places/src/nsLivemarkService.js; /cvsroot/mozilla/toolkit/components/places/src/nsLivemarkService.js,v <-- nsLivemarkService.js new revision: 1.35; previous revision: 1.34 done

need to keep an eye out for feedback about whether we're starting the update timer too late. might need to tweak it to startup a little sooner.

I think this caused bug 412953

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h". In Thunderbird 3.0b, you do that as follows: Tools | Message Filters Make sure the correct account is selected. Click "New" Conditions: Body contains places-to-b-and-h Change the action to "Delete Message". Select "Manually Run" from the dropdown at the top. Click OK. Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter. Gerv