Bug 82112

Created attachment 133609 [details] Patch

Comment on attachment 133609 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133609&action=review > Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:763 > - if self._options.worker_model == 'inline': > + if self._options.child_processes == 1: Should we use self._options.child_processes == 0 for inline (rather than child_processes == 1)? In the inline model, there aren't any child processes, right?

(In reply to comment #2) > (From update of attachment 133609 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=133609&action=review > > > Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:763 > > - if self._options.worker_model == 'inline': > > + if self._options.child_processes == 1: > > Should we use self._options.child_processes == 0 for inline (rather than child_processes == 1)? In the inline model, there aren't any child processes, right? Depends on how you look at it, but the real answer is that child_processes is actually kind of a bad name, it should really be --workers or --jobs.

(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 133609 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=133609&action=review > > > > > Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:763 > > > - if self._options.worker_model == 'inline': > > > + if self._options.child_processes == 1: > > > > Should we use self._options.child_processes == 0 for inline (rather than child_processes == 1)? In the inline model, there aren't any child processes, right? > > Depends on how you look at it, but the real answer is that child_processes is actually kind of a bad name, it should really be --workers or --jobs. Another way of looking at it is that --child-processes actually refers to the # of DRTs running in parallel; what's going on in the Python code is beside the point.

Comment on attachment 133609 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133609&action=review I like that this patch deletes a bunch of code. I'm slightly worried about overloading the semantics of --child-processes, but I don't think that's a big deal. > Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:790 > + This change seems spurious.

(In reply to comment #5) > (From update of attachment 133609 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=133609&action=review > > I like that this patch deletes a bunch of code. I'm slightly worried about overloading the semantics of --child-processes, but I don't think that's a big deal. I'd be surprised if anybody but me really used --worker-model=inline much. > > > Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:790 > > + > > This change seems spurious. Indeed. Will fix. Thanks for the review!

Change committed in http://trac.webkit.org/changeset/112171. The changelog on that change is wrong due to a bad merge (or possibly a bug in webkit-patch, I'm not sure).