fennecmark needs to be updated for e10s

RESOLVED FIXED

Status

Testing
Talos
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: jmaher, Assigned: jmaher)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 3 obsolete attachments)

(Assignee)

Description

8 years ago
Created attachment 461247 [details] [diff] [review]
udpates to fennecmark to work with e10s

updates to fennecmark to work in an e10s environment.
Attachment #461247 - Flags: review?
Attachment #461247 - Flags: feedback?(mark.finkle)
(Assignee)

Comment 1

8 years ago
Created attachment 461248 [details] [diff] [review]
changes to talos code to work with e10s fennecmark

this is the talos part of this patch and requires the fennecmark update patch to work.
Assignee: nobody → jmaher
Attachment #461248 - Flags: review?(aki)
Comment on attachment 461247 [details] [diff] [review]
udpates to fennecmark to work with e10s

approach looks good to me
Attachment #461247 - Flags: feedback?(mark.finkle) → feedback+

Comment 3

8 years ago
Created attachment 461375 [details]
tpan errors on n900 mozilla-central staging run

Hm, did this work for you?
I'm getting a ton of errors (attached) when running with this in staging.
(Assignee)

Comment 4

8 years ago
those are the errors I see everytime I run linux desktop fennec (after the e10s merge last month)

You do see results mixed in there though:)

Comment 5

8 years ago
Yes, though it didn't actually get to the graph server post.
It's certainly possible that isn't related, though.

Comment 6

8 years ago
Failed to initialize browser (m-c).
I'm testing with the new pageloader, the new mobile_profile, and this patch.
(Assignee)

Comment 7

8 years ago
I recall that this wasn't working for aki.  I found that we need to use the distribution/bundles stuff.  Basically check out the fennecmark tree into <appdir>/distribution/bundles/fennecmark

Then this should work.

Comment 8

8 years ago
I believe I have this running in mobile staging, with a timeout for tzoom and a can't initialize browser for tpan; no green runs.  The pageloader tarball has distribution/bundles/pageloader.xpi and distribution/bundles/fennecmark/

Let me verify that your patch is in distribution/bundles/fennecmark/.

Comment 9

8 years ago
My mistake. Updated the pageloader tarball and we're now testing your patch correctly; I will update this later with status.

Comment 10

8 years ago
Ok, progress: we're now timing out in both tpan and tzoom.

Do you know what that timeout is?  Is that the talos timeout?

NOISE: ###################################### forms.js loaded
NOISE: ###################################### content loaded
NOISE: !! remote browser loaded
NOISE: loading about:blank, 1
NOISE: loading file:///builds/talos/startup_test/fennecmark/fennecmark.html?test=PanDown, 1
NOISE: [TabChild] MOVE to (x,y)=(0d, 0d), (w,h)= (0d, 0d)
NOISE: [TabChild] MOVE to (x,y)=(0d, 0d), (w,h)= (800d, 424d)
Failed tpan: 
		Stopped Wed, 11 Aug 2010 21:51:04
FAIL: Busted: tpan
FAIL: timeout exceeded
(Assignee)

Comment 11

8 years ago
that isn't much information to go on.  Did the page load and tests run?  From what I see here it did some page loading, but that is it.  What OS is this on?  I get timeouts on linux desktop because fennec is so slow a pan or zoom eats up all my cpu.  But that is in general not related to Talos, although I do get noise in the log about the numbers.

Comment 12

8 years ago
This is n900-gtk.
We can land this if you're fine with the timeouts, which is an improvement (fails later in the process).

Updated

8 years ago
Attachment #461247 - Flags: review? → review+

Comment 13

8 years ago
Comment on attachment 461248 [details] [diff] [review]
changes to talos code to work with e10s fennecmark

We're getting a lot further here.
Attachment #461248 - Flags: review?(aki) → review+

Comment 14

8 years ago
Created attachment 466707 [details]
tzoom errors

I watched this one -- it was opening the browser, zooming in, zooming out, then closing the browser, repeat.  Not sure what all the nulls are for?
(Assignee)

Comment 15

8 years ago
so it appears that tpan is working (some of the time) and failing others.  For tZoom (I have seen the log file results above) it seems like we are timing out.  The browser loads the page, but never actually zooms.  Doing the zoom manually I was unable to get it to work.  After a few tries of running tZoom, I was able to see some real zooming happen, but after a few zooms (not the full set for a single tZoom run) it timed out.

I think with mozilla-central fennec (both n900 and linux desktop) we have a lot of performance issues.

Comment 16

8 years ago
(In reply to comment #12)
> This is n900-gtk.
> We can land this if you're fine with the timeouts, which is an improvement
> (fails later in the process).

Joel: is this the approach you want to take, or do you want to have a fully green solution before we land?

Aiui a fully green solution will require listening for the panning/zooming hooks landed in bug 586313, correct?
(Assignee)

Comment 17

8 years ago
I honestly have no idea why this is failing.  Listening for pan/zoom events would not hurt in this test, but my experience has been that it takes so long to zoom that we timeout just as a result of waiting for the zoom to complete.

If others are seeing something else, please let me know and we can debug it from there.
(Assignee)

Comment 18

8 years ago
Created attachment 474981 [details] [diff] [review]
ipc + file logging (non stdout)

superset patch of the IPC code + the ability to log talos results to a local file instead of just stdout (required for android)
Attachment #474981 - Flags: review?(aki)
(Assignee)

Comment 19

8 years ago
I have an idea on how to fix the tzoom from emitting 'null' for the values.  I can get to that tomorrow most likely.
(Assignee)

Updated

8 years ago
Blocks: 579566
(Assignee)

Comment 20

8 years ago
Created attachment 475087 [details] [diff] [review]
ipc + file logging (non stdout) + tzoom fix! (3.0)

updated code that encompasses all code needed for IPC file logging (using talos.logfile preference) instead of stdout, and now a fix for tzoom using the ZoomChanged event.  

This should allow us to see:
1) tpan/tzoom results for maemo (n900/n810)
2) tpan/tzoom results for android

there is still some question about the quality of the results for tpan and tzoom.  Some intermittent failures here and there.
Attachment #461247 - Attachment is obsolete: true
Attachment #474981 - Attachment is obsolete: true
Attachment #475087 - Flags: review?(aki)
Attachment #475087 - Flags: feedback?
Attachment #474981 - Flags: review?(aki)

Comment 21

8 years ago
Comment on attachment 475087 [details] [diff] [review]
ipc + file logging (non stdout) + tzoom fix! (3.0)

Put this in staging with last night's m-c nightly:

(lots of exceptions)

NOISE: [Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getCharPref]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: chrome://fennecbench/content/report.js :: dumpLog :: line 78"  data: no]
NOISE: undefined
WARNING: failed os.kill: 3 : No such process
Failed tzoom: 
		Stopped Tue, 14 Sep 2010 10:30:22
FAIL: Busted: tzoom
FAIL: timeout exceeded
Completed test tzoom: 
		Stopped Tue, 14 Sep 2010 10:30:22

---

NOISE: [TabChild] MOVE to (x,y)=(0d, 0d), (w,h)= (980d, 519d)
NOISE: 424,422,184 pageH, pageBottom, timeToPan
NOISE: [Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getCharPref]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: chrome://fennecbench/content/report.js :: dumpLog :: line 78"  data: no]
NOISE: undefined
Failed tpan: 
		Stopped Tue, 14 Sep 2010 10:27:42
FAIL: Busted: tpan
FAIL: timeout exceeded
Completed test tpan: 
		Stopped Tue, 14 Sep 2010 10:27:42

I can attach full logs if needed.
(Assignee)

Comment 22

8 years ago
Created attachment 475143 [details] [diff] [review]
ipc + file logging (non stdout) + tzoom fix! (3.1)

updated patch to try/catch around getCharPref when talos.logfile is not present.

Ran on n900 with results as expected.  Also with the regular talos, not a patched version which I have locally.
Attachment #475087 - Attachment is obsolete: true
Attachment #475143 - Flags: review?(aki)
Attachment #475087 - Flags: review?(aki)
Attachment #475087 - Flags: feedback?

Comment 23

8 years ago
Comment on attachment 475143 [details] [diff] [review]
ipc + file logging (non stdout) + tzoom fix! (3.1)

I'm not that strong at js, so if you want a code review, you should probably add someone.

But this went green in staging on m-c for tpan+tzoom.
Attachment #475143 - Flags: review?(aki) → review+
(Assignee)

Comment 24

8 years ago
Comment on attachment 475143 [details] [diff] [review]
ipc + file logging (non stdout) + tzoom fix! (3.1)

taras, this is your original code.  I changed a few things here and could use an eye on it before committing it to your repo
Attachment #475143 - Flags: review?(tglek)

Comment 25

8 years ago
Comment on attachment 475143 [details] [diff] [review]
ipc + file logging (non stdout) + tzoom fix! (3.1)

>+      this._test = (new this.tests[this.currentTest](this));
Don't need the outer () here.

I barely recognize the code at this point, but here is an r+.
Attachment #475143 - Flags: review?(tglek) → review+
(Assignee)

Comment 26

8 years ago
landed as http://hg.mozilla.org/users/tglek_mozilla.com/fennecmark/pushloghtml?changeset=5e8a04f38530
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.