Date: Mon, 21 May 2007 15:28:16 -0700 From: Jeremy Lea <reg@FreeBSD.ORG> To: Alexander Leidinger <Alexander@Leidinger.net> Cc: ports@FreeBSD.ORG Subject: Re: Speedup for make clean-depends (and thus make clean) Message-ID: <20070521222816.GA93550@flint.openpave.org> In-Reply-To: <20070521102026.d5y5ckqvk8cwcsg0@webmail.leidinger.net> References: <20070520090149.190a919c@deskjail> <20070521021313.GA63269@flint.openpave.org> <20070521102026.d5y5ckqvk8cwcsg0@webmail.leidinger.net>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi,
On Mon, May 21, 2007 at 10:20:26AM +0200, Alexander Leidinger wrote:
> I tried to do the WRKDIR and _DEPEND_DIRS part in one go myself, but
> it was slower than the patch I did post (in the case where all dirs
> are already clean). But I did use another implementation, I did a "set
> -- $$children" and used shift instead of state variables. Did you
> compare the speed of your patch with the speed of my patch (in a
> directory with a lot of dependencies like e.g. gnome2)?
In /usr/ports/x11/gnome2 (on an up to date port's tree):
make clean-depends:
real 1030.98 user 916.21 sys 102.80
make all-depends-list:
real 348.25 user 310.27 sys 32.01
make clean-depends-list: (Your patch)
real 685.53 user 611.60 sys 65.93
make clean-depends-full:
real 346.18 user 310.53 sys 31.94
make clean-depends-quick:
real 124.72 user 119.88 sys 3.73
In other words, it takes my poor old machine 17 minutes to do a 'make
clean' of gnome2 with no existing work directories (i.e. to do nothing).
Of that 5.8 minutes is spent building the list of directories to clean.
Your patch increases that to 11.4 minutes (meaning that 'make clean' is
33% faster - which matches the numbers you posted). With my patch it
takes the same time to build the list (meaning 'make clean' is 66%
faster).
The cheating quick version takes one tenth of the time, because gnome2
directly depends on 57 ports, out of 508 total (on my machine).
> There's a bug. You continue if the WRKDIR does not exist, but you
> don't change the state, so you don't check if the first dependency was
> already checked. You need to switch to the second state before
> checking the existence of the WRKDIR.
Ooops, that should, of course, have been a 'break' not a 'continue'...
> >One might want to use the logic that 'make clean' does a 'make clean
> >limited-clean' if NOCLEANDEPENDS is not defined, and 'make clean' if it
> >is, and leave 'make clean-depends' to do the full clean.
>
> I don't think changing the semantic of some existing stuff without a
> major reason is a good idea. I think it is better to introduce a new
> target and let people use the new target instead.
>From bsd.port.mk:
# clean - Remove ${WRKDIR} and other temporary files used for building.
# clean-depends - Do a "make clean" for all dependencies.
This is the only documentation of the targets that I found. Using the
limited-clean target would get users exactly the this behaviour - it
would clean up the port and all of the other ports it depended on that
were built to statisfy the building of this port.
The only difference would be that if you say built some stuff (say
firefox), doing a clean, removed some stuff (say gmake - which is now a
leaf port/package), then built some other stuff (like mysql) without a
clean, then built something else (like gnome2) with a clean, it would
still leave behind some dirty dirs like gmake, which would have been
cleaned before.
You could come up with a number of contrived examples where this would
shoot you in the foot -- mostly when you install something then
pkg_delete it, without cleaning, and then build something else which
depends on it, and because it's not clean the sub-make will return
without doing an install because the install cookie exists, but the
build will fail right there. (This case has been broken in the ports
tree for a long time - changing DEPENDS_TARGET to 'reinstall' fixes most
cases). 'make clean' will still get you back on track because you will
have a trail of partially built ports down to this old build.
I've added some more to the attched patch, including the idea of a
pre-clean, which would be useful for portupgrade, since it could define
DEPENDS_PRECLEAN and not have to worry about cleaning before building...
Regards,
-Jeremy
--
FreeBSD - Because the best things in life are free...
http://www.freebsd.org/
--- bsd.port.mk.orig Sat May 19 12:57:27 2007
+++ bsd.port.mk Mon May 21 15:11:53 2007
@@ -3216,10 +3216,14 @@
.endif
.if !defined(DEPENDS_TARGET)
+.if defined(DEPENDS_PRECLEAN)
+DEPENDS_TARGET= clean
+DEPENDS_ARGS= NOCLEANDEPENDS=yes
+.endif
.if make(reinstall)
-DEPENDS_TARGET= reinstall
+DEPENDS_TARGET+= reinstall
.else
-DEPENDS_TARGET= install
+DEPENDS_TARGET+= install
.endif
.if defined(DEPENDS_CLEAN)
DEPENDS_TARGET+= clean
@@ -4453,7 +4457,7 @@
.if !target(clean)
clean:
.if !defined(NOCLEANDEPENDS)
- @cd ${.CURDIR} && ${MAKE} ${__softMAKEFLAGS} clean-depends
+ @cd ${.CURDIR} && ${MAKE} ${__softMAKEFLAGS} limited-clean-depends
.endif
@${ECHO_MSG} "===> Cleaning for ${PKGNAME}"
.if target(pre-clean)
@@ -4846,6 +4850,14 @@
if ${EXPR} "$$dir" : '.*:' > /dev/null; then \
target=`${ECHO_CMD} $$dir | ${SED} -e 's/.*://'`; \
dir=`${ECHO_CMD} $$dir | ${SED} -e 's/:.*//'`; \
+ if [ X${DEPENDS_PRECLEAN} != "X" ]; then \
+ target="clean $$target"; \
+ depends_args="$$depends_args NOCLEANDEPENDS=yes"; \
+ fi; \
+ if [ X${DEPENDS_CLEAN} != "X" ]; then \
+ target="$$target clean"; \
+ depends_args="$$depends_args NOCLEANDEPENDS=yes"; \
+ fi; \
else \
target="${DEPENDS_TARGET}"; \
depends_args="${DEPENDS_ARGS}"; \
@@ -5050,9 +5062,95 @@
L=$$l; \
done
+CLEAN-DEPENDS-FULL= \
+ L="${_DEPEND_DIRS}"; \
+ checked=""; \
+ while [ -n "$$L" ]; do \
+ l=""; \
+ for d in $$L; do \
+ case $$checked in \
+ $$d\ *|*\ $$d\ *|*\ $$d) \
+ continue;; \
+ esac; \
+ checked="$$checked $$d"; \
+ if [ ! -d $$d ]; then \
+ ${ECHO_MSG} "${PKGNAME}: \"$$d\" non-existent -- dependency list incomplete" >&2; \
+ continue; \
+ fi; \
+ if ! children=$$(cd $$d && ${MAKE} -V WRKDIR -V _DEPEND_DIRS); then \
+ ${ECHO_MSG} "${PKGNAME}: \"$$d\" erroneous -- dependency list incomplete" >&2; \
+ continue; \
+ fi; \
+ state=0; \
+ for child in $$children; do \
+ case $$state in \
+ 0) \
+ if [ -d $child ]; then \
+ ${ECHO_CMD} $$d; \
+ fi; \
+ state=1;; \
+ 1) \
+ case "$$checked $$l" in \
+ $$child\ *|*\ $$child\ *|*\ $$child) \
+ continue;; \
+ esac; \
+ l="$$l $$child";; \
+ esac; \
+ done; \
+ done; \
+ L=$$l; \
+ done
+
+CLEAN-DEPENDS-LIMITED= \
+ L="${_DEPEND_DIRS}"; \
+ checked=""; \
+ while [ -n "$$L" ]; do \
+ l=""; \
+ for d in $$L; do \
+ case $$checked in \
+ $$d\ *|*\ $$d\ *|*\ $$d) \
+ continue;; \
+ esac; \
+ checked="$$checked $$d"; \
+ if [ ! -d $$d ]; then \
+ ${ECHO_MSG} "${PKGNAME}: \"$$d\" non-existent -- dependency list incomplete" >&2; \
+ continue; \
+ fi; \
+ if ! children=$$(cd $$d && ${MAKE} -V WRKDIR -V _DEPEND_DIRS); then \
+ ${ECHO_MSG} "${PKGNAME}: \"$$d\" erroneous -- dependency list incomplete" >&2; \
+ continue; \
+ fi; \
+ state=0; \
+ for child in $$children; do \
+ case $$state in \
+ 0) \
+ if [ ! -d $child ]; then \
+ break; \
+ fi; \
+ state=1; \
+ ${ECHO_CMD} $$d;; \
+ 1) \
+ case "$$checked $$l" in \
+ $$child\ *|*\ $$child\ *|*\ $$child) \
+ continue;; \
+ esac; \
+ l="$$l $$child";; \
+ esac; \
+ done; \
+ done; \
+ L=$$l; \
+ done
+
.if !target(clean-depends)
clean-depends:
- @for dir in $$(${ALL-DEPENDS-LIST}); do \
+ @for dir in $$(${CLEAN-DEPENDS-FULL}); do \
+ (cd $$dir; ${MAKE} NOCLEANDEPENDS=yes clean); \
+ done
+.endif
+
+.if !target(limited-clean-depends)
+limited-clean-depends:
+ @for dir in $$(${CLEAN-DEPENDS-LIMITED}); do \
(cd $$dir; ${MAKE} NOCLEANDEPENDS=yes clean); \
done
.endif
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070521222816.GA93550>