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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[(not currently active) Ted Mielczarek]

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?

Comment 1

[Zack Weinberg (:zwol)]

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.

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: 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.

Comment 3

[Zack Weinberg (:zwol)]

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").

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d2175a963653