[PATCHES] {master} Optimize tests `instspc-*.test' for speed.

[Stefano Lattarini]

Finally, here it is the promised patch optimizing the `instspc-*.test'
tests for speed, as suggested by Ralf in a previous thread:
 <http://lists.gnu.org/archive/html/automake-patches/2010-09/msg00172.html>

This patch seems to truly improve the speed of the affected tests, at
least according to these superficial measurements:

 ## PATCH APPLIED ##
 $ time for i in 1 2 3; do
 >   rm -rf instspc-tests.dir;
 >   make check TESTS="`echo instspc*.test`";
 > done
 ...
 real    18m13.248s
 user    5m26.844s
 sys     4m38.737s
 $ time (rm -rf instspc-tests.dir \
 >  && make check -j4 TESTS="`echo instspc*.test`")
 ...
 real    4m8.431s
 user    1m54.875s
 sys     1m34.030s

 ## PATCH NOT APPLIED ##
 $ time for i in 1 2 3; do
 >   make check TESTS="`echo instspc*.test`";
 > done
 ...
 real    26m57.287s
 user    16m36.230s
 sys     7m15.531s
 $ time make check -j4 TESTS="`echo instspc*.test`"
 ...
 real    9m3.231s
 user    5m52.978s
 sys     2m20.049s


There is also a follow-up patch (very minor) which prevents the
`instspc-*.test' tests from creating an unused source file.

OK for master?

Regards,
   Stefano

From 1e2e48ce98a63bf4a197fd9ea1a104e525b1d65e Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Fri, 5 Nov 2010 15:51:56 +0100
Subject: [PATCH 1/2] Optimize tests `instspc-*.test' for speed.

After the split of `instspc.test' into various generated tests,
the running time of the testsuite has noticeably increased, since
all these new generated tests must run aclocal, autoconf and
automake, whereas previously they were run only once (at the
beginning of `instspc.test').  But luckily, since the new tests
share the same input files for the autotools, this situation can
be easily worked around (at the expenses of a slight increase of
complexity for the testsuite scaffolding).

* tests/Makefile.am (instspc-tests.dir/.timestamp): New rule, used
to create the directory `instspc-tests.dir', which contains the
common data files and some autotools-generated files used by the
`instspc-*.test' tests.
($(instspc_tests:.test=.log)): Depend on it.
* tests/instspc-tests.sh: Recognize new option `--generate-data',
and use it to create hand-written and autotools-generated static
files shared by all the `instspc-*.test' tests.
When sourced by the `instspc-*.test' tests, use those previously
created files instead of recreating them from scratch.
(deindent, create_input_data): New subroutines.
($shared_datadir): New global variable.
Some other related changes and refactorings.

From a suggestion by Ralf Wildenhues.
---
 ChangeLog              |   26 +++++
 tests/Makefile.am      |   15 +++-
 tests/Makefile.in      |   15 +++-
 tests/instspc-tests.sh |  242 +++++++++++++++++++++++++++++------------------
 4 files changed, 203 insertions(+), 95 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 4c327b7..0364a7c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,29 @@
+2010-11-14  Stefano Lattarini  <address@hidden>
+
+       Optimize tests `instspc-*.test' for speed.
+       After the split of `instspc.test' into various generated tests,
+       the running time of the testsuite has noticeably increased, since
+       all these new generated tests must run aclocal, autoconf and
+       automake, whereas previously they were run only once (at the
+       beginning of `instspc.test').  But luckily, since the new tests
+       share the same input files for the autotools, this situation can
+       be easily worked around (at the expenses of a slight increase of
+       complexity for the testsuite scaffolding).
+       * tests/Makefile.am (instspc-tests.dir/.timestamp): New rule, used
+       to create the directory `instspc-tests.dir', which contains the
+       common data files and some autotools-generated files used by the
+       `instspc-*.test' tests.
+       ($(instspc_tests:.test=.log)): Depend on it.
+       * tests/instspc-tests.sh: Recognize new option `--generate-data',
+       and use it to create hand-written and autotools-generated static
+       files shared by all the `instspc-*.test' tests.
+       When sourced by the `instspc-*.test' tests, use those previously
+       created files instead of recreating them from scratch.
+       (deindent, create_input_data): New subroutines.
+       ($shared_datadir): New global variable.
+       Some other related changes and refactorings.
+       From a suggestion by Ralf Wildenhues.
+
 2010-11-12  Stefano Lattarini  <address@hidden>
 
        Fix bug in test `backcompat6.test' (MSYS portability).
