from irc: <sayrer> sspitzer regressed Ts last night <sayrer> sspitzer, yeah Txul and Ts on all platforms :/ <mfinkle> sspitzerMsgMe: not even a little bit :) <mfinkle> 956ms -> 972ms <mfinkle> on linux <mfinkle> Ts <sspitzerMsgMe> part of my patch does some work in browser.js one time (creating the "Places" folder), on delayed startup. but if we start with a new profile, or a profile-from-before-my-fix, we'd pay that price every time. <sspitzerMsgMe> sayrer / mfinkle: let me take this to a bug, I'll cc you guys. my plan would be to make the default profile have the places folder and the queries (so the hidden browser.places.createdDefaultQueries pref would be set to true) and then see the Ts / Txul numbers. otherwise, we're measuring code that that the user runs once.

rob, can you help me with changing the default profile for tinderbox?

I don't think the profile issue is the problem: the Ts test does N startups (10?) and discards the slowest one. And that doesn't explain the Txul times anyway.

> I don't think the profile issue is the problem does it use a new profile each time or do start ups #2-#10 use the profile created from start up #1? For Txul, I'm creating a new folder on the personal toolbar and inserting 6 bookmarked queries into it, which could affect Txul. But even without creating the new folder, we'd still have a new folder on the personal toolbar that wasn't there before, which could affect Txul.

For the Ts regression: looking at StartupPerformanceTest in http://lxr.mozilla.org/mozilla/source/tools/tinderbox/build-seamonkey-util.pl#3211, it appears that we kill the process after each run to figure out the best start up time (out of 10). in my code for bug #387996, I'm setting a hidden pref to avoid re-adding the special "Places" folder (with the pre-defined queries) on the personal toolbar folder. since we aren't flushing prefs to disk via an explicit call to savePrefFile(), what might be going on is that we are paying the "one time only" cost each time.

I've disabled the feature in bug #387996 by setting the default pref to be true in browser/app/profile/firefox.js: +// disable adding the "Places" folder with pre-defined queries +pref("browser.places.createdDefaultQueries", true); before bug #387996: Txul:243ms Ts:956ms after bug #387996: Txul:256ms Ts:972ms after disabling bug #387996 via a pref: Txul:239ms Ts:957ms For the Txul test (./firefox.exe -chrome "file:///c:/builds/trunk/mozilla/xpfe/test/winopen.xul"), if we start out with a new profile at the start of opening 10 windows, the first one will pay the "one time only cost" but the rest would then pay for having an additional item in the personal toolbar. The Txul number is an average of 10 runs, that might explain the 16 ms bump.

> The Txul number is an average of 10 runs, that might explain the 16 ms bump. correction, looking at mozilla/xpfe/test/winopen.js and mozilla/ xpfe/test/winopen.xul, Txul is the average of the last 9 of 10 runs, excluding the first one. Investigating if the new "Places" folder on the personal toolbar (created on the first run) explain a 16 ms bump in Txul.

from my local tests (on Mac) this additional folder on the personal toolbar is costing 10 ms. as for Ts, I'd to change the default profile so that we aren't paying the "first run cost" on each run. Alternatively, I could use a "kill" safe way (instead of prefs) to determine if the queries were created. I could call savePrefFile() after setting the pref.

(In reply to comment #7) > from my local tests (on Mac) this additional folder on the personal toolbar is > costing 10 ms. > Creating the folder itself? or creating and filling it? Either way, that number is crazy given that you're really only creating a folder and adding 6 bookmarks to it.

> Creating the folder itself? or creating and filling it? To clarify, I'm not measuring the time to create or fill it, I'm measuring the impact having the folder has on Txul.

Asaf, do you plan on working on Txul, Ts, or both?

* batch(!) * initialize the toolbar only after creating the queries

Comment on attachment 284646 [details] [diff] [review] initialize places-popups lazily r=sspitzer. from my Txul tests on my mac, this will lower our number. Locally, by %1 (5 ms), so I'm curious to see what happens on tbox.

Comment on attachment 284645 [details] [diff] [review] some improvements r=sspitzer, sorry for the delay Asaf. on my mac, these changes save me 12 ms.

mozilla/browser/base/content/browser.js 1.869 mozilla/browser/components/places/content/menu.xml 1.87

the call to savePrefFile() might be coming (for bug #399460, pending review). if so, that should lower Ts.

mano has landed his patches, and with his patches I'm not seeing the leaks (see bug #399418 ), and I've landed the patch for bug #399460 (so Ts should be better), so let's re-enable this feature on tinderbox and get some new Txul / Ts numbers.

after re-enabling, I'm seeing a 10ms cost to Txul and a 10ms cost to Ts MacOSX Darwin 8.8.4 bm-xserve08 Dep Universal Nightly before: Txul:143ms Ts:865ms after: Txul:153ms Ts:874ms Linux bl-bldlnx03 Dep fx-linux-tbox perf test before: Txul:241ms Ts:956ms after: Txul:248ms Ts:966ms

Looks like Asaf's views patch got most of the Txul hit back by fixing our views to not be insane, which is where we were hurting from the added bits to the default profile. Ts went down some, but not completely. Without the Places folder, does the patch regress anything, or are we just screwing with the baseline by adding more stuff to the profile? I'm going to move this out until we can answer this more clearly.

One question to ask is whether we need 6 views in that menu.

That menu isn't even built until you open it...

The rest of the Ts regression was fixed in bug 401753.

> The rest of the Ts regression was fixed in bug 401753. Cool, nice work! But, if I remember correctly, we still pay a Txul price for having an additional folder on the personal toolbar. this bug covered both Ts and Txul. Should we spin up a new one about just Txul?

All/most of the Txul part was fixed by bug 399729 afaict.

> All/most of the Txul part was fixed by bug 399729 afaict. short version: ok long version: I think we need a new bug about the toolbar and Txul details: When I last tried on my mac, I was able to consistently measure that every item on the personal toolbar affected Txul. (And, Txul impacts Ts.) small fluctuations in Ts is a wild goose that I prefer not to chase. since we are calling savePrefFile(), and Ts takes the best of <n> times, the "real" Ts cost of creating this folder and these queries is not being measured.) I think it would be worth re-measuring, so see what the current cost is. it might be linear (per item), or worse. (New profiles have 3 items, but if "real" users have more, then it could be worse, and worth profiling / investigating / fixing to improve Ts / Txul.) from my notes, I also think we might be calling updateChevron() twice. Note, We're going have to pay some Ts / Txul price for this "places" folder. I'll log a perf bug on these issues.

plenty of bake time

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