(sorry for the top post) This looks good to my eye. I’d be tempted to toss in a comment about the __wait=.WAIT construct is due to the primitive nature of bmake’s parser so it runs afoul of the .for/.if construction rules and this is needed to expand the for variable. It tripped me up when I looked at it, until I recalled a comment from similar code in NetBSD.
Warner On Apr 21, 2014, at 7:15 AM, Ian Lepore <i...@freebsd.org> wrote: > On Thu, 2014-03-27 at 20:44 +0100, Dimitry Andric wrote: >> On 27 Mar 2014, at 19:12, Jilles Tjoelker <jil...@stack.nl> wrote: >>> On Thu, Mar 27, 2014 at 11:05:00AM -0600, Warner Losh wrote: >>>> On Mar 26, 2014, at 4:30 PM, Dimitry Andric <d...@freebsd.org> wrote: >>>>> Author: dim >>>>> Date: Wed Mar 26 22:30:38 2014 >>>>> New Revision: 263778 >>>>> URL: http://svnweb.freebsd.org/changeset/base/263778 >>> >>>>> Log: >>>>> Add a SUBDIR_PARALLEL option to bsd.subdir.mk, to allow make to process >>>>> all the SUBDIR entries in parallel, instead of serially. Apply this >>>>> option to a selected number of Makefiles, which can greatly speed up the >>>>> build on multi-core machines, when using make -j. >>> >>>>> This can be extended to more Makefiles later on, whenever they are >>>>> verified to work correctly with parallel building. >>> >>>> Why not have this ‘opt out’ rather than ‘opt in’ like it is now? Are >>>> there any known bad dependencies this introduces? >>> >>> I'm paranoid about build systems ;) It is easy to add dependencies >>> across directories and as long as directories are built in sequence, >>> nothing goes wrong. >>> >>> In fact, I had enabled SUBDIR_PARALLEL in sys/modules/Makefile as well, >>> but this caused mysterious failures with some kernels such as mips >>> ADM5120. >> >> There are a bunch of other parts that don't really like parallel builds >> at the moment. For example, gnu/usr.bin/binutils needs its libraries >> (libbfd.a, etc) built first, before it can link the programs. Similar >> for gnu/usr.bin/cc, which needs libiberty, libcpp, etc before being able >> to build the rest of gcc. >> >> Most of these cases can hopefully be solved by adding .WAIT targets at >> strategic points in the SUBDIR lists, but this also needs a bit of extra >> logic in bsd.subdir.mk. >> >> -Dimitry >> > > It turns out I needed the .WAIT functionality to use SUBDIR_PARALLEL for > $work, so I came up with the attached, does this look okay to commit? > > -- Ian > > diff -r 67802e319fc6 share/mk/bsd.subdir.mk > --- a/share/mk/bsd.subdir.mk Sun Apr 20 21:01:07 2014 -0600 > +++ b/share/mk/bsd.subdir.mk Mon Apr 21 06:59:37 2014 -0600 > @@ -4,10 +4,10 @@ > # The include file <bsd.subdir.mk> contains the default targets > # for building subdirectories. > # > -# For all of the directories listed in the variable SUBDIRS, the > +# For all of the directories listed in the variable SUBDIR, the > # specified directory will be visited and the target made. There is > # also a default target which allows the command "make subdir" where > -# subdir is any directory listed in the variable SUBDIRS. > +# subdir is any directory listed in the variable SUBDIR. > # > # > # +++ variables +++ > @@ -42,7 +42,7 @@ distribute: > > _SUBDIR: .USE > .if defined(SUBDIR) && !empty(SUBDIR) && !defined(NO_SUBDIR) > - @${_+_}for entry in ${SUBDIR}; do \ > + @${_+_}for entry in ${SUBDIR:N.WAIT}; do \ > if test -d ${.CURDIR}/$${entry}.${MACHINE_ARCH}; then \ > ${ECHODIR} "===> ${DIRPRFX}$${entry}.${MACHINE_ARCH} > (${.TARGET:realinstall=install})"; \ > edir=$${entry}.${MACHINE_ARCH}; \ > @@ -57,7 +57,7 @@ distribute: > done > .endif > > -${SUBDIR}: .PHONY > +${SUBDIR:N.WAIT}: .PHONY > ${_+_}@if test -d ${.TARGET}.${MACHINE_ARCH}; then \ > cd ${.CURDIR}/${.TARGET}.${MACHINE_ARCH}; \ > else \ > @@ -65,13 +65,18 @@ distribute: > fi; \ > ${MAKE} all > > +__wait=.WAIT > .for __target in all all-man checkdpadd clean cleandepend cleandir \ > depend distribute lint maninstall manlint \ > obj objlink realinstall regress tags \ > ${SUBDIR_TARGETS} > .ifdef SUBDIR_PARALLEL > +__subdir_targets= > .for __dir in ${SUBDIR} > -${__target}: ${__target}_subdir_${__dir} > +.if ${__wait} == ${__dir} > +__subdir_targets+= .WAIT > +.else > +__subdir_targets+= ${__target}_subdir_${__dir} > ${__target}_subdir_${__dir}: .MAKE > @${_+_}set -e; \ > if test -d ${.CURDIR}/${__dir}.${MACHINE_ARCH}; then \ > @@ -85,7 +90,9 @@ distribute: > fi; \ > ${MAKE} ${__target:realinstall=install} \ > DIRPRFX=${DIRPRFX}$$edir/ > +.endif > .endfor > +${__target}: ${__subdir_targets} > .else > ${__target}: _SUBDIR > .endif _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
