[webkit-reviews] review granted: [Bug 81379] NRWT should not take memory used as disk cache into account when deciding how many processes to launch : [Attachment 148225] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel (OOO until 6/19) <eric at webkit.org> has granted Dirk Pranke
<dpranke at chromium.org>'s request for review:
Bug 81379: NRWT should not take memory used as disk cache into account when
deciding how many processes to launch
https://bugs.webkit.org/show_bug.cgi?id=81379

Attachment 148225: Patch
https://bugs.webkit.org/attachment.cgi?id=148225&action=review

------- Additional Comments from Eric Seidel (OOO until 6/19) <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=148225&action=review


I predict that you'll find folks who complain about this change. :)  But it's
fine with me.  I don't believe it's possible to make everyone happy here.

I might suggesting just changing the int/long thing first and seeing if that
makes more people happy. :)

> Tools/Scripts/webkitpy/common/system/platforminfo.py:86
> -	       return int(self._executive.run_command(["sysctl", "-n",
"hw.memsize"]))
> +	       return long(self._executive.run_command(["sysctl", "-n",
"hw.memsize"]))

I wonder if this fix alone would be enough. :)

> Tools/Scripts/webkitpy/layout_tests/port/mac.py:131
> +	   overhead = 2048 * 1024 * 1024  # Assume we need 2GB free for the O/S


This is similar to what I originally implemented when I first added this
feature.   Didn't work very well on loaded machines with other things running. 
But none of these heuristics work well in all cases.  I'm happy to use whatever
heuristics makes the largest % of people happy.  If this is that, then great.
:)