All users were logged out of Bugzilla on October 13th, 2018

test_pm.xul fails on EC2 VM because it can't measure CPU data

RESOLVED FIXED in mozilla21

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: ted, Assigned: jmaher)

Tracking

(Blocks: 1 bug)

Trunk
mozilla21
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [js:t], URL)

Attachments

(1 attachment)

jmaher is testing out running our unit tests on EC2 VMs. test_pm.xul is one of the few test failures we have left:
14362 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/components/perf/test_pm.xul | all events measurable - got 1920, expected 2047

It looks like what's happening here is that the jsperf component is unable to get any of the CPU measurement counters (like CPU_CYCLES), perhaps something to do with running in a VM. Can we change the test to be more lenient here, and accept "got some sort of measurement" as a pass condition?
Hm, so we can only measure context switches and page faults in one of these VMs? Those are perhaps the *least* interesting events...

But since I wrote the thing, as far as I can tell there has been zero interest in this module (if nothing else I expected someone to do an OSX or Windows back end by now) so I agree we should not let it stand in the way of other desirable work.

I think it would be best if someone who can work directly with these EC2 VMs wrote the patch, and then I reviewed it.  The line with the failures you showed should just be deleted (if it's changed to "isnot(pm.eventsMeasured, 0)" then it's redundant with the check for a stub implementation immediately above) and then each of the isnot()s on lines 31-41 needs to assume the form

    ((pm.eventsMeasured & PerfMeasurement.THING) ? isnot : todo_isnot)(pm.thing, -1, "thing");

or anyway that *should* be the right change.
Whiteboard: [js:t]
(Assignee)

Updated

6 years ago
Blocks: 834725
(Assignee)

Comment 2

6 years ago
Created attachment 712585 [details] [diff] [review]
mark as todo if we don't record any data (1.0)

This seems to solve the problem using a similar approach to as suggested above.
Assignee: general → jmaher
Status: NEW → ASSIGNED
Attachment #712585 - Flags: review?(zackw)
Comment on attachment 712585 [details] [diff] [review]
mark as todo if we don't record any data (1.0)

Don't you need to delete line 25 (a little bit above the patch) too?

     is(pm.eventsMeasured, PerfMeasurement.ALL, "all events measurable");

r+ with that, or if you can explain why it's not necessary.  I like replacing the chain of isnot()s with a loop (although "iter" is ick, please just use "i").
Attachment #712585 - Flags: review?(zackw) → review+
https://hg.mozilla.org/mozilla-central/rev/d2175a963653
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.