Bug 73847 – NRWT creates too many workers on RAM-limited machines

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Adam Roben (:aroben)]

NRWT creates the same number of workers regardless of how much RAM is installed. On RAM-limited machines (e.g., 12 cores, 3GB RAM), so many workers are created that the machine starts paging like crazy and all tests end up timing out.

It would be nice if NRWT were to take RAM into account when deciding how many workers to create.

[Eric Seidel (no email)]

http://support.hyperic.com/display/SIGAR/Home is a way to do this.  But I don't think we can auto-install that, due to the binary files it includes.  I suspect we'll have to roll our own if we want this functionality.  Juse like we do for Executive.cpu_count()

[Eric Seidel (no email)]

How much ram should NRWT expect a DRT instance to need?  500mb?

[Tony Chang]

(In reply to comment #2)
> How much ram should NRWT expect a DRT instance to need?  500mb?

500mb sounds plenty conservative.  Make sure to leave ram for the system and NRWT.

[Dirk Pranke]

It hadn't occurred to me before that we might have a lot of many-core but low-RAM machines. Makes sense to change this.

[Eric Seidel (no email)]

Created attachment 117944 [details]
Patch

[Dirk Pranke]

Comment on attachment 117944 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=117944&action=review

> Tools/Scripts/run-webkit-tests:79
> +    return !isQt();

Are we sure that it was a RAM issue and not something else?

Also, in the future I'd probably change the defaults in port/mac.py et al. rather than duplicating the logic here in Perl.

> Tools/Scripts/webkitpy/layout_tests/port/base.py:177
> +        return cpu_count

I'm not sure zero should be a legal value here, so maybe you should return at least 1? If it is zero (essentially saying we need at least 300MB to run), there should probably at least be a check in run_webkit_tests: _set_up_derived_options() to handle zero cleanly and bail out.

[Eric Seidel (no email)]

(In reply to comment #6)
> (From update of attachment 117944 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=117944&action=review
> 
> > Tools/Scripts/run-webkit-tests:79
> > +    return !isQt();
> 
> Are we sure that it was a RAM issue and not something else?

I don't know.  But it doesn't hurt to try.

> Also, in the future I'd probably change the defaults in port/mac.py et al. rather than duplicating the logic here in Perl.

This is intentional.  run-webkit-tests is meant to match ORWT, to be a shim.  Folks who run new-run-webkit-test directly always get parallel testing, regardless of the state of the bots.

> > Tools/Scripts/webkitpy/layout_tests/port/base.py:177
> > +        return cpu_count
> 
> I'm not sure zero should be a legal value here, so maybe you should return at least 1? If it is zero (essentially saying we need at least 300MB to run), there should probably at least be a check in run_webkit_tests: _set_up_derived_options() to handle zero cleanly and bail out.

This is no different from setting --child-count=0 directly.  I think that's out of scope of this change.

[Dirk Pranke]

Comment on attachment 117944 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=117944&action=review

>>> Tools/Scripts/webkitpy/layout_tests/port/base.py:177
>>> +        return cpu_count
>> 
>> I'm not sure zero should be a legal value here, so maybe you should return at least 1? If it is zero (essentially saying we need at least 300MB to run), there should probably at least be a check in run_webkit_tests: _set_up_derived_options() to handle zero cleanly and bail out.
> 
> This is no different from setting --child-count=0 directly.  I think that's out of scope of this change.

I disagree, in the sense that there was no way default_child_processes() could return 0 before, and now it can, potentially breaking things down the road. That said, it'll probably never come up in practice. Up to you ...

[Eric Seidel (no email)]

Committed r102056: <http://trac.webkit.org/changeset/102056>