[cmake-developers] Ninja pools

[Brad King]

On 11/05/2013 02:15 PM, Peter Kümmel wrote:
> I merged a proposal to next which adds support for Ninja's "pool":
> 
>    http://martine.github.io/ninja/manual.html#ref_pool

This will be a really nice feature to support.

> It adds three new properties, POOLS, LINK_POOL, COMPILE_POOL:
> 
>    http://www.cmake.org/cmake/help/git-next/manual/cmake-properties.7.html
> 
> With a "pool" it is possible to limit the number of concurrent
> processes of a specific rule.

Without context the name "POOL" may not have clear meaning to
a reader not already familiar with Ninja's feature.  I think
a name like JOB_POOL would be clearer.

The documentation of these properties all note that they are
only for Ninja.  While it is possible a future generator will
also support them, it is unclear whether the semantics will be
exactly the same.  Therefore I prefer to use a NINJA_ prefix
on the property names for now:

 NINJA_JOB_POOLS
 NINJA_JOB_POOL_LINK
 NINJA_JOB_POOL_COMPILE

Also look at cmTarget::SetMakefile for calls to SetPropertyDefault
to see how to add variables like

 CMAKE_NINJA_JOB_POOL_LINK
 CMAKE_NINJA_JOB_POOL_COMPILE

that set a default for the target properties at their creation.
This will make it easy for projects to put all targets into the
pools.  We should also consider a way to allow users to define
pools with cache entries when the projects don't.

> Current patch adds only the essentials, but maybe there are more
> comfortable ways to use pools.

The patch also appears to try adding a POOL option to the
add_custom_command command but does not provide documentation
except in the commit message.  In the final version of this
topic that goes to master, I'd prefer not to have any docs or
examples in the commit messages, just in the actual docs.
(Again, I think the option should be named NINJA_JOB_POOL.)

Should we have a

 NINJA_JOB_POOL_CUSTOM

property that is used for custom commands within a target
that do not explicitly set a job pool?  It would of course
come with a supporting CMAKE_NINJA_JOB_POOL_CUSTOM variable
to initialize the property.  This would allow projects to
quickly add all their build rules to pools with just a few
lines of code.

Also please add error messages when the NINJA_JOB_POOLS
value does not match the expected format, or when one of
the other rules names a pool not on the global list.  The
add_custom_command option processing:

+         case doing_pool:
+           pool = copy;
+           break;

should switch back to doing = doing_nothing so that a second
value after the keyword is rejected with an error message.
(Some of the other keyword options should be doing this but
can't now for compatibility.)

The interface should be as strict as possible when first created.
It can always be relaxed in the future, but would require a
policy to make it more strict.  Please add RunCMake test cases
that cover these error messages.

Thanks,
-Brad