diff --git a/tests/Makefile.am b/tests/Makefile.am
index a221ffb..8e00344 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -84,9 +84,22 @@ $(instspc_tests): Makefile.am
          } > address@hidden
        $(AM_V_at)chmod a+rx address@hidden && mv -f address@hidden $@
 
-# All instspc*.test tests work by sourcing the instspc-tests.sh script.
+# All instspc-*.test tests work by sourcing the instspc-tests.sh script.
 $(instspc_tests:.test=.log): instspc-tests.sh
 
+# All instspc-*.test tests use shared data generated by instspc-tests.sh,
+# for reasons of speed.  So we must declare dependency of test logs from
+# those data, and rebuild the data when it's out-of-date.
+$(instspc_tests:.test=.log): instspc-tests.dir/.timestamp
+instspc-tests.dir/.timestamp: instspc-tests.sh Makefile.am
+       $(AM_V_at)dir=instspc-tests.dir; \
+         if test -d $$dir; then \
+           find $$dir -type d '!' -perm -200 -exec chmod u+w {} ';'; \
+           rm -rf $$dir; \
+         fi
+       $(AM_V_GEN)$(SHELL) $(srcdir)/instspc-tests.sh --generate-data
+       $(AM_V_at)date > $@
+
 MAINTAINERCLEANFILES += $(instspc_tests)
 EXTRA_DIST += instspc-tests.sh
 XFAIL_TESTS += $(instspc_xfail_tests)
diff --git a/tests/Makefile.in b/tests/Makefile.in
index d6a6877..1bfa192 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -1539,9 +1539,22 @@ $(instspc_tests): Makefile.am
          } > address@hidden
        $(AM_V_at)chmod a+rx address@hidden && mv -f address@hidden $@
 
-# All instspc*.test tests work by sourcing the instspc-tests.sh script.
+# All instspc-*.test tests work by sourcing the instspc-tests.sh script.
 $(instspc_tests:.test=.log): instspc-tests.sh
 
+# All instspc-*.test tests use shared data generated by instspc-tests.sh,
+# for reasons of speed.  So we must declare dependency of test logs from
+# those data, and rebuild the data when it's out-of-date.
+$(instspc_tests:.test=.log): instspc-tests.dir/.timestamp
+instspc-tests.dir/.timestamp: instspc-tests.sh Makefile.am
+       $(AM_V_at)dir=instspc-tests.dir; \
+         if test -d $$dir; then \
+           find $$dir -type d '!' -perm -200 -exec chmod u+w {} ';'; \
+           rm -rf $$dir; \
+         fi
+       $(AM_V_GEN)$(SHELL) $(srcdir)/instspc-tests.sh --generate-data
+       $(AM_V_at)date > $@
+
 # Each test case depends on defs, aclocal, and automake.
 $(TEST_LOGS): defs aclocal-$(APIVERSION) automake-$(APIVERSION)
 
diff --git a/tests/instspc-tests.sh b/tests/instspc-tests.sh
index 90087eb..9beea56 100755
--- a/tests/instspc-tests.sh
+++ b/tests/instspc-tests.sh
@@ -21,18 +21,21 @@
 # Original report from James Amundson about file names with spaces.
 # Other characters added by Paul Eggert.
 #
-# This script fulfills a double role:
+# This script fulfills a threefold role:
 #   1. It generates a Makefile.am snippet, containing the definition
 #      of proper lists of tests.
-#   2. It is sourced by said generated tests with proper parameters
+#   2. It sets up a directory containing some common data files and
+#      autotools-generated files used by said generated tests (this
+#      is done for speed reasons only).
+#   3. It is sourced by said generated tests with proper parameters
 #      pre-set, to run the "meat" of the checks.
-# This setup might seem tricky and over-engineered abuse, but past
+# This setup might seem a tricky and over-engineered abuse, but past
 # (painful) experiences showed that it is indeed required, because
 # the test generation code and test execution code tend to be
 # inextricably coupled and intertwined.
 #
 
