diff mbox

RFC: PATCH to avoid linking multiple front ends at once with parallel make

Message ID 518077C0.4050703@redhat.com
State New
Headers show

Commit Message

Jason Merrill May 1, 2013, 2:02 a.m. UTC 
Since GNU Make doesn't support anything like the .MUTEX directive 
(http://savannah.gnu.org/bugs/?func=detailitem&item_id=17873), and 
accidentally doing "make -j8 -l4" makes my laptop useless for several 
minutes while it tries to link all the front ends at once, I decided to 
kludge a workaround.

This hack uses mkdir as a locking mechanism, as it fails if the 
directory already exists.  Each front-end rule first tries to get the 
lock, and spins if the lock isn't available.  Currently I'm enabling the 
locking by default on build hosts with less than 8GB of memory.

Releasing the lock is not reliable; if the user interrupts the link with 
^C, the lock will remain.  So I adjusted 'make all' to remove the lock 
early on, though that only works for the typical case, and users that do 
something like 'make cc1plus' could still run into trouble.

Thoughts?  Is this too horrible a hack, or does it seem like something 
we might want?

Maybe I should fix Make instead.

Jason

Comments

Mike Stump May 1, 2013, 11:40 p.m. UTC | #1 
On Apr 30, 2013, at 7:02 PM, Jason Merrill <jason@redhat.com> wrote:
> Releasing the lock is not reliable; if the user interrupts the link with ^C, the lock will remain.  So I adjusted 'make all' to remove the lock early on, though that only works for the typical case, and users that do something like 'make cc1plus' could still run into trouble.

I don't like the unreliable nature of the lock…  I've been burned so many times over the years from bad locks, I just want to run away screaming, that said.  It might be ok to leave it unreliable.

> Maybe I should fix Make instead.

Well, if you can wait 3 years for a fix.  :-)  Until then, having something in the gcc build system seems like the right approach.

+      mem="$(free|sed -n 2p|awk '{print $2}')" 2>/dev/null
+      if test "$mem" -lt 8000000 2>/dev/null; then
+        do_link_mutex=yes
+      else
+        do_link_mutex=no
+      fi)

when the system doesn't have free, this fails.  I think:

+      mem="$(free|sed -n 2p|awk '{print $2}')" 2>/dev/null
+      if test "0$mem" -lt 8000000 2>/dev/null; then
+        do_link_mutex=yes
+      else
+        do_link_mutex=no
+      fi)

should fix it.  Notice the 0$mem.  Also, when there is no free on the system, this fails with the mutex on.  I think I might prefer it to fail off, though, not a big issue.

if test "0$mem" -lt 8000000 -a  "0$mem" -ne 0 2>/dev/null; then

I think would flip the default for treeless systems.

and last, 


> cc1$(exeext): $(C_OBJS) cc1-checksum.o $(BACKEND) $(LIBDEPS)
> +	$(MAKE) link-mutex
> 	+$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ $(C_OBJS) \
> -	  cc1-checksum.o $(BACKEND) $(LIBS) $(BACKENDLIBS)
> +	  cc1-checksum.o $(BACKEND) $(LIBS) $(BACKENDLIBS); \
> +	  $(MAKE) remove-link-mutex
> #
> # Build hooks:

I think this blows error returns from LINKER?!  That is deadly.  Not cleaning up the lock when the link fails would be down right anti-social, given we already wedge in typical development style builds.  The solution is done like this:

$ { true && echo remove lock; } || { echo remove lock && false; }; echo $?
remove lock
0
$ { false && echo remove lock; } || { echo remove lock && false; }; echo $?
remove lock
1

That's old-skool.  The new fangled approach, if you can tolerate new tricks would be:

$ bash -c "trap 'echo remove lock' 0; true"; echo $?
remove lock
0
$ bash -c "trap 'echo remove lock' 0; false"; echo $?
remove lock
1

