Closed Bug 471595 Opened 13 years ago Closed 13 years ago
Talos performance tests do not set dom
.max _chrome _script _run _time to zero
As far as I can see talos performace tests do not set dom.max_chrome_script_run_time to 0 in the profile to run tests, only dom.max_script_run_time is zeroed. (Please mark this bug is invalid if it is not so!) This means that if a chrome script runs for more than 20 seconds (the default value for dom.max_chrome_script_run_time), the system may show the slow script warning dialog which will result in the test reported as busted. This could the reason why the current patch from the bug 453157 (1.9.1 blocker) could not pass the testing. With the patch the timing accounting becomes more precise and the 20 seconds limit could expire on a busy box.
We've specifically not set dom.max_chrome_script_run_time as it has caught a regression in the past having to do with slow script warnings that were being raised inappropriately. All talos machines are dedicated boxes so there is no 'busy' box issue, all a talos box does is run the given test. Is there any reason that we shouldn't consider a slow script warning being raised where one was not seen before as something other than a regression?
(In reply to comment #1) > All talos machines are dedicated boxes so there is no 'busy' box issue, all a > talos box does is run the given test. The problem is that at least with TraceMonkey tinderboxes the total timing for gfx tests varies from 30-40 seconds to over 3 minutes with Windows tests. If a cycle collector would kick in during the tests, I suspect it may well push the timing for one of the pages from the test beyond 20 seconds. At least this what happens when we run the tests locally. So if setting max_chrome_script_run_time to 0 is not an option, at least bumping it to, say 100 (100 seconds) should be considered. Otherwise I suspect the patch for bug 453157 would never land. It makes the accounting for long-running scripts much more reliable and the current default limit 20 is not enough. Note also that the patch itself already contains the changes to multiply the limits by 10 for debug builds that do the mochitests. The need for that was discovered during the previous attempts to land the patch.
Okay, this sounds more reasonable now. I'm always wary of making changes to the test harness to ensure tests passing - in case we are just avoiding investigating a regression.
Assignee: nobody → anodelman
Priority: -- → P2
Attachment #354901 - Flags: review?(aki) → review+
Comment on attachment 354901 [details] [diff] [review] [Checked in]add requested preference to talos configuration Looks good.
Comment on attachment 354901 [details] [diff] [review] [Checked in]add requested preference to talos configuration /cvsroot/mozilla/testing/performance/talos/sample.config,v <-- sample.config new revision: 1.23; previous revision: 1.22 done
Is this fixed so, for example, the talos boxes for http://tinderbox.mozilla.org/showbuilds.cgi?tree=TraceMonkey uses the profile with dom.max_chrome_script_run_time set to 0?
Any input on the previous comment?
IANATalosGuru, but the change you requested has been checked in, and Talos machines pull that file from the CVS tip at the start of every run, so I think you are good to go.
Yes, the checked in change affects all automated talos runs regardless of branch.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.