-# Be more Bourne compatible (snippet copied from `tests/defs.in').
+# Be more Bourne compatible (snippet copied from `tests/defs').
 DUALCASE=1; export DUALCASE # for MKS sh
 if test -n "${ZSH_VERSION+set}" && (emulate sh) >/dev/null 2>&1; then
   emulate sh
@@ -48,9 +51,12 @@ fi
 set -e
 
 # Sanity and usage checks.
+
 if test x"$instspc_action" = x; then
   if test "$#,$1" = "1,--generate-makefile"; then
     instspc_action=generate-makefile
+  elif test "$#,$1" = "1,--generate-data"; then
+    instspc_action=generate-data
   else
     echo "$0: empty action and no proper command line" >&2
     exit 99
@@ -58,21 +64,31 @@ if test x"$instspc_action" = x; then
 elif test $# -gt 0; then
   echo "$0: action specified and command line arguments used" >&2
   exit 99
-elif test x"$instspc_action" = x"generate-makefile"; then
-  :
-else
-  case $instspc_action in
-    test-build|test-install)
-      if test x"$instspc_test_name" = x; then
-        echo "$0: test name undefined for action '$instspc_action'" >&2
-        exit 99
-      fi;;
-    *)
-      echo "$0: invalid action: '$instspc_action'"
-      exit 99;;
-  esac
 fi
 
+case $instspc_action in
+  generate-makefile)
+    ;;
+  generate-data)
+    # We'll need some definitions provided by `tests/defs-static'.
+    # Temporarly disable the errexit flag, since the code in that
+    # file might not be prepared to deal with it.
+    set +e
+    . ./defs-static || exit 1
+    set -e
+    ;;
+  test-build|test-install)
+    if test x"$instspc_test_name" = x; then
+      echo "$0: test name undefined for action '$instspc_action'" >&2
+      exit 99
+    fi
+    ;;
+  *)
+    echo "$0: invalid action: '$instspc_action'"
+    exit 99
+    ;;
+esac
+
 # Helper subroutine for test data definition.
 # Usage: define_problematic_string NAME STRING
 define_problematic_string ()
@@ -94,6 +110,106 @@ define_problematic_string ()
   esac
 }
 
+# Helper subroutines for creation of input data files.
+
+deindent ()
+{
+  sed 's/^ *//' # we don't strip leading tabs -- this is deliberate!
+}
+
+# Usage: create_input_data TARGET-DIRECTORY
+create_input_data ()
+{
+  ocwd=`pwd` || exit 1
+  dir=$1; shift
+
+  mkdir "$dir" || {
+     echo "$me: cannot create data directory \`$dir'" >&2
+     exit 1
+  }
+  cd "$dir" || {
+     echo "$me: cannot chdir to data directory \`$dir'" >&2
+     exit 1
+  }
+
+  mkdir sub
+
+  deindent > configure.in << 'EOF'
+    AC_INIT([instspc], [1.0])
+    AM_INIT_AUTOMAKE
+    AC_CONFIG_FILES([Makefile])
+    AC_PROG_CC
+    AC_PROG_RANLIB
+    AC_OUTPUT
+EOF
+
+  : > sub/base.h
+  : > sub/nobase.h
+  : > sub/base.dat
+  : > sub/nobase.dat
+  : > sub/base.sh
+  : > sub/nobase.sh
+
+  deindent > source.c << 'EOF'
+    int
+    main (int argc, char **argv)
+    {
+      return 0;
+    }
+EOF
+  cp source.c source2.c
+
+  deindent > Makefile.am << 'EOF'
+    foodir = $(prefix)/foo
+    fooexecdir = $(prefix)/foo
+
+    foo_HEADERS = sub/base.h
+    nobase_foo_HEADERS = sub/nobase.h
+
+    dist_foo_DATA = sub/base.dat
+    nobase_dist_foo_DATA = sub/nobase.dat
+
+    dist_fooexec_SCRIPTS = sub/base.sh
+    nobase_dist_fooexec_SCRIPTS = sub/nobase.sh
+
+    fooexec_PROGRAMS = sub/base
+    nobase_fooexec_PROGRAMS = sub/nobase
+    sub_base_SOURCES = source.c
+    sub_nobase_SOURCES = source.c
+
+    fooexec_LIBRARIES = sub/libbase.a
+    nobase_fooexec_LIBRARIES = sub/libnobase.a
+    sub_libbase_a_SOURCES = source.c
+    sub_libnobase_a_SOURCES = source.c
+
+    .PHONY: test-install-sep
+    test-install-sep: install
+       test   -f '$(DESTDIR)/$(file)-prefix/foo/sub/nobase.h'
+       test ! -f '$(DESTDIR)/$(file)-prefix/foo/nobase.h'
+       test   -f '$(DESTDIR)/$(file)-prefix/foo/base.h'
+       test   -f '$(DESTDIR)/$(file)-prefix/foo/sub/nobase.dat'
+       test ! -f '$(DESTDIR)/$(file)-prefix/foo/nobase.dat'
+       test   -f '$(DESTDIR)/$(file)-prefix/foo/base.dat'
+       test   -f '$(DESTDIR)/$(file)-prefix/foo/sub/nobase.sh'
+       test ! -f '$(DESTDIR)/$(file)-prefix/foo/nobase.sh'
+       test   -f '$(DESTDIR)/$(file)-prefix/foo/base.sh'
+       test   -f '$(DESTDIR)/$(file)-prefix/foo/sub/nobase$(EXEEXT)'
+       test ! -f '$(DESTDIR)/$(file)-prefix/foo/nobase$(EXEEXT)'
+       test   -f '$(DESTDIR)/$(file)-prefix/foo/base$(EXEEXT)'
+       test   -f '$(DESTDIR)/$(file)-prefix/foo/sub/libnobase.a'
+       test ! -f '$(DESTDIR)/$(file)-prefix/foo/libnobase.a'
+       test   -f '$(DESTDIR)/$(file)-prefix/foo/libbase.a'
+EOF
+
+  $ACLOCAL
+  $AUTOCONF
+  $AUTOMAKE -a
+
+  cd "$ocwd" || exit 1
+}
+
+shared_datadir=instspc-tests.dir
+
 # Be sure to avoid interferences from the environment.
 instspc_names_list=''
 instspc_xfail_builds_list=''
