All users were logged out of Bugzilla on October 13th, 2018
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.
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+
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.