On Android 4.3 Debug, test_value_storage.html runs in mochitest-25, which runs for about 2 hours now -- much higher than our target job run time, and on the cusp of exceeding the 2 hour limit for the job. The job sometimes exceeds 2 hours, failing as in bug 1317602. We would normally increase the number of Android Debug mochitest chunks to reduce the run time of each chunk, but in this case, the chunks are very unbalanced: mochitest-25 runs for 2 hours, but all other chunks run in under 90 minutes, and some run in under 60 minutes. test_value_storage.html is consistently the slowest test in mochitest-25, often taking about 30 minutes. (It was timing out recently and the test timeout increased in bug 1244978.) Could the run time of this test be reduced significantly? Could it be skipped on Android Debug?

For comparison, test_value_storage approximate run times on other platforms: Android opt - 10 minutes Linux64 opt - 2 minutes Linux64 debug - 3 minutes

$ ./mach file-info bugzilla-component layout/style/test/test_value_storage.html Core :: CSS Parsing and Computation layout/style/test/test_value_storage.html

Do you have any idea what the performance issues are on the hardware? Is the test CPU-bound, I/O-bound (due to test output), or are we swapping due to low memory?

I don't know. I can look into it a bit. I'm open to suggestions for experiments/measurements. Your mention of test output reminded me of bug 1045205 and the long history of this test...but I don't know if that's relevant today.

There are many, many assertions in this test, resulting in lots of structured log activity. If the body of StructuredLog.testStatus() is commented out, as in https://treeherder.mozilla.org/#/jobs?repo=try&revision=387ac483a0070f139357c3ad222d7c4ea0448841, the run time of Android Debug test_value_storage is reduced from 30 minutes to under 10 minutes. So it looks like any performance improvement from bug 1045205 would go a long way here.

This drastically reduces the logging during test_value_storage.html, by guarding almost all is()/isnot() calls with conditionals. One isnot() is left unguarded, at the top of test_value() to provide a gross measure of progress and ensure regular log writes (avoid no-output timeout). There is a significant reduction in run time: linux64 debug: 3 minutes -> 1 minute android opt: 10 minutes -> 4 minutes android debug: 30 minutes -> 10 minutes I would prefer to see this resolved with drastic improvements to logging efficiency, but I don't think that is a realistic expectation. As an alternate "solution", we could disable this test on Android Debug.

So when actually debugging test failures (which are very common in this test for people adding new CSS properties or values), it's often very useful to have the failures in the context of where they are in the test. Similar for debugging against try server, which people sometimes have to do. So I'd prefer that the change here only be done for Android, and not for all platforms. Likewise, I'd prefer that you not have to edit the substance of the test and make it less readable. (It's already hard enough -- but that's another issue!) So, given those two things, I'd prefer that you wrap the existing is/isnot functions with new is/isnot functions that are used in this file. This has three advantages: * you don't have to edit the body of the test at all * the wrapping can easily conditional (e.g., make it Android-only for now) * the conditions under which we do the wrapping can be easily changed (You'll probably need a dummy test to call the real "ok" function in the function that does the wrapping so that you don't get a no-tests-run failure, though.) Does that sound reasonable?

That's a great idea! This works fine for me locally. https://treeherder.mozilla.org/#/jobs?repo=try&revision=d38272403e218edb9ffa6217185fd14a655f8ac8 is in progress for good measure. Logging on Android is reduced to one TEST-PASS for each call to test_value(); logging is unchanged on all other platforms.

Comment on attachment 8812235 [details] [diff] [review] reduce logging in test_value_storage.html, on Android only >+// On Android, avoid most 'TEST-PASS' logging, to improve performance Maybe explicitly say "by overriding the is and isnot functions" here? r=dbaron

Pushed by gbrown@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/07ed7e5fa5ff Reduce logging in test_value_storage.html on Android only; r=dbaron