@@ -187,6 +303,9 @@ if test x"$instspc_action" = x"generate-makefile"; then
     echo "instspc_xfail_tests += instspc-$test_name-install.test"
   done
   exit 0
+elif test x"$instspc_action" = x"generate-data"; then
+  create_input_data "$shared_datadir"
+  exit 0
 fi
 
 ###  If we are still here, we have to run a test ...
@@ -198,6 +317,14 @@ set +e
 . ./defs || Exit 99
 set -e
 
+# But the directory set up by the `--generate-data' option should contain
+# all the files we need.  So remove the other files created by ./defs.
+# And check we really are in a temporary `*.dir' directory in the build
+# tree, since the last thing we want is to remove soem randoom user files!
+test -f ../defs-static && test -f ../defs || Exit 99
+case `pwd` in *.dir);; *) Exit 99;; esac
+rm -f *
+
 eval "instspc_test_string=\${instspc__$instspc_test_name}" || Exit 99
 if test x"$instspc_test_string" = x; then
   echo "$me: invalid test name: '$instspc_test_name'" >&2
@@ -207,84 +334,15 @@ fi
 # Skip if this system doesn't support these characters in file names.
 mkdir "./$instspc_test_string" || Exit 77
 
-mkdir sub sub1
-
-cat >> configure.in << 'EOF'
-AC_PROG_CC
-AC_PROG_RANLIB
-AC_OUTPUT
-EOF
-
-: > sub/base.h
-: > sub/nobase.h
-: > sub/base.dat
-: > sub/nobase.dat
-: > sub/base.sh
-: > sub/nobase.sh
-
-cat > source.c << 'EOF'
-int
-main (int argc, char **argv)
-{
-  return 0;
-}
-EOF
-cp source.c source2.c
-
-cat > Makefile.am << 'EOF'
-foodir = $(prefix)/foo
-fooexecdir = $(prefix)/foo
-
-foo_HEADERS = sub/base.h
-nobase_foo_HEADERS = sub/nobase.h
-
-dist_foo_DATA = sub/base.dat
-nobase_dist_foo_DATA = sub/nobase.dat
-
-dist_fooexec_SCRIPTS = sub/base.sh
-nobase_dist_fooexec_SCRIPTS = sub/nobase.sh
-
-fooexec_PROGRAMS = sub/base
-nobase_fooexec_PROGRAMS = sub/nobase
-sub_base_SOURCES = source.c
-sub_nobase_SOURCES = source.c
-
-fooexec_LIBRARIES = sub/libbase.a
-nobase_fooexec_LIBRARIES = sub/libnobase.a
-sub_libbase_a_SOURCES = source.c
-sub_libnobase_a_SOURCES = source.c
-
-.PHONY: test-install-sep
-test-install-sep: install
-       test   -f '$(DESTDIR)/$(file)-prefix/foo/sub/nobase.h'
-       test ! -f '$(DESTDIR)/$(file)-prefix/foo/nobase.h'
-       test   -f '$(DESTDIR)/$(file)-prefix/foo/base.h'
-       test   -f '$(DESTDIR)/$(file)-prefix/foo/sub/nobase.dat'
-       test ! -f '$(DESTDIR)/$(file)-prefix/foo/nobase.dat'
-       test   -f '$(DESTDIR)/$(file)-prefix/foo/base.dat'
-       test   -f '$(DESTDIR)/$(file)-prefix/foo/sub/nobase.sh'
-       test ! -f '$(DESTDIR)/$(file)-prefix/foo/nobase.sh'
-       test   -f '$(DESTDIR)/$(file)-prefix/foo/base.sh'
-       test   -f '$(DESTDIR)/$(file)-prefix/foo/sub/nobase$(EXEEXT)'
-       test ! -f '$(DESTDIR)/$(file)-prefix/foo/nobase$(EXEEXT)'
-       test   -f '$(DESTDIR)/$(file)-prefix/foo/base$(EXEEXT)'
-       test   -f '$(DESTDIR)/$(file)-prefix/foo/sub/libnobase.a'
-       test ! -f '$(DESTDIR)/$(file)-prefix/foo/libnobase.a'
-       test   -f '$(DESTDIR)/$(file)-prefix/foo/libbase.a'
-EOF
-
-$ACLOCAL
-$AUTOCONF
-$AUTOMAKE -a
-
 case $instspc_action in
   test-build)
-    build=$instspc_test_string
-    dest=`pwd`/sub1
+    dest=`pwd`/_dest
+    relbuilddir=../..
+    cd "./$instspc_test_string"
     ;;
   test-install)
-    build=sub1
     dest=`pwd`/$instspc_test_string
+    relbuilddir=..
     ;;
   *)
     echo "$me: internal error: invalid action '$instspc_action'"
@@ -292,9 +350,7 @@ case $instspc_action in
     ;;
 esac
 
-cd "./$build"
-
-../configure --prefix "/$instspc_test_string-prefix"
+$relbuilddir/$shared_datadir/configure --prefix "/$instspc_test_string-prefix"
 $MAKE
 DESTDIR="$dest" file="$instspc_test_string" $MAKE -e test-install-sep
 
-- 
1.7.1

From bd51ef14239101a58691e2d6c62291814c7622d5 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Sun, 14 Nov 2010 18:10:37 +0100
Subject: [PATCH 2/2] Tests `instspc-*.test': do not create useless source file.

* tests/instspc-tests.sh (create_input_data): Do not create
unused source file `source2.c'.
---
 ChangeLog              |    4 ++++
 tests/instspc-tests.sh |    1 -
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 0364a7c..61eb611 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2010-11-14  Stefano Lattarini  <address@hidden>
 
+       Tests `instspc-*.test': do not create useless source file.
+       * tests/instspc-tests.sh (create_input_data): Do not create
+       unused source file `source2.c'.
+
        Optimize tests `instspc-*.test' for speed.
        After the split of `instspc.test' into various generated tests,
        the running time of the testsuite has noticeably increased, since
diff --git a/tests/instspc-tests.sh b/tests/instspc-tests.sh
index 9beea56..3deac1a 100755
--- a/tests/instspc-tests.sh
+++ b/tests/instspc-tests.sh
@@ -157,7 +157,6 @@ EOF
       return 0;
     }
 EOF
-  cp source.c source2.c
 
   deindent > Makefile.am << 'EOF'
     foodir = $(prefix)/foo
-- 
1.7.1