The advantage of this is, even in exceptional cases (but only trivial exceptional cases like ^C), the lock is removed, though, blunting further the fact the lock is poor.

I can't help, but wonder, if having a LLINKER like this:

LLINKER := 'linkit () { $(MAKE) link-mutex && { { $(LINKER) "$$@" && $(MAKE) remove-link-mutex; } || { $(MAKE) remove-link-mutex && false; } } } && linkit"

(pseudo code, one has to ensure quoting and meta characters are right)

Then you can use LLINKER or LINKER depending upon if you want to serialize the link.  This is then easy to maintain, as there is only one definition of LLINKER.  If a mistake was made in the semantic for it, it only would need fixing in the one place, not 8.  Ensuring the quoting is right, is the only hard part.

If you can tolerate the new school solution, this goes a long way to lessening the impact of the bad lock.
Dave Korn May 5, 2013, 1:57 p.m. UTC | #2 
On 01/05/2013 03:02, Jason Merrill wrote:
> Since GNU Make doesn't support anything like the .MUTEX directive
> (http://savannah.gnu.org/bugs/?func=detailitem&item_id=17873), and
> accidentally doing "make -j8 -l4" makes my laptop useless for several
> minutes while it tries to link all the front ends at once, I decided to
> kludge a workaround.
> 
> This hack uses mkdir as a locking mechanism, as it fails if the
> directory already exists.  Each front-end rule first tries to get the
> lock, and spins if the lock isn't available.  Currently I'm enabling the
> locking by default on build hosts with less than 8GB of memory.
> 
> Releasing the lock is not reliable; if the user interrupts the link with
> ^C, the lock will remain.  So I adjusted 'make all' to remove the lock
> early on, though that only works for the typical case, and users that do
> something like 'make cc1plus' could still run into trouble.
> 
> Thoughts?  Is this too horrible a hack, or does it seem like something
> we might want?
> 
> Maybe I should fix Make instead.

  This sounds like a bad idea to me, and not just because the locking
mechanism is dodgy.  Is the problem more widespread than just your laptop?
Does it affect other host OSs?  Linking multiple frontends at once doesn't
lock up my desktop PC, so I'd rather not have it disabled.

  Why don't you just nice your build shell?  Shouldn't that make the rest of
your system responsive?

    cheers,
      DaveK
Jason Merrill May 6, 2013, 4:41 p.m. UTC | #3 
On 05/05/2013 09:57 AM, Dave Korn wrote:
>    This sounds like a bad idea to me, and not just because the locking
> mechanism is dodgy.  Is the problem more widespread than just your laptop?
> Does it affect other host OSs?  Linking multiple frontends at once doesn't
> lock up my desktop PC, so I'd rather not have it disabled.

Part of the issue is that I build with -g3, which greatly increases the 
memory consumption at link time.  And builds are sharing 4GB with 
everything else I want to run.

I would be happy to have the mechanism off by default and manually 
enabled by people in my situation.

>    Why don't you just nice your build shell?  Shouldn't that make the rest of
> your system responsive?

I don't think that helps much with swap thrashing, though I could be wrong.

Jason
Dave Korn May 7, 2013, 12:41 a.m. UTC | #4 
On 06/05/2013 17:41, Jason Merrill wrote:
> On 05/05/2013 09:57 AM, Dave Korn wrote:
>> This sounds like a bad idea to me, and not just because the locking 
>> mechanism is dodgy. Is the problem more widespread than just your laptop?
>>  Does it affect other host OSs? Linking multiple frontends at once 
>> doesn't lock up my desktop PC, so I'd rather not have it disabled.
> 
> Part of the issue is that I build with -g3, which greatly increases the
> memory consumption at link time.  And builds are sharing 4GB with
> everything else I want to run.
> 
> I would be happy to have the mechanism off by default and manually
> enabled by people in my situation.

  Well I couldn't possibly object to that :)

>>    Why don't you just nice your build shell?  Shouldn't that make the
>> rest of your system responsive?
> 
> I don't think that helps much with swap thrashing, though I could be wrong.

  Ah, no, you're probably right there.  So serialising those final links
sounds like a reasonable solution.

    cheers,
      DaveK
diff mbox

Patch

commit 47980ab5ea7f1037b8b7cce9b8abfe1c0a75c032
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Apr 25 17:54:17 2013 -0400

    	* Makefile.in (link-mutex, remove-link-mutex): New targets.
    	(native, mostlyclean): Remove link mutex.
    	* configure.ac: Handle --enable-link-mutex.
    	* */Make-lang.in: Use link mutex.

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 903125e..3934e16 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1737,8 +1737,8 @@  start.encap: native xgcc$(exeext) cpp$(exeext) specs \
 rest.encap: lang.rest.encap
 # This is what is made with the host's compiler
 # whether making a cross compiler or not.
-native: config.status auto-host.h build-@POSUB@ $(LANGUAGES) \
-	$(EXTRA_PROGRAMS) $(COLLECT2) lto-wrapper$(exeext) \
+native: remove-link-mutex config.status auto-host.h build-@POSUB@ \
+	$(LANGUAGES) $(EXTRA_PROGRAMS) $(COLLECT2) lto-wrapper$(exeext) \
 	gcc-ar$(exeext) gcc-nm$(exeext) gcc-ranlib$(exeext)
 
 ifeq ($(enable_plugin),yes)
@@ -4529,6 +4529,8 @@  mostlyclean: lang.mostlyclean
 	-rm -f gtype.state
 # Delete genchecksum outputs
 	-rm -f *-checksum.c
+# Delete front-end linking mutex directory
+	-rm -rf link-mutex
 
 # Delete all files made by compilation
 # that don't exist in the distribution.
@@ -5335,3 +5337,14 @@  po/gcc.pot: force
 	$(MAKE) srcextra
 	AWK=$(AWK) $(SHELL) $(srcdir)/po/exgettext \
 		$(XGETTEXT) gcc $(srcdir)
+
+do_link_mutex = @DO_LINK_MUTEX@
+.PHONY: link-mutex
+link-mutex:
+ifeq ($(do_link_mutex),true)
+	while ! mkdir link-mutex 2>/dev/null; do sleep 1; done
+endif
+
+.PHONY: remove-link-mutex
+remove-link-mutex:
+	-rm -rf link-mutex
diff --git a/gcc/ada/gcc-interface/Make-lang.in b/gcc/ada/gcc-interface/Make-lang.in
index ef12b4b..7d88961 100644
--- a/gcc/ada/gcc-interface/Make-lang.in
+++ b/gcc/ada/gcc-interface/Make-lang.in
@@ -562,7 +562,10 @@  TARGET_ADA_SRCS =
 # Since the RTL should be built with the latest compiler, remove the
 #  stamp target in the parent directory whenever gnat1 is rebuilt
 gnat1$(exeext): $(TARGET_ADA_SRCS) $(GNAT1_OBJS) $(ADA_BACKEND) libcommon-target.a $(LIBDEPS)
-	+$(GCC_LINK) -o $@ $(GNAT1_OBJS) $(ADA_BACKEND) libcommon-target.a $(LIBS) $(SYSLIBS) $(BACKENDLIBS) $(CFLAGS)
+	$(MAKE) link-mutex
+	+$(GCC_LINK) -o $@ $(GNAT1_OBJS) $(ADA_BACKEND) \
+	  libcommon-target.a $(LIBS) $(SYSLIBS) $(BACKENDLIBS) $(CFLAGS); \
+	  $(MAKE) remove-link-mutex
 	$(RM) stamp-gnatlib2-rts stamp-tools
 
 gnatbind$(exeext): ada/b_gnatb.o $(CONFIG_H) $(GNATBIND_OBJS) ggc-none.o libcommon-target.a $(LIBDEPS)
diff --git a/gcc/c/Make-lang.in b/gcc/c/Make-lang.in
index 8310e0a..cf61d74 100644
--- a/gcc/c/Make-lang.in
+++ b/gcc/c/Make-lang.in
@@ -75,8 +75,10 @@  cc1-checksum.c : build/genchecksum$(build_exeext) checksum-options \
 cc1-checksum.o : cc1-checksum.c $(CONFIG_H) $(SYSTEM_H)
 
 cc1$(exeext): $(C_OBJS) cc1-checksum.o $(BACKEND) $(LIBDEPS)
+	$(MAKE) link-mutex
 	+$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ $(C_OBJS) \
-	  cc1-checksum.o $(BACKEND) $(LIBS) $(BACKENDLIBS)
+	  cc1-checksum.o $(BACKEND) $(LIBS) $(BACKENDLIBS); \
+	  $(MAKE) remove-link-mutex
 #
 # Build hooks:
 
diff --git a/gcc/configure b/gcc/configure
index fd4a0eb..ab56d21 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -670,6 +670,7 @@  subdirs
 dollar
 gcc_tooldir
 enable_lto
+DO_LINK_MUTEX
 MAINT
 zlibinc
 zlibdir
@@ -916,6 +917,7 @@  with_long_double_128
 with_gc
 with_system_zlib
 enable_maintainer_mode
+enable_link_mutex
 enable_version_specific_runtime_libs
 enable_plugin
 enable_libquadmath_support
@@ -1627,6 +1629,8 @@  Optional Features:
   --enable-maintainer-mode
                           enable make rules and dependencies not useful (and
                           sometimes confusing) to the casual installer
+  --enable-link-mutex     avoid linking multiple front-ends at once to avoid
+                          thrashing on the build machine
   --enable-version-specific-runtime-libs
                           specify that runtime libraries should be installed
                           in a compiler-specific directory
@@ -17830,7 +17834,7 @@  else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 17833 "configure"
+#line 17837 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -17936,7 +17940,7 @@  else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 17939 "configure"
+#line 17943 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -27041,6 +27045,32 @@  else
   MAINT='#'
 fi
 
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to avoid linking multiple front-ends at once" >&5
+$as_echo_n "checking whether to avoid linking multiple front-ends at once... " >&6; }
+  # Check whether --enable-link-mutex was given.
+if test "${enable_link_mutex+set}" = set; then :
+  enableval=$enable_link_mutex; do_link_mutex=$enableval
+else
+  # By default avoid simultaneous links on machines with less than 8GB.
+      mem="$(free|sed -n 2p|awk '{print $2}')" 2>/dev/null
+      if test "$mem" -lt 8000000 2>/dev/null; then
+        do_link_mutex=yes
+      else
+        do_link_mutex=no
+      fi
+fi
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $do_link_mutex" >&5
+$as_echo "$do_link_mutex" >&6; }
+
+if test "$do_link_mutex" = "yes"; then
+   DO_LINK_MUTEX=true
+else
+   DO_LINK_MUTEX=false
+fi
+
+
 # --------------
 # Language hooks
 # --------------
