from https://bugzilla.mozilla.org/show_bug.cgi?id=449640#c17: *********** before this lands, we must be able to measure performance of various actions that flex this code. a low-bar approach i've been thinking about is to write chrome mochitests that contain perf scores in the test comments. for example, we could write a mochitest that times the opening of the history menu, and the test comment could be "ui-perf|places|open history menu|20ms" or something like that. eventually we could build a datastore and UI, making it generally useful, but the log data alone provides a simple solution for our immediate needs. *********** the attached patch provides a basic test framework for doing this, and tests the history and bookmarks sidebars. there's no reporting at all yet - need to manually look at the logs before/after your check-in. issues: - should up the history visits to 10,000 - or maybe should do multiple runs: empty profile, small profile, large profile - i'm not able to get the menus to open yet, so that test is disabled other tests that should be added: - open the library - add different quantities of visits, bookmarks - remove different quantities of visits, bookmarks - annotations - switch between the various history sidebar views

this blocks landing of the partitioning code. we need to get baseline data before landing.

basic framework and some tests to start with. we should get this in asap, to start building baseline data for these tests, to determine if the numbers are stable enough to be useful. can run tests manually like so: $ cd ~/path/to/objdir/_tests/testing/mochitest $ python runtests.py --browser-chrome --test-path=../browser/browser/components/places/tests/perf/ in tinderbox logs, you can find test results by searching for "ui-perf-test".

You know that these won't run by default because they aren't prefixed with test_ right? I do think we should get some automated system to add perf tests though. Also, I think it'd be best to have each one of these in it's one file so if we ever regress things, it's easy to profile (and shark hooks like I did in the other file would be nice, IMO).

