Skip site navigation (1)Skip section navigation (2)
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>