diff --git a/gcc/configure.ac b/gcc/configure.ac
index a859d99..71f8e31 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -4938,6 +4938,30 @@  else
 fi
 AC_SUBST(MAINT)dnl
 
+dnl Whether to prevent multiple front-ends from linking at the same time
+
+AC_MSG_CHECKING([whether to avoid linking multiple front-ends at once])
+  AC_ARG_ENABLE(link-mutex,
+[AS_HELP_STRING([--enable-link-mutex],
+		[avoid linking multiple front-ends at once to avoid thrashing
+		 on the build machine])],
+      do_link_mutex=$enableval,
+      # By default avoid simultaneous links on machines with less than 8GB.
+      mem="$(free|sed -n 2p|awk '{print $2}')" 2>/dev/null
+      if test "$mem" -lt 8000000 2>/dev/null; then
+        do_link_mutex=yes
+      else
+        do_link_mutex=no
+      fi)
+AC_MSG_RESULT($do_link_mutex)
+
+if test "$do_link_mutex" = "yes"; then
+   DO_LINK_MUTEX=true
+else
+   DO_LINK_MUTEX=false
+fi
+AC_SUBST(DO_LINK_MUTEX)
+
 # --------------
 # Language hooks
 # --------------
diff --git a/gcc/cp/Make-lang.in b/gcc/cp/Make-lang.in
index cda4897..760d801 100644
--- a/gcc/cp/Make-lang.in
+++ b/gcc/cp/Make-lang.in
@@ -100,8 +100,10 @@  cc1plus-checksum.c : build/genchecksum$(build_exeext) checksum-options \
 cc1plus-checksum.o : cc1plus-checksum.c $(CONFIG_H) $(SYSTEM_H)
 
 cc1plus$(exeext): $(CXX_OBJS) cc1plus-checksum.o $(BACKEND) $(LIBDEPS)
