On 2 Jul 2014, at 11:34 pm, Daz DeBoer <darrell.deb...@gradleware.com> wrote:
> On Mon, Jun 30, 2014 at 7:24 PM, Daniel Lacasse <daniel.lacass...@gmail.com> > wrote: > I did some research on how to implement option #1 and here is the > implementation: > In org.gradle.nativebinaries.language.c.tasks.AbstractNativeCompilerTask: > Add option int maximumNumberOfCompilerFork = 1 > 1 is to prevent any side effect. It will mainly behave just like it was > previously. > I'm open to suggestion on the chosen name. > No @Input on the new option > In org.gradle.nativebinaries.toolchain.internal.NativeCompileSpec: > set/getMaximumNumberOfCompilerFork > In org.gradle.nativebinaries.language.internal.AbstractNativeCompilerSpec: > set/getMaximumNumberOfCompilerFork implementation > In > org.gradle.nativebinaries.toolchain.internal.{gcc|msvcpp}.NativeCompilerSpec: > Add modification to method WorkResult execute(T spec) > Use a ThreadPoolExecutor with maximumPoolSize set to > maximumNumberOfCompileFork and corePoolSize set to 1 > Submit anonymous Callable<WorkResult> class to the ThreadPoolExecutor > Use the get method on each Future returned by Submit to join all the work > together. > This cover pretty much the option #1 for parallel compilation in the native > extension. The 2 open issues would be: > Is maximumNumberOfCompilerFork a good choice for the option name? If not, > what would be the appropriate name? > Let's avoid adding a configuration option for this at this stage. What we > really want is to make this configurable at a much higher level, and > auto-detect the optimal setting. > > For now, let's use the value of 'parallel-threads' for this setting. Even > though this is a bit coarse grained, it takes us in the right direction. So > instead of passing the value from Task->CompileSpec->NativeCompiler, we would > access the value from StartParameter#getParallelThreadCount. > > I think the easiest way to do this would be to add a method to > ExecActionFactory that provided an AsyncExecAction, which would be similar to > ExecAction, but something like: > > public interface AsyncExecAction extends ExecSpec { > Future<ExecResult> submit(); > } > > You could then modify > org.gradle.nativebinaries.toolchain.internal.CommandLineTool to use async > actions, and to present some sort of submit/collect API. This would mean that > the parallel functionality would be available everywhere that CommandLineTool > is used, which means both VisualCpp and Gcc based tool chains. > > How does this sound? While it may seem a bit more involved, the design will > be largely the same. You'll still need a ThreadPoolExecuter and code to > submit/collect, but this design takes your suggested solution and moves most > of the code out of the NativeCompiler implementations into the > CommandLineTool and ExecActionFactory. The nice thing about this is that it > makes this feature more generally available, and makes is simpler for us to > have a global setting for parallelisation. (The funny thing is that this is > looking more like Option #2!) > > So native compilation would happen in parallel whenever --parallel and/or > --parallel-threads is used. As Adam suggested, we should really separate > these 2 options so that --parallel is a separate option to enable parallel > project execution, while parallel-threads is a general purpose setting that > defaults to a reasonable value. (Currently > StartParameter#getParallelThreadCount defaults to zero if neither --parallel > or --parallel-threads is specified). > How can we test this new feature? The only way I can think of right now is to > compile 2 file with maximumNumberOfCompileFork set to 1 and a second time > with the option set to 2 and compare the execution time. This heavily depend > on timing which will probably be non deterministic. > No, timing-sensitive tests are not the way to go. We could certainly unit > test the AsyncExecAction stuff, and also integration test with a few 'sleep 1 > second' actions and ensure that they don't take much more than 1 second in > total. ConcurrentSpec has stuff to write these kinds of unit tests without sleeping. > I guess we could integration test parallel native compilation by using a > 'sleep exe' in place of the actual compiler... I'll think about this a little > more. At the integration test level, we should just make sure that we produce a working binary from multiple source files. i.e update the test apps to include multiple source files, make parallel compilation the default, and make sure the tests keep working. We should prove that parallel compilation has an effect using a performance test, rather than using an integration test for this. -- Adam Murdoch Gradle Co-founder http://www.gradle.org CTO Gradleware Inc. - Gradle Training, Support, Consulting http://www.gradleware.com