[Webkit-unassigned] [Bug 37377] Don't reinvent Executive.cpu_count for every port
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat Apr 10 07:56:47 PDT 2010
--- Comment #7 from Adam Barth <abarth at webkit.org> 2010-04-10 07:56:46 PST ---
> > + 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.
Ok. It seems better to just fix the bug than to create something complicated
to work around the issue.
> 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.
How about a base.default_dump_render_tree_count method? The number of cores
your machine has doesn't vary by port, but the number of DRT instances you want
> 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.
There's really two things going on. We have a notion of a port (Gtk, Chromium,
Qt, etc) and a notion of a platform (Mac, Windows, Linux). At the moment, the
two are smashed together, which leads to the copy/paste code.
In this particular case, the OS-specific code is just making up for a
deficiency in the Python 2.5 library. In Python 2.6, the multiprocessing
package abstracts away the platform specific logic for counting CPUs.
In any case, I'll write a patch that I think will make you happy. :)
> > @@ -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 "
> 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?
The name "platform" conflicts with the import platform statements I added at
the top of the file.
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