(In reply to comment #3) > You know that these won't run by default because they aren't prefixed with > test_ right? these are browser chrome tests, so needs be prefixed with browser_. see: http://developer.mozilla.org/en/Browser_chrome_tests > > I do think we should get some automated system to add perf tests though. > > Also, I think it'd be best to have each one of these in it's one file so if we > ever regress things, it's easy to profile (and shark hooks like I did in the > other file would be nice, IMO). agreed, will break em out.

oh...the existing test in that path is a chrome test, so I was making the assumption that these were as well.

Comment on attachment 337122 [details] [diff] [review] basic test set could you add a test that opens the awesomebar too? that would be interesting it would be interesting also changing history sidebar view by, clearly this is a first step so you don't have to put all tests now :) also for checkin i would cleanup the latest part with commented out, commenting on why it's commented out + var bookmarks_total = total_visits/10; // bookmark a tenth of your visits + var bookmarks_per_day = bookmarks_total/days; + + // reset visit date counter + visit_date_microsec = Date.now() * 1000; if you reset visit_date_microsec you're not bookmarking your visits, rather you're adding new bookmarks on new pages, so the comment "// bookmark a tenth of your visits" is not completely correct + make_test_report("open_history_sidebar", duration) what about adding a unit, like duration+"ms" also i get TEST-UNEXPECTED-FAIL because of empty tests... Maybe you shoul repeat the test 10 times, discard worst and best and then take an average time? How can we track these results for future? good starting point!

> also i get TEST-UNEXPECTED-FAIL because of empty tests... which empty tests? i do not see this. > > Maybe you shoul repeat the test 10 times, discard worst and best and then take > an average time? sounds good, will do. > > How can we track these results for future? future bug

(In reply to comment #7) > > also i get TEST-UNEXPECTED-FAIL because of empty tests... > > which empty tests? i do not see this. this +// test opening of the library +ptests.push({ + run: function() { + } +});

changes: - broke tests out into separate files - added report param for specifying units - added tests for the different history sidebar views - run UI tests 10x times, report avg sans highest/lowest issues for followup: - menu test is still disabled - shared header

sorry for late (damn late!), i've unbitrotted this for you, so forgive me :) + +/** + * + * Update Behavior in Firefox: + * + * Five seconds after the browser UI starts up, the livemark service's update + * timer is started via nsILivemarkService.start(). The update timer fires + * immediately and by default every 15 minutes (is dynamically calculated as + * gExpiration/4), with a maximum limit of 1 hour. + * + * When the update timer fires, it iterates over the list of livemarks, and will + * refresh a livemark *only* if it's expired. + * + * The expiration time for a livemark is determined by using information + * provided by the server when the feed was requested, specifically + * nsICacheEntryInfo.expirationTime. If no information was provided by the + * server, the default expiration time is 1 hour. + * + * Users can modify the default expiration time via the + * browser.bookmarks.livemark_refresh_seconds preference. + * + */ Since we are about changing this i'd remove the description for now fixe lines over 80 chars where possible check for exceptions when instantiating services apart that, r=me

Comment on attachment 337673 [details] [diff] [review] unbitrot WTF, I posted on wrong bug :\

Comment on attachment 337605 [details] [diff] [review] more + var bookmarks_total = total_visits/10; // bookmark a tenth of your URLs still not clear, these are NEW urls, not our urls :) however not a problem +const TEST_COUNT = 10; what about TEST_REPEAT_COUNT? + // XXX does not work, is still open=false immediately after setting it to true + menu.open = true; does it change something using menu.lastChild.showPopup()? +/* +Tests the performance of various UI actions. + +setup: +- add XXX visits distributed over XXX days +- add XXX bookmarks distributed over XXX days + +tests done: +- add many visits +- add many bookmarks +- open the default history sidebar view +- open the bookmarks sidebar + +tests todo: +- open the history menu +- open the bookmarks menu +- open the other history sidebar views +- open the library +- delete many bookmarks +- clear history +- annotations? +- autocomplete? + +*/ this is in history sidebar, should not talk about other tests Is it complex adding a test that opens and close the awesomebar?

Jonas, Ben T, and I were talking about performance tests in mochitest today (regarding an unrelated matter). How do you plan to address the problem that the tests may bog down on a unit test machine (those machines are flaky with their run times) and cause spurious oranges on tboxen? Maybe we can use what you've got for other performance related mochitests.

(In reply to comment #13) > Jonas, Ben T, and I were talking about performance tests in mochitest today > (regarding an unrelated matter). How do you plan to address the problem that > the tests may bog down on a unit test machine (those machines are flaky with > their run times) and cause spurious oranges on tboxen? don't know. i'm going to check these in, and watch the numbers to see if they're at all reliable. if it's too noisy, i'll get a dedicated set of boxes to run only these tests. or something.

tree is closed. will check-in when it re-opens.

checked-in: http://hg.mozilla.org/mozilla-central/rev/4fb45fa51685

Dietrich: Let me know how well the numbers report. I might want to use this approach for other types of testing too.

A windows user has reported doing the "Cut" action on a folder that had over 3000 bookmarks in multiple subfolders was taking over half an hour, with process monitor showing millions of accesses to places.sqlite / places.sqlite-journal

<Megzlna> the file offset its reading in process monitor is changing very slowly it reads the entire 12MB then does it over and over again every time rereading the whole file and the last offset it reads is slowly going down. <Megzlna> judging by watching process monitor, it appears that the whole entire 12MB is being re-read for every single entry in the bookmarks <Megzlna> so like, reread 12MB 3000 times <Megzlna> it got all the way down to 0MB finally <Megzlna> and then went back to 8MB <Megzlna> it finished, so about 1:15 total <Megzlna> And all my data is gone <Megzlna> the folder is gone, and there's no "Paste" <Megzlna> possibly because I cut and pasted something in the past hour (expletives removed)

(In reply to comment #18) > A windows user has reported doing the "Cut" action on a folder that had over > 3000 bookmarks in multiple subfolders was taking over half an hour, with > process monitor showing millions of accesses to places.sqlite / > places.sqlite-journal which version of the browser?

Dietrich, can we resolve this and file a new bug about using the provided data in some sort of graph/tabular view?

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

Not working on this anymore, so unassigning.

the purpose is still good, but the method is not, we also removed these tests from the tree. telemetry is better.

Yeah, this bug is dead, thanks for coming back around and closing it. Telemetry is good, but not for per-check-in regression testing. Any new perf tests should use peptest once it's up and running on tbpl: https://wiki.mozilla.org/Auto-tools/Projects/peptest.