diff mbox

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

Message ID 51924903.1070807@redhat.com
State New
Headers show

Commit Message

Jason Merrill May 14, 2013, 2:24 p.m. UTC 
On 05/14/2013 12:49 AM, Alexandre Oliva wrote:
> However, rather than implementing the locking in Makefiles, I'm thinking
> it might be wiser to do so in a script that takes the lock name and the
> command to run while holding the lock.

Good point.

> trap 'rmdir "$lockdir"; exit $status' 0 1 2 15

In my testing I found that trapping signals other than 0 resulted in 
trying to rmdir twice if the process is interrupted.  The extra exit 
here also seems unnecessary.  And I added a check so that the script 
gives up and steals the lock after waiting 5 minutes.

Comments

Alexandre Oliva May 15, 2013, 7:51 a.m. UTC | #1 
On May 14, 2013, Jason Merrill <jason@redhat.com> wrote:

>> trap 'rmdir "$lockdir"; exit $status' 0 1 2 15

> In my testing I found that trapping signals other than 0 resulted in
> trying to rmdir twice if the process is interrupted.

Interesting...  I guess it's one for the signal and one for the exit...

I borrowed the general approach from libtool's shell locking machinery.
I suppose it might be the case that lock files (in libtool's case) get
removed twice, but I'm pretty sure there was broad portability testing
in the process that led to that construct.  Indeed, I vaguely recall
some oddities about exit status preservation involving such obnoxious
constructs as 'rm ....; (exit $status); exit', but I won't even pretend
to recall the rationale for that ;-)

> The extra exit here also seems unnecessary.

Wouldn't the exit status of an interrupted or failed compilation be that
of the successful rmdir rather than what you'd get without the wrapper?

> And I added a check so that the script gives up and steals the lock
> after waiting 5 minutes.

I'm now a bit concerned about the portability of some constructs you
used there, although the use of this script is only enabled manually.

My concern is that, in spite of:

> +#! /bin/bash

the script is ran with $(SHELL), which IIRC is typically /bin/sh, not
necessarily bash.

> +    if [ $((count++ % 30)) == 0 ]; then

Let's make this more portable, shall we?

How about

  count=`expr $count + 1`
  case $count in *0)

this goes from 30 to 10 seconds, and it doesn't match the first
iteration (should it?)

> +	# Reset if the lock has been renewed.
> +	if [ "$lockdir" -nt lock-stamp.$$ ]; then

Hmm...  I'm not sure how portable -nt is, but there's always

case `ls -dt $lockdir lock-stamp.$$` in
lock-stamp.$$*) ;; # Our stamp file is newer.
*) ...

> +	    echo waiting to acquire $lockdir

Please make it >&2

> +echo $prog "$@"
> +$prog "$@"

I wonder if some day someone will be tempted to add an “exec” to the
final line, without realizing that it will cause the lock dir to be
released too early if at all.  Perhaps a comment at the end noting the
trap will release the lock would avoid this mistake and make the whole
thing clearer.


It would probably be wise to add lock-stamp.* to the make clean rules.


Patch with the above change (the last one) is pre-approved, but if the
other proposed portability changes make sense to you (even though you
documented that bash is required), please put them in too.

Thanks!
Andreas Schwab May 15, 2013, 8 a.m. UTC | #2 
Alexandre Oliva <aoliva@redhat.com> writes:

> constructs as 'rm ....; (exit $status); exit', but I won't even pretend
> to recall the rationale for that ;-)

See the autoconf manual.

Andreas.
diff mbox

Patch

2013-05-14  Jason Merrill  <jason@redhat.com>

	* Makefile.in (LLINKER): New variable.
	* configure.ac: Handle --enable-link-mutex.
	* lock-and-run.sh: New script.

diff --git a/gcc/lock-and-run.sh b/gcc/lock-and-run.sh
new file mode 100644
index 0000000..8beac5a
--- /dev/null
+++ b/gcc/lock-and-run.sh
@@ -0,0 +1,27 @@ 
+#! /bin/bash
+# Shell-based mutex using mkdir.
+
+lockdir=$1 prog=$2; shift 2 || exit 1
+count=0
+# Remember when we started trying to acquire the lock.
+touch lock-stamp.$$
+trap 'rm -r "$lockdir" lock-stamp.$$' 0
+until mkdir "$lockdir" 2>/dev/null; do
+    # Say something periodically in case the lock is stale.
+    if [ $((count++ % 30)) == 0 ]; then
+	# Reset if the lock has been renewed.
+	if [ "$lockdir" -nt lock-stamp.$$ ]; then
+	    touch lock-stamp.$$
+	    count=0
+	# Give up after 5 minutes.
+	elif [ $count == 300 ]; then
+	    echo removing stale $lockdir
+	    rm -r "$lockdir"
+	else
+	    echo waiting to acquire $lockdir
+	fi
+    fi
+    sleep 1
+done
+echo $prog "$@"
+$prog "$@"
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 903125e..b7ade78 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -235,6 +235,13 @@  LINKER = $(CC)
 LINKER_FLAGS = $(CFLAGS)
 endif
 
+# Like LINKER, but use a mutex for serializing front end links.
+ifeq (@DO_LINK_MUTEX@,true)
+LLINKER = $(SHELL) $(srcdir)/lock-and-run.sh linkfe.lck $(LINKER)
+else
+LLINKER = $(LINKER)
+endif
+
 # -------------------------------------------
 # Programs which operate on the build machine
 # -------------------------------------------
diff --git a/gcc/ada/gcc-interface/Make-lang.in b/gcc/ada/gcc-interface/Make-lang.in
index ef12b4b..4fed34f 100644
--- a/gcc/ada/gcc-interface/Make-lang.in
+++ b/gcc/ada/gcc-interface/Make-lang.in
@@ -185,6 +185,7 @@  endif
 GCC_LINKERFLAGS = $(filter-out -Werror, $(ALL_LINKERFLAGS))
 
 GCC_LINK=$(LINKER) $(GCC_LINKERFLAGS) $(LDFLAGS)
