Closed Bug 578343 Opened 10 years ago Closed 10 years ago
talos needs to be retrofitted for e10s
7.13 KB, application/octet-stream
8.47 KB, patch
|Details | Diff | Splinter Review|
currently talos uses quit.js and this doesn't work when running inside of e10s. We need to look through all the talos code and make sure we proxy everything to chrome process.
Is there an equivalent bug for unit tests? Looks like we have false greens on maemo devices on unit tests (bug 579184); the browser isn't actually launched.
this patch requires the mobile_profile.zip (which I will upload next) and works on fennec desktop linux. This changes the profile for mobile from base_profile to mobile_profile and mobile_profile has an extension which adds an overlay (same technique as mochitest)
Assignee: nobody → jmaher
Attachment #461104 - Flags: review?(aki)
this is the new profile directory.
Comment on attachment 461106 [details] the new mobile_profile with the talos e10s extension (1.0) I'll be staging this and the other patches shortly. One thing about this zip -- you included the CVS subdirectories, which means any changes or cvs actions on this directory tree will affect the directory you copied it from (base_profile) instead of the new subdirectory tree you want to create. So you want to remove mobile_profile/bookmarkbackups/CVS, mobile_profile/Cache/CVS and mobile_profile/CVS.
Comment on attachment 461104 [details] [diff] [review] core changes required to get ts, twinopen working on talos (1.0) Things are looking good here, other than fennecmark (bug 582997). The mobile config changes are easy peasy to r+. Have you verified that the other changes won't break desktop?
Comment on attachment 461106 [details] the new mobile_profile with the talos e10s extension (1.0) This appears to work. Do you know if this will conflict at all with the new pageloader xpi? I'm putting that in distribution/bundles (bug 580698) and that also has a quit.js. Other than that question and the CVS dirs, this looks good to me.
I don't know much about the pageloader xpi stuff. It could conflict, but when is that landing? This solution is more of a temporary shim until we have a more complete solution in place. As for the CVS dirs, I have updated the patch to just have the extensions directory (all other dirs just had CVS cruft in there)
(In reply to comment #7) > I don't know much about the pageloader xpi stuff. It could conflict, but when > is that landing? I'm testing it in conjunction with this. I can switch over to that pageloader at any point.
Comment on attachment 461638 [details] the new mobile_profile with the talos e10s extension (2.0) Did you delete the zip before zipping everything up? zip will add to an archive, not overwrite it. new-host-3:foo asasaki$ unzip ../mobile_profile.zip Archive: ../mobile_profile.zip inflating: mobile_profile/localstore.rdf inflating: mobile_profile/prefs.js creating: mobile_profile/bookmarkbackups/ creating: mobile_profile/bookmarkbackups/CVS/ extracting: mobile_profile/bookmarkbackups/CVS/Entries inflating: mobile_profile/bookmarkbackups/CVS/Repository extracting: mobile_profile/bookmarkbackups/CVS/Root creating: mobile_profile/Cache/ creating: mobile_profile/Cache/CVS/ extracting: mobile_profile/Cache/CVS/Entries extracting: mobile_profile/Cache/CVS/Repository extracting: mobile_profile/Cache/CVS/Root creating: mobile_profile/CVS/ inflating: mobile_profile/CVS/Entries inflating: mobile_profile/CVS/Entries.Log extracting: mobile_profile/CVS/Repository extracting: mobile_profile/CVS/Root creating: mobile_profile/extensions/ creating: email@example.com/ creating: firstname.lastname@example.org/chrome/ creating: email@example.com/chrome/content/ inflating: firstname.lastname@example.org/chrome/content/ipc-overlay.xul inflating: email@example.com/chrome/content/ipc.js inflating: firstname.lastname@example.org/chrome/content/quit.js inflating: email@example.com/chrome.manifest inflating: firstname.lastname@example.org/install.rdf
updated .zip to be just the few files we need for the mobile_profile.
Comment on attachment 463239 [details] [checked in]the new mobile_profile with the talos e10s extension (2.1) /me changes zip from patch->application/octet-stream again :)
Comment on attachment 463239 [details] [checked in]the new mobile_profile with the talos e10s extension (2.1) I'm going to stamp this, since it can't make anything worse, and I think even if it does conflict with the new pageloader, I did see some successful talos runs with this.
Attachment #463239 - Flags: review?(aki) → review+
Comment on attachment 461104 [details] [diff] [review] core changes required to get ts, twinopen working on talos (1.0) This has wfm in testing (spotty, because of network flakiness, but I've seen greens). However, I'm still concerned that quit.js and getInfo.html changes might affect desktop talos negatively, so keeping the Alice r?
Attachment #461104 - Flags: review?(aki) → review+
Have these changes been testing on staging talos?
They have not, afaik.
I would want at least a small run in staging before we consider changing the basic functionality.
I've agreed to do the staging for jmaher for this patch.
updated patch to work better with firefox desktop (in non ipc mode). Tested on linux desktop fennec (ipc on/off) and firefox (non ipc). I did not do any tp tests or pageloader tests. Alice if you could indicate when this might get a change at some talos staging that would be helpful.
I will start a staging run on this tomorrow morning, so that I can leave my current staging test overnight to run to completion.
Staging run started.
So far I'm seeing green mixed in with quite a bit of orange related to failure to initialize the browser. I'm trying to sort through it now and I'll work with jmaher tomorrow to see if this can be resolved.
after looking over this code, it is the same code I am using in mochitest which has passed on all platforms and has been running in this fashion on all tinderbox unittests (for mochitest) for the last 7 weeks. With that said there still could be issues with the difference in setup and how Firefox is being run.
From the test runs overnight the issue is only on mac talos (snow leopard/leopard). The patch causes the browser to fail to be initialized and no tests are run. As such this is r- for production landing.
So I ran these tests on my MBP OSX and it completed a run (I didn't have the TP4 stuff setup), but ts, ts_cold, ts_shutdown, ts_places*, twinopen all ran and produced results (in the output/*.csv format.) I did nothing to change my code, I just used the patches applied to this bug. Could it be there was a bad build of OSX Firefox when testing these in staging?
I'm also seeing success when I manually ran on a single slave (talos-r3-leopard-002). This is going to require a second staging run to determine what is going on.
We are getting some mixed results in staging, but our confidence is high enough that we'd like to land during a downtime with a close, close eye on mac talos results.
Checking in getInfo.html; /cvsroot/mozilla/testing/performance/talos/getInfo.html,v <-- getInfo.html new revision: 1.4; previous revision: 1.3 done Checking in mobile.config; /cvsroot/mozilla/testing/performance/talos/mobile.config,v <-- mobile.config new revision: 1.6; previous revision: 1.5 done RCS file: /cvsroot/mozilla/testing/performance/talos/mobile_profile/localstore.rdf,v done Checking in mobile_profile/localstore.rdf; /cvsroot/mozilla/testing/performance/talos/mobile_profile/localstore.rdf,v <-- localstore.rdf initial revision: 1.1 done RCS file: /cvsroot/mozilla/testing/performance/talos/mobile_profile/prefs.js,v done Checking in mobile_profile/prefs.js; /cvsroot/mozilla/testing/performance/talos/mobile_profile/prefs.js,v <-- prefs.js initial revision: 1.1 done RCS file: /email@example.com/chrome.manifest,v done Checking in firstname.lastname@example.org/chrome.manifest; /email@example.com/chrome.manifest,v <-- chrome.manifest initial revision: 1.1 done RCS file: /firstname.lastname@example.org/install.rdf,v done Checking in email@example.com/install.rdf; /firstname.lastname@example.org/install.rdf,v <-- install.rdf initial revision: 1.1 done RCS file: /email@example.com/chrome/content/ipc-overlay.xul,v done Checking in firstname.lastname@example.org/chrome/content/ipc-overlay.xul; /email@example.com/chrome/content/ipc-overlay.xul,v <-- ipc-overlay.xul initial revision: 1.1 done RCS file: /firstname.lastname@example.org/chrome/content/ipc.js,v done Checking in email@example.com/chrome/content/ipc.js; /firstname.lastname@example.org/chrome/content/ipc.js,v <-- ipc.js initial revision: 1.1 done RCS file: /email@example.com/chrome/content/quit.js,v done Checking in firstname.lastname@example.org/chrome/content/quit.js; /email@example.com/chrome/content/quit.js,v <-- quit.js initial revision: 1.1 done Checking in page_load_test/quit.js; /cvsroot/mozilla/testing/performance/talos/page_load_test/quit.js,v <-- quit.js new revision: 1.2; previous revision: 1.1 done Checking in startup_test/fennecmark/fennecmark.html; /cvsroot/mozilla/testing/performance/talos/startup_test/fennecmark/fennecmark.html,v <-- fennecmark.html new revision: 1.2; previous revision: 1.1 done
Attachment #463239 - Attachment description: the new mobile_profile with the talos e10s extension (2.1) → [checked in]the new mobile_profile with the talos e10s extension (2.1)
Attachment #464577 - Attachment description: core changes required to get ts, twinopen working on talos (2.0) → [checked in]core changes required to get ts, twinopen working on talos (2.0)
Joel, John, Alice: you guys rock.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.