Date: Mon, 21 Apr 2014 07:35:03 -0600 From: Warner Losh <imp@bsdimp.com> To: Ian Lepore <ian@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Dimitry Andric <dim@FreeBSD.org>, Jilles Tjoelker <jilles@stack.nl> Subject: Re: svn commit: r263778 - in head: bin lib lib/clang sbin share/mk usr.bin usr.sbin Message-ID: <A3C5D651-D1A4-46AF-900A-6D019AD2D39C@bsdimp.com> In-Reply-To: <1398086131.1124.371.camel@revolution.hippie.lan> References: <201403262230.s2QMUdH6021943@svn.freebsd.org> <AA90F6B0-3A7A-473D-82C2-CFDFD263E9AC@gmail.com> <20140327181245.GA69977@stack.nl> <7A86F5E9-DBE9-4D3F-B166-C02F8386B722@FreeBSD.org> <1398086131.1124.371.camel@revolution.hippie.lan>
next in thread | previous in thread | raw e-mail | index | archive | help
(sorry for the top post) This looks good to my eye. I=A2d be tempted to toss in a comment about = the __wait=3D.WAIT construct is due to the primitive nature of bmake=A2s = 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 <ian@FreeBSD.org> wrote: > On Thu, 2014-03-27 at 20:44 +0100, Dimitry Andric wrote: >> On 27 Mar 2014, at 19:12, Jilles Tjoelker <jilles@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 <dim@freebsd.org> = wrote: >>>>> Author: dim >>>>> Date: Wed Mar 26 22:30:38 2014 >>>>> New Revision: 263778 >>>>> URL: http://svnweb.freebsd.org/changeset/base/263778 >>>=20 >>>>> 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. >>>=20 >>>>> This can be extended to more Makefiles later on, whenever they are >>>>> verified to work correctly with parallel building. >>>=20 >>>> Why not have this =A1opt out=A2 rather than =A1opt in=A2 like it is = now? Are >>>> there any known bad dependencies this introduces? >>>=20 >>> 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. >>>=20 >>> 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. >>=20 >> 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. >>=20 >> 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. >>=20 >> -Dimitry >>=20 >=20 > 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? >=20 > -- Ian >=20 > 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: >=20 > _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} "=3D=3D=3D> = ${DIRPRFX}$${entry}.${MACHINE_ARCH} (${.TARGET:realinstall=3Dinstall})"; = \ > edir=3D$${entry}.${MACHINE_ARCH}; \ > @@ -57,7 +57,7 @@ distribute: > done > .endif >=20 > -${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 >=20 > +__wait=3D.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=3D > .for __dir in ${SUBDIR} > -${__target}: ${__target}_subdir_${__dir} > +.if ${__wait} =3D=3D ${__dir} > +__subdir_targets+=3D .WAIT > +.else > +__subdir_targets+=3D ${__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=3Dinstall} \ > DIRPRFX=3D${DIRPRFX}$$edir/ > +.endif > .endfor > +${__target}: ${__subdir_targets} > .else > ${__target}: _SUBDIR > .endif
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?A3C5D651-D1A4-46AF-900A-6D019AD2D39C>