+GCC_LLINK=$(LLINKER) $(GCC_LINKERFLAGS) $(LDFLAGS)
 
 # Lists of files for various purposes.
 
@@ -562,7 +563,8 @@  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)
+	+$(GCC_LLINK) -o $@ $(GNAT1_OBJS) $(ADA_BACKEND) \
+	  libcommon-target.a $(LIBS) $(SYSLIBS) $(BACKENDLIBS) $(CFLAGS)
 	$(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..47aa4cb 100644
--- a/gcc/c/Make-lang.in
+++ b/gcc/c/Make-lang.in
@@ -75,7 +75,7 @@  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)
-	+$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ $(C_OBJS) \
+	+$(LLINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ $(C_OBJS) \
 	  cc1-checksum.o $(BACKEND) $(LIBS) $(BACKENDLIBS)
 #
 # Build hooks:
diff --git a/gcc/configure b/gcc/configure
index 39e911c..1f03eac 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
@@ -27061,6 +27065,26 @@  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
+  do_link_mutex=no
+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 a6cdf24..855affd 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -4958,6 +4958,24 @@  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,
+      do_link_mutex=no)
+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..df8ed3e 100644
--- a/gcc/cp/Make-lang.in
+++ b/gcc/cp/Make-lang.in
@@ -100,7 +100,7 @@  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)
-	+$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
+	+$(LLINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
 	      $(CXX_OBJS) cc1plus-checksum.o $(BACKEND) $(LIBS) $(BACKENDLIBS)
 
 ifeq ($(ENABLE_MAINTAINER_RULES), true)
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 1265129..7d7a5c7 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -1293,6 +1293,13 @@  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 on build
+systems with limited free memory.  This functionality requires that
+the default shell be bash, as it uses the 'trap' built-in command.
+The default is not to use such a mutex.
+
 @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..ee70423 100644
--- a/gcc/fortran/Make-lang.in
+++ b/gcc/fortran/Make-lang.in
@@ -98,7 +98,7 @@  gfortran-cross$(exeext): gfortran$(exeext)
 # The compiler itself is called f951.
 f951$(exeext): $(F95_OBJS) \
 		$(BACKEND) $(LIBDEPS) attribs.o
-	+$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
+	+$(LLINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
 		$(F95_OBJS) $(BACKEND) $(ZLIB) $(LIBS) attribs.o \
 		$(BACKENDLIBS)
 
diff --git a/gcc/go/Make-lang.in b/gcc/go/Make-lang.in
index dd98080..5fe4083 100644
--- a/gcc/go/Make-lang.in
+++ b/gcc/go/Make-lang.in
@@ -76,7 +76,7 @@  GO_OBJS = \
 	go/unsafe.o
 
 go1$(exeext): $(GO_OBJS) attribs.o $(BACKEND) $(LIBDEPS)
-	+$(CXX) $(ALL_CXXFLAGS) $(LDFLAGS) -o $@ \
+	+$(LLINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
 	      $(GO_OBJS) attribs.o $(BACKEND) $(LIBS) $(BACKENDLIBS)
 
 # Documentation.
diff --git a/gcc/java/Make-lang.in b/gcc/java/Make-lang.in
index 7ed3220..8a6210f 100644
--- a/gcc/java/Make-lang.in
+++ b/gcc/java/Make-lang.in
@@ -99,7 +99,7 @@  jvspec.o-warn = -Wno-error
 
 jc1$(exeext): $(JAVA_OBJS) $(BACKEND) $(LIBDEPS) attribs.o
 	rm -f $@
-	+$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
+	+$(LLINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
 		$(JAVA_OBJS) $(BACKEND) $(ZLIB) $(LIBICONV) $(LIBS) attribs.o $(BACKENDLIBS)
 
 jcf-dump$(exeext): $(JCFDUMP_OBJS) $(LIBDEPS)
diff --git a/gcc/lto/Make-lang.in b/gcc/lto/Make-lang.in
index a0bdc26..22df32c 100644
--- a/gcc/lto/Make-lang.in
+++ b/gcc/lto/Make-lang.in
@@ -71,7 +71,7 @@  lto.stagefeedback:
 lto-warn = $(STRICT_WARN)
 
 $(LTO_EXE): $(LTO_OBJS) $(BACKEND) $(LIBDEPS)
-	+$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
+	+$(LLINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
 		$(LTO_OBJS) $(BACKEND) $(BACKENDLIBS) $(LIBS)
 
 # Dependencies
diff --git a/gcc/objc/Make-lang.in b/gcc/objc/Make-lang.in
index 9e005c1..f04d606 100644
--- a/gcc/objc/Make-lang.in
+++ b/gcc/objc/Make-lang.in
@@ -68,7 +68,7 @@  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)
-	+$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
+	+$(LLINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
 	      $(OBJC_OBJS) $(C_AND_OBJC_OBJS) cc1obj-checksum.o \
 	      $(BACKEND) $(LIBS) $(BACKENDLIBS)
 
diff --git a/gcc/objcp/Make-lang.in b/gcc/objcp/Make-lang.in
index 731ca9e..ec10fc8 100644
--- a/gcc/objcp/Make-lang.in
+++ b/gcc/objcp/Make-lang.in
@@ -72,7 +72,7 @@  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)
-	+$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
+	+$(LLINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
 		$(OBJCXX_OBJS) cc1objplus-checksum.o $(BACKEND) $(LIBS) $(BACKENDLIBS)
 
 # Objective C++ language specific files.