Process all input files in parallel

Process all input files in parallel

When given several input files on the command line in parallel mode,
load them all in parallel instead of doing a bunch of separate parallel
runs.

Fixes gyp --parallel for projects with multiple .gyp files.

Patch from simonb@opera.com

BUG=gyp:347
R=dmazzoni@chromium.org, scottmg@chromium.org

Committed: https://code.google.com/p/gyp/source/detail?r=1753

[Simon Brenner, 2013-08-29 12:34:17]

Not quite sure who to set as a reviewer(s) for this as the OWNERS file just says
"*", but it looks like Dominic wrote the original parallelization code that this
modifies.

[dmazzoni, 2013-08-29 15:04:45]

lgtm

Thanks! One of the gyp owners should also approve, but this looks reasonable.

https://codereview.chromium.org/23475007/diff/1/pylib/gyp/input.py
File pylib/gyp/input.py (right):

https://codereview.chromium.org/23475007/diff/1/pylib/gyp/input.py#newcode560
pylib/gyp/input.py:560: parallel_state.dependencies = list(build_files)
You're converting |build_files| to both a list and a set...what is it initially?

It might be cleaner to assume the argument is always a list, and create a set
only when needed. When it's a set, call it build_file_set

https://codereview.chromium.org/23475007/diff/1/pylib/gyp/input.py#newcode2596
pylib/gyp/input.py:2596: LoadTargetBuildFilesParallel(build_files, data,
aux_data,
Fix indentation

[Simon Brenner, 2013-08-29 16:15:48]

https://codereview.chromium.org/23475007/diff/1/pylib/gyp/input.py
File pylib/gyp/input.py (right):

https://codereview.chromium.org/23475007/diff/1/pylib/gyp/input.py#newcode560
pylib/gyp/input.py:560: parallel_state.dependencies = list(build_files)
On 2013/08/29 15:04:46, Dominic Mazzoni wrote:
> You're converting |build_files| to both a list and a set...what is it
initially?
> 
> It might be cleaner to assume the argument is always a list, and create a set
> only when needed. When it's a set, call it build_file_set

If |build_files| is already a list, list() serves to make a copy that we can
safely modify. Likewise with set() when the argument is already a set.

Since |dependencies| and |scheduled| will both be updated continuously by the
loop below and by ParallelState.LoadTargetBuildFileCallback, I wanted to work on
copies to avoid surprising the caller.

In this case, I believe the collection is always a set, so I guess
build_file_set? Then again, I like that the code is agnostic in the type of
collection that comes in, it just needs it to be iterable.

[dmazzoni, 2013-08-29 16:23:00]

On 2013/08/29 16:15:48, Simon Brenner wrote:
> If |build_files| is already a list, list() serves to make a copy that we can
> safely modify. Likewise with set() when the argument is already a set.

Oh! I didn't think about the fact that making a copy was intentional. Maybe
comment that?

> In this case, I believe the collection is always a set, so I guess
> build_file_set? Then again, I like that the code is agnostic in the type of
> collection that comes in, it just needs it to be iterable.

I'm fine with that - commenting that the copy is intentional will make it more
clear

[Simon Brenner, 2013-09-18 14:07:07]

Hmm, looks like I just made codereview send out an empty e-mail last time...
Expected it to e.g. include or mention the new patch sets in that mail, but
alas.

Dominic suggested this should be reviewed by a gyp owner as well - so adding
scottmg as reviewer.
Hope that's OK, looks like you've reviewed gyp changes before.

[scottmg, 2013-09-18 17:32:15]

lgtm

[M-A Ruel, 2013-09-20 19:53:17]

On 2013/09/18 17:32:15, scottmg wrote:
> lgtm

CQ is not working for GYP, have a committer commit on your behalf.

[Simon Brenner, 2013-10-11 13:27:16]

On 2013/09/20 19:53:17, M-A Ruel wrote:
> On 2013/09/18 17:32:15, scottmg wrote:
> > lgtm
> 
> CQ is not working for GYP, have a committer commit on your behalf.

Ended up working on other things for a while, but maybe I should try to poke
this patch forward again :)

Any gyp committers willing to commit this for me?

[scottmg, 2013-10-11 17:01:28]

Committed patchset #4 manually as r1753.