+	$(MAKE) link-mutex
 	+$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
-	      $(CXX_OBJS) cc1plus-checksum.o $(BACKEND) $(LIBS) $(BACKENDLIBS)
+	      $(CXX_OBJS) cc1plus-checksum.o $(BACKEND) $(LIBS) $(BACKENDLIBS); \
+	      $(MAKE) remove-link-mutex
 
 ifeq ($(ENABLE_MAINTAINER_RULES), true)
 # Special build rule.  This is a maintainer rule, that is only
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 1265129..89b8260 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -1293,6 +1293,11 @@  opposite effect.  If neither option is specified, the configure script
 will try to guess whether the @code{.init_array} and
 @code{.fini_array} sections are supported and, if they are, use them.
 
+@item --enable-link-mutex
+When building GCC, use a mutex to avoid linking the compilers for
+multiple languages at the same time, to avoid thrashing.  By default,
+this is enabled if the build machine has less than 8GB of memory.
+
 @item --enable-maintainer-mode
 The build rules that regenerate the Autoconf and Automake output files as
 well as the GCC master message catalog @file{gcc.pot} are normally
diff --git a/gcc/fortran/Make-lang.in b/gcc/fortran/Make-lang.in
index c3f826c..2c8bcac 100644
--- a/gcc/fortran/Make-lang.in
+++ b/gcc/fortran/Make-lang.in
@@ -98,9 +98,10 @@  gfortran-cross$(exeext): gfortran$(exeext)
 # The compiler itself is called f951.
 f951$(exeext): $(F95_OBJS) \
 		$(BACKEND) $(LIBDEPS) attribs.o
