smart--petea / gyp

Automatically exported from code.google.com/p/gyp
0 stars 0 forks source link

Parallel processing fails with multiple input files #347

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
> What steps will reproduce the problem?

1. Make two .gyp files where both gyp files eventually add a dependency on the 
same gyp file.
2. Run gyp --parallel with both gyp files as inputs on the command line

Unfortunately I can't attach the real gyp project (it's quite large), but I 
think these are the relevant properties of it.

> What is the expected output? What do you see instead?

Fails with an "error occurred while running gyp using multiprocessing." 
message. Rerunning with GYP_PARALLEL=0 or without --parallel runs succesfully 
with no errors.

Expected: anything that works without it should work with --parallel too.

> What version of the product are you using? On what operating system?
r1633, Linux

> Please provide any additional information below.

Did a bit of debugging in the Gyp code and believe that the core of the issue 
is that when LoadTargetBuildFile encounters an already-loaded build file, it 
returns "False". In non-parallel builds this is fine, because all those code 
paths ignore the return value. But in parallel builds, the return value has to 
be (buildfile, deps) and a false value is interpreted as an error.

Now, the fact that this case even happens at all is a bit weird - the parallel 
gyp stuff already has code to make sure that every file is visited only once. 
So why is the same target being loaded twice in the first place?

Turns out that a required circumstance to trigger the return value mismatch bug 
is that multiple .gyp files are given on the command line. Even with 
--parallel, the top-level input files gets processed serially, and the code for 
avoiding double work only sees the files processed during the current call to 
LoadTargetBuildFileParallel.

So A.gyp is processed and its dependency C.gyp gets loaded, then B.gyp is 
processed with a fresh empty parallel_state.scheduled, tries to load C.gyp 
again and crashes.

I think a start would be to process all the inputs in one batch when using the 
parallel mechanism, that should make it slightly faster/more parallel and also 
works around this bug.

Might also be a good time to let the single-threaded code pass through the same 
queue-up-dependencies logic that is currently used by the parallel code path. 
Then you get rid of the two different calling conventions for 
LoadTargetBuildFile, and more stuff would work the same in the parallel and 
non-parallel cases.

Original issue reported on code.google.com by ols...@gmail.com on 10 Jun 2013 at 1:40

GoogleCodeExporter commented 9 years ago
Submitted a fix at https://codereview.chromium.org/23475007

Original comment by sim...@opera.com on 29 Aug 2013 at 11:58

GoogleCodeExporter commented 9 years ago

Original comment by thakis@chromium.org on 15 Oct 2013 at 6:34

GoogleCodeExporter commented 9 years ago
(fyi, --parallel will go away in 6 months if it isn't on by default by then: 
https://code.google.com/p/chromium/issues/detail?id=154205#c22 )

Original comment by thakis@chromium.org on 15 Oct 2013 at 6:35