[Webkit-unassigned] [Bug 37377] Don't reinvent Executive.cpu_count for every port
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Apr 9 20:27:32 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=37377
--- Comment #5 from Dirk Pranke <dpranke at chromium.org> 2010-04-09 20:27:31 PST ---
(From update of attachment 53026)
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
b/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
> index e5a0108..5185947 100755
> --- a/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
> +++ b/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
> @@ -50,6 +50,7 @@ import logging
> import math
> import optparse
> import os
> +import platform
> import Queue
> import random
> import re
> @@ -71,6 +72,8 @@ from test_types import image_diff
> from test_types import test_type_base
> from test_types import text_diff
>
> +from webkitpy.common.system.executive import Executive
> +
> import port
>
> _log = logging.getLogger("webkitpy.layout_tests.run_webkit_tests")
> @@ -1389,6 +1392,7 @@ def main(options, args):
> stream=meter)
>
> port_obj = port.get(options.platform, options)
> + executive = Executive()
>
> if not options.configuration:
> options.configuration = port_obj.default_configuration()
> @@ -1422,8 +1426,13 @@ def main(options, args):
> ignore_errors=True)
>
> if not options.num_dump_render_trees:
> - # TODO(ojan): Investigate perf/flakiness impact of using numcores + 1.
> - options.num_dump_render_trees = port_obj.num_cores()
> + # FIXME: Investigate perf/flakiness impact of using cpu_count + 1.
> + options.num_dump_render_trees = executive.cpu_count()
> + # FIXME: new-run-webkit-tests is unstable on Mac running more than
> + # four threads in parallel.
> + # See https://bugs.webkit.org/show_bug.cgi?id=36622
> + if platform.system() == "Dawin" and options.num_dump_render_trees > 4:
> + options.num_dump_render_trees = 4
The instability isn't generic, it's port-specific, and there should be a
port-specific way to limit parallelism. For example, the fact that the base mac
port is unstable due to DumpRenderTree doesn't mean that we should limit the
parallelism that Chromoium gets.
I think a better way to implement this patch would've been to have
base.num_cores() call the Executive module, and leave the hook in for any ports
that didn't want the default logic.
I went pretty far ensuring that anything platform-specific needed by
run-webkit-tests would be wrapped up in the Port interface, so that a Port only
needs to change things in one place. The rest of the Python code does not seem
to have a similar since of layering (as evidenced by the switch statement in
system/executive.py). Please don't undo this layering without replacing it with
something equivalent or at least having more of a discussion about this.
>
> write = create_logging_writer(options, 'config')
> write("Running %s DumpRenderTrees in parallel" % options.num_dump_render_trees)
> @@ -1461,9 +1470,9 @@ def main(options, args):
> # Creating the expecations for each platform/configuration pair does
> # all the test list parsing and ensures it's correct syntax (e.g. no
> # dupes).
> - for platform in port_obj.test_platform_names():
> - test_runner.parse_expectations(platform, is_debug_mode=True)
> - test_runner.parse_expectations(platform, is_debug_mode=False)
> + for platform_name in port_obj.test_platform_names():
> + test_runner.parse_expectations(platform_name, is_debug_mode=True)
> + test_runner.parse_expectations(platform_name, is_debug_mode=False)
> meter.update("")
> print ("If there are no fail messages, errors or exceptions, then the "
> "lint succeeded.")
I have no objection to this change, but it doesn't seem to have anything to do
with the rest of this patch. Did it sneak in accidentally?
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list