+	$(MAKE) link-mutex
 	+$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
 		$(F95_OBJS) $(BACKEND) $(ZLIB) $(LIBS) attribs.o \
-		$(BACKENDLIBS)
+		$(BACKENDLIBS); $(MAKE) remove-link-mutex
 
 gt-fortran-trans.h    : s-gtype; @true
 #
diff --git a/gcc/go/Make-lang.in b/gcc/go/Make-lang.in
index dd98080..7059672 100644
--- a/gcc/go/Make-lang.in
+++ b/gcc/go/Make-lang.in
@@ -76,8 +76,10 @@  GO_OBJS = \
 	go/unsafe.o
 
 go1$(exeext): $(GO_OBJS) attribs.o $(BACKEND) $(LIBDEPS)
+	$(MAKE) link-mutex
 	+$(CXX) $(ALL_CXXFLAGS) $(LDFLAGS) -o $@ \
-	      $(GO_OBJS) attribs.o $(BACKEND) $(LIBS) $(BACKENDLIBS)
+	      $(GO_OBJS) attribs.o $(BACKEND) $(LIBS) $(BACKENDLIBS); \
+	      $(MAKE) remove-link-mutex
 
 # Documentation.
 
diff --git a/gcc/java/Make-lang.in b/gcc/java/Make-lang.in
index 7ed3220..c1d7f5c 100644
--- a/gcc/java/Make-lang.in
+++ b/gcc/java/Make-lang.in
@@ -99,8 +99,10 @@  jvspec.o-warn = -Wno-error
 
 jc1$(exeext): $(JAVA_OBJS) $(BACKEND) $(LIBDEPS) attribs.o
 	rm -f $@
