Closed Bug 669613 Opened 13 years ago Closed 13 years ago

Create an endurance test that recreates membuster

Categories

(Mozilla QA Graveyard :: Mozmill Tests, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: davehunt, Assigned: davehunt)

References

()

Details

(Whiteboard: [mozmill-endurance][MemShrink:P1])

Attachments

(2 files, 4 obsolete files)

Membuster opens a bunch of sites in new windows to push the memory usage of the browser up. It would be great if we can reproduce this in an adhoc endurance test and measure the impact on performance.

Membuster: http://random.pavlov.net/membuster/index.html
Blocks: 629065
Whiteboard: [mozmill-endurance]
Attached patch Membuster endurance test. v1.0 (obsolete) — Splinter Review
Added a membuster test that uses tabs instead of windows. If needed, I will look into using windows.

To run, you will need Mozmill installed (pip install mozmill). Check out http://hg.mozilla.org/qa/mozmill-tests and switch to the appropriate branch for the Firefox version you want to test before applying the patch.

Use the following command line:

mozmill -b /Applications/Firefox.app -t tests/endurance/adhoc/testMemBuster/test1.js

If you want to be able to see the results you will need to run using the automation scripts. Check out from http://hg.mozilla.org/qa/mozmill-automation and apply the automation scripts patch from this bug.

Use the following command line:

./testrun_endurance.py --report=http://mozmill-archive.brasstacks.mozilla.com/db --repository=../mozmill-tests /Applications/Firefox.app

Where the value of --repository is the location of the patched mozmill-tests repository.

Results will be available at http://mozmill-archive.brasstacks.mozilla.com/#/endurance/reports
Assignee: nobody → dave.hunt
Attachment #544225 - Flags: feedback?(hskupin)
A Pivotal Tracker story has been created for this Bug: https://www.pivotaltracker.com/story/show/15419445
This is cool.  Random thoughts:

- Opening in tabs seems fine, and much more representative of normal browsing than opening in windows.

- What is a Pivotal Tracker story?

- Membuster's strategy (open lots of sites) is a good one, but the selection of sites is odd.  It's mostly wiki pages of one sort or another.  One possibility would be to use the Tp5 site list, which I believe is the top 100 Alexa sites.  Because there's 100 of them, you wouldn't need to open each one more than once, I think.

Whatever the outcome of this bug, it seems like it shouldn't be too hard for it to measure memory usage at the end after closing all the tabs, in which case it would subsume bug 669322, right?
Comment on attachment 544225 [details] [diff] [review]
Membuster endurance test. v1.0

>+++ b/tests/endurance/adhoc/testMemBuster/test1.js

Not sure about the location yet. It's something we will have to discuss. So my feedback on this patch doesn't cover that part.

>+ * Contributor(s):
>+ *   Anthony Hughes <ahughes@mozilla.com> (Original Author)

I assume it's you instead.

>+const TEST_SITES = ["http://www.armchairgm.com/Main_Page",
>+                    "http://www.armchairgm.com/Anderson_Continues_to_Thrive_for_Cleveland",
>+                    "http://www.armchairgm.com/Special:ImageRating",
>+                    "http://bioshock.wikia.com/wiki/Main_Page",
>+                    "http://en.marveldatabase.com/Main_Page",
>+                    "http://uncyclopedia.org/wiki/Main_Page",
>+                    "http://uncyclopedia.org/wiki/Babel:Vi",
>+                    "http://ja.uncyclopedia.info/wiki/",
>+                    "http://spademanns.wikia.com/wiki/Forside",
>+                    "http://www.wowwiki.com/Main_Page",
>+                    "http://pushingdaisies.wikia.com/wiki/Pushing_Daisies",
>+                    "http://creativecommons.org/",
>+                    "http://en.wikinews.org/wiki/Main_Page",
>+                    "http://www.vodcars.com/",
>+                    "http://wikitravel.org/en/China",
>+                    "http://wikitravel.org/en/Main_Page",
>+                    "http://wikitravel.org/ja/",
>+                    "http://wikitravel.org/he/",
>+                    "http://wikitravel.org/hi/",
>+                    "http://wikitravel.org/ru/",
>+                    "http://en.wikipedia.org/wiki/Main_Page",
>+                    "http://ja.wikipedia.org/wiki/",
>+                    "http://ru.wikipedia.org/wiki/",
>+                    "http://zh.wikipedia.org/wiki/",
>+                    "http://de.wikipedia.org/wiki/Hauptseite",
>+                    "http://forums.studentdoctor.net/forumdisplay.php?s=718b9d0e8692d7c3f4cc7c64faffd17b&f=10",
>+                    "http://forums.studentdoctor.net/showthread.php?t=469342",
>+                    "http://joi.ito.com/jp/",
>+                    "http://joi.ito.com/archives/email/"
>+];

We should save all URLs inside a separate txt file and just load it in setupModule.

>+function testMemBuster() {
>+  var numTabs = 30;
>+  var microIterations = 11;
>+  var counter = 0;
>+  var siteIndex = TEST_SITES.length - 1 - counter % TEST_SITES.length;
>+  var numLoads = siteIndex.length * enduranceManager.iterations;

So what should be --iterations and --microiterations later?

>+        enduranceManager.addCheckpoint(counter++ + "/" + numLoads);

Hard to read that part because the string concatenation.

Otherwise I like it. We need to solve some questions first and probably should also run this with new windows to compare the difference in memory usage between windows and tabs.
Attachment #544225 - Flags: feedback?(hskupin) → feedback+
Attached patch Membuster endurance test. v1.1 (obsolete) — Splinter Review
(In reply to comment #6)
> Comment on attachment 544225 [details] [diff] [review] [review]
> Membuster endurance test. v1.0
> 
> >+++ b/tests/endurance/adhoc/testMemBuster/test1.js
> 
> >+ * Contributor(s):
> >+ *   Anthony Hughes <ahughes@mozilla.com> (Original Author)
> 
> I assume it's you instead.

Oops. Fixed.

> >+const TEST_SITES = ["http://www.armchairgm.com/Main_Page",
...
> >+                    "http://joi.ito.com/archives/email/"
> >+];
> 
> We should save all URLs inside a separate txt file and just load it in
> setupModule.

Sounds like a good idea - do you have any examples of this?

> >+function testMemBuster() {
> >+  var numTabs = 30;
> >+  var microIterations = 11;
> >+  var counter = 0;
> >+  var siteIndex = TEST_SITES.length - 1 - counter % TEST_SITES.length;
> >+  var numLoads = siteIndex.length * enduranceManager.iterations;
> 
> So what should be --iterations and --microiterations later?

An iteration is a full run through, so this is likely to be run with only one iteration. A micro-iteration is a single loop through all sites, this is currently hard-coded to 11 based on the original membuster code. Ultimately this will be configurable from the command line like you suggest.

> >+        enduranceManager.addCheckpoint(counter++ + "/" + numLoads);
> 
> Hard to read that part because the string concatenation.

I've made this a little easier to read, and also fixed up some issues. I also dump the progress for extra information during the test.

> Otherwise I like it. We need to solve some questions first and probably
> should also run this with new windows to compare the difference in memory
> usage between windows and tabs.

Will it be simple to do windows? Do you have any good example of this you can point me to?
Attachment #544225 - Attachment is obsolete: true
Attachment #544440 - Flags: feedback?(hskupin)
(In reply to comment #5)
> This is cool.  Random thoughts:
> 
> - Opening in tabs seems fine, and much more representative of normal
> browsing than opening in windows.

It would be good to do a comparison, I'll work on a new window version once this is close to being finished.

> - What is a Pivotal Tracker story?

It's a task tracking tool that we are using to manage the Mozmill tests/automation/dashboard projects.

> - Membuster's strategy (open lots of sites) is a good one, but the selection
> of sites is odd.  It's mostly wiki pages of one sort or another.  One
> possibility would be to use the Tp5 site list, which I believe is the top
> 100 Alexa sites.  Because there's 100 of them, you wouldn't need to open
> each one more than once, I think.

Good idea. I'm going to be splitting the list of the sites into a separate file, and will update it at the same time to the Alexa top 100.

> Whatever the outcome of this bug, it seems like it shouldn't be too hard for
> it to measure memory usage at the end after closing all the tabs, in which
> case it would subsume bug 669322, right?

I wasn't aware of that bug. This would definitely cover that, as memory usage is captured at the beginning of each iteration, after each site is loaded, and at the end of the iteration. Tabs are closed at the end of the iteration, so running for two iterations would be a good way to show what the memory does after the tabs are closed.
Whiteboard: [mozmill-endurance] → [mozmill-endurance][MemShrink:P1]
Comment on attachment 544440 [details] [diff] [review]
Membuster endurance test. v1.1

Functionality wise it looks ok. Re: loading a text file, you could check the bookmarks import from JSON how they are doing that. That way we could store the list of websites directly in JSON format on disk.
Attachment #544440 - Flags: feedback?(hskupin) → feedback+
Attached patch Membuster endurance test. v1.2 (obsolete) — Splinter Review
(In reply to comment #10)
> Comment on attachment 544440 [details] [diff] [review] [review]
> Membuster endurance test. v1.1
> 
> Functionality wise it looks ok. Re: loading a text file, you could check the
> bookmarks import from JSON how they are doing that. That way we could store
> the list of websites directly in JSON format on disk.

I wasn't able to find a way to read in the sites from an external file.

I have updated to use the delay and micro-iterations from the command line, and changed the list of sites to reflect the top 6 from each of Alexa's top sites categories (excluding Adult).
Attachment #544440 - Attachment is obsolete: true
Attachment #546125 - Flags: feedback?(hskupin)
Comment on attachment 546125 [details] [diff] [review]
Membuster endurance test. v1.2

(In reply to comment #11)
> I wasn't able to find a way to read in the sites from an external file.
 
You can ask Marco Bonardo (mak77). He should be able to help you.

>+function testMemBuster() {
>+  var numTabs = 30;
>+  var counter = 0;
>+  var numLoads = enduranceManager.iterations * enduranceManager.microIterations;

Can you add some documentation for what iterations and microIterations are for? Probably that would be helpful for all of our endurance tests. It's hard to figure out if you are not familiar with the code.

>+    for (var i = 0; i < enduranceManager.microIterations; i++) {

We have to figure out that too on the bug you have created. A callback should also work here.
Attachment #546125 - Flags: feedback?(hskupin) → feedback+
> I have updated to use the delay and micro-iterations from the command line,
> and changed the list of sites to reflect the top 6 from each of Alexa's top
> sites categories (excluding Adult).

That's a really good idea, but you're now repeating work already done by Tp5.  And Tp5 tries to be smart, eg. just visiting facebook.com isn't very helpful, you need to login to a real page.  I think Tp5 does this.
The sites can easily be tweaked to go directly to content rather than home pages, and I believe this is what tp5 does[1].

One difference is that we are not waiting for the page to load, and the page load time is not a measurement we are interested in. We are also gathering the metrics before as each site is loaded, which means the interval of the metrics is much more flexible than tp5's 20 seconds.

Another difference is that we will be able to get users to run this test on their machines and report to our Mozmill Crowd dashboard.

We have discussed including 'modules' for interacting with some of the sites (logging in, paging through content, etc) but I'd say this would be a considerable future enhancement, and may even be a completely different endurance test.

[1] https://wiki.mozilla.org/Buildbot/Talos#tp5_2
Summary: Create an endurance tests that recreates membuster → Create an endurance test that recreates membuster
Blocks: 676087
Blocks: 676091
Blocks: 676784
Attached patch Membuster endurance test. v1.3 (obsolete) — Splinter Review
I've raised bug 676784 for loading the list of sites from an external file.
Attachment #546125 - Attachment is obsolete: true
Attachment #550990 - Flags: review?(hskupin)
Comment on attachment 550990 [details] [diff] [review]
Membuster endurance test. v1.3

>+++ b/tests/endurance/adhoc/testMemBuster/test1.js

As discussed in the mail thread it should become 'reserved' or 'requested'. I would be ok for now with either of those.  Btw. we could also completely get rid of such a sub folder. It won't be executed in any way by our testrun because the folder name doesn't start with 'test'.

If we want to use such a command line option then you should probably also rename 'testMemBuster' to 'membuster', so we can map it easier.

>+  prefs.preferences.setPref(TAB_MODAL, false);

Please add the reference to the bug, why we need this.

>+  var numTabs = 30;

That should be a global constant for now.

>+        if (tabBrowser.length < numTabs) {
>+          tabBrowser.openTab();
>+        } else {

Hanging if/else please.

>+      }
>+      var siteIndex = (currentMicroIteration - 1) % TEST_SITES.length;
>+      var site = TEST_SITES[siteIndex];
>+      controller.open(site);
>+      enduranceManager.addCheckpoint("opening " + site);

Please add some blank lines to separate the code a bit for better reading.
Attachment #550990 - Flags: review?(hskupin) → review-
Depends on: 678175
Depends on: 678183
(In reply to Henrik Skupin (:whimboo) from comment #16)
> Comment on attachment 550990 [details] [diff] [review]
> Membuster endurance test. v1.3
> 
> >+++ b/tests/endurance/adhoc/testMemBuster/test1.js
> 
> As discussed in the mail thread it should become 'reserved' or 'requested'.
> I would be ok for now with either of those.  Btw. we could also completely
> get rid of such a sub folder. It won't be executed in any way by our testrun
> because the folder name doesn't start with 'test'.

Renamed to 'reserved'.

> If we want to use such a command line option then you should probably also
> rename 'testMemBuster' to 'membuster', so we can map it easier.

Renamed to 'membuster'.

> >+  prefs.preferences.setPref(TAB_MODAL, false);
> 
> Please add the reference to the bug, why we need this.

Done.

> >+  var numTabs = 30;
> 
> That should be a global constant for now.

Done.

> >+        if (tabBrowser.length < numTabs) {
> >+          tabBrowser.openTab();
> >+        } else {
> 
> Hanging if/else please.

Done.

> >+      }
> >+      var siteIndex = (currentMicroIteration - 1) % TEST_SITES.length;
> >+      var site = TEST_SITES[siteIndex];
> >+      controller.open(site);
> >+      enduranceManager.addCheckpoint("opening " + site);
> 
> Please add some blank lines to separate the code a bit for better reading.

I'm not sure I see a clear place to add a blank line. I've added one after site is defined, but I would normally keep this all together as it's used on the following line.

Note that this bug now depends on bug 678175 and bug 678183.
Attachment #550990 - Attachment is obsolete: true
Attachment #553416 - Flags: review?(hskupin)
Comment on attachment 553416 [details] [diff] [review]
Membuster endurance test. v1.4

>+// XXX: Tab modal dialogs are not yet supported so we switch back to browser modal dialogs (bug 673399)
>+const TAB_MODAL = "prompts.tab_modal.enabled";

As how it can be seen in other tests we usually use the format below. Also please put the comment right above the code which is using the constant. That's a more prominent position.

// XXX: Bug #
//      %Comment%

>+      }
>+      var siteIndex = (currentEntity - 1) % TEST_SITES.length;
>+      var site = TEST_SITES[siteIndex];
>+
>+      controller.open(site);
>+      enduranceManager.addCheckpoint("opening " + site);
>+    });
>+    tabBrowser.closeAllTabs();

Re blank lines, separate blocks of code e.g. before and after brackets. It helps a lot to get a better overview.

If something is unclear just ask me on IRC so we can figure it out and have a final patch next time. r=me with those changes.
Attachment #553416 - Flags: review?(hskupin) → review+
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: