Closed Bug 805841 Opened 11 years ago Closed 10 years ago

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


(Core :: JavaScript Engine, defect)

Not set





(Reporter: ted, Assigned: jmaher)


(Blocks 1 open bug, )


(Whiteboard: [js:t])


(1 file)

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]
This seems to solve the problem using a similar approach to as suggested above.
Assignee: general → jmaher
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+
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.