+	$(MAKE) link-mutex
 	+$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
-		$(JAVA_OBJS) $(BACKEND) $(ZLIB) $(LIBICONV) $(LIBS) attribs.o $(BACKENDLIBS)
+		$(JAVA_OBJS) $(BACKEND) $(ZLIB) $(LIBICONV) $(LIBS) attribs.o $(BACKENDLIBS); \
+	        $(MAKE) remove-link-mutex
 
 jcf-dump$(exeext): $(JCFDUMP_OBJS) $(LIBDEPS)
 	rm -f $@
diff --git a/gcc/lto/Make-lang.in b/gcc/lto/Make-lang.in
index a0bdc26..b73b3d4 100644
--- a/gcc/lto/Make-lang.in
+++ b/gcc/lto/Make-lang.in
@@ -71,8 +71,10 @@  lto.stagefeedback:
 lto-warn = $(STRICT_WARN)
 
 $(LTO_EXE): $(LTO_OBJS) $(BACKEND) $(LIBDEPS)
+	$(MAKE) link-mutex
 	+$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
-		$(LTO_OBJS) $(BACKEND) $(BACKENDLIBS) $(LIBS)
+		$(LTO_OBJS) $(BACKEND) $(BACKENDLIBS) $(LIBS); \
+	        $(MAKE) remove-link-mutex
 
 # Dependencies
 lto/lto-lang.o: lto/lto-lang.c $(CONFIG_H) coretypes.h debug.h \
diff --git a/gcc/objc/Make-lang.in b/gcc/objc/Make-lang.in
index 9e005c1..f45d567 100644
--- a/gcc/objc/Make-lang.in
+++ b/gcc/objc/Make-lang.in
@@ -68,9 +68,11 @@  cc1obj-checksum.c : build/genchecksum$(build_exeext) checksum-options \
 cc1obj-checksum.o : cc1obj-checksum.c $(CONFIG_H) $(SYSTEM_H)
 
 cc1obj$(exeext): $(OBJC_OBJS) $(C_AND_OBJC_OBJS) cc1obj-checksum.o $(BACKEND) $(LIBDEPS)
+	$(MAKE) link-mutex
 	+$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
 	      $(OBJC_OBJS) $(C_AND_OBJC_OBJS) cc1obj-checksum.o \
-	      $(BACKEND) $(LIBS) $(BACKENDLIBS)
+	      $(BACKEND) $(LIBS) $(BACKENDLIBS); \
+	      $(MAKE) remove-link-mutex
 
 # Objective C language specific files.
 
diff --git a/gcc/objcp/Make-lang.in b/gcc/objcp/Make-lang.in
index 731ca9e..bd84b5f 100644
--- a/gcc/objcp/Make-lang.in
+++ b/gcc/objcp/Make-lang.in
@@ -72,8 +72,10 @@  cc1objplus-checksum.c : build/genchecksum$(build_exeext) checksum-options \
 cc1objplus-checksum.o : cc1objplus-checksum.c $(CONFIG_H) $(SYSTEM_H)
 
 cc1objplus$(exeext): $(OBJCXX_OBJS) cc1objplus-checksum.o $(BACKEND) $(LIBDEPS)
+	$(MAKE) link-mutex
 	+$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
-		$(OBJCXX_OBJS) cc1objplus-checksum.o $(BACKEND) $(LIBS) $(BACKENDLIBS)
+		$(OBJCXX_OBJS) cc1objplus-checksum.o $(BACKEND) $(LIBS) $(BACKENDLIBS); \
+	        $(MAKE) remove-link-mutex
 
 # Objective C++ language specific files.