WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
73847
NRWT creates too many workers on RAM-limited machines
https://bugs.webkit.org/show_bug.cgi?id=73847
Summary
NRWT creates too many workers on RAM-limited machines
Adam Roben (:aroben)
Reported
2011-12-05 11:10:31 PST
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.
Attachments
Patch
(8.78 KB, patch)
2011-12-05 15:04 PST
,
Eric Seidel (no email)
dpranke
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1
2011-12-05 11:17:44 PST
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)
Comment 2
2011-12-05 11:18:23 PST
How much ram should NRWT expect a DRT instance to need? 500mb?
Tony Chang
Comment 3
2011-12-05 11:23:16 PST
(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
Comment 4
2011-12-05 12:20:51 PST
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)
Comment 5
2011-12-05 15:04:27 PST
Created
attachment 117944
[details]
Patch
Dirk Pranke
Comment 6
2011-12-05 15:22:52 PST
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)
Comment 7
2011-12-05 15:37:54 PST
(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 8
2011-12-05 15:45:06 PST
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)
Comment 9
2011-12-05 15:48:59 PST
Committed
r102056
: <
http://trac.webkit.org/changeset/102056
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug