[PATCH 5/8] gen-pert-test: parallel build support

[PATCH 5/8] gen-pert-test: parallel build support

[Doug Evans]

Hi.

This patch adds parallel build support for perf testcases.
To use the existing machinery, GDB_PARALLEL now contains
the subdirectory in which to put the parallel builds.

2015-07-20  Doug Evans  <dje@google.com>

	* Makefile.in (check/%.exp): Pass directory for GDB_PARALLEL.
	(workers/%.worker, build-perf): New rule.
	(GDB_PERFTEST_MODE): New variable.
	(check-perf): Use it.
	(clean): Clean up gdb.perf parallel build subdirs.
	* lib/build-piece.exp: New file.
	* lib/cache.exp (gdb_do_cache): Include $GDB_PARALLEL in path name.
	* lib/gdb.exp (standard_output_file): Include $GDB_PARALLEL in path
	name.
	(standard_temp_file): Ditto.
	(GDB_PARALLEL handling): Make outputs,temp,cache directories as subdirs
	of $GDB_PARALLEL.

diff --git a/gdb/testsuite/Makefile.in b/gdb/testsuite/Makefile.in
index c064f06..ffda7b9 100644
--- a/gdb/testsuite/Makefile.in
+++ b/gdb/testsuite/Makefile.in
@@ -227,16 +227,48 @@ do-check-parallel: $(TEST_TARGETS)

  @GMAKE_TRUE@check/%.exp:
  @GMAKE_TRUE@	-mkdir -p outputs/$*
-@GMAKE_TRUE@	@$(DO_RUNTEST) GDB_PARALLEL=yes --outdir=outputs/$* $*.exp  
$(RUNTESTFLAGS)
+@GMAKE_TRUE@	@$(DO_RUNTEST) GDB_PARALLEL=. --outdir=outputs/$* $*.exp  
$(RUNTESTFLAGS)

  check/no-matching-tests-found:
  	@echo ""
  	@echo "No matching tests found."
  	@echo ""

+# Utility rule invoked by step 2 of the build-perf rule.
+@GMAKE_TRUE@workers/%.worker:
+@GMAKE_TRUE@	mkdir -p gdb.perf/outputs/$*
+@GMAKE_TRUE@	$(DO_RUNTEST) --status --outdir=gdb.perf/outputs/$*  
lib/build-piece.exp WORKER=$* GDB_PARALLEL=gdb.perf $(RUNTESTFLAGS)  
GDB_PERFTEST_MODE=compile GDB_PERFTEST_SUBMODE=build-pieces
+
+# Utility rule to build tests that support it in parallel.
+# The build is broken into 3 steps distinguished by GDB_PERFTEST_SUBMODE:
+# gen-workers, build-pieces, final.
+#
+# GDB_PERFTEST_MODE appears *after* RUNTESTFLAGS here because we don't want
+# anything in RUNTESTFLAGS to override it.
+#
+# We don't delete the outputs directory here as these programs can take
+# awhile to build, and perftest.exp has support for deciding whether to
+# recompile them.  If you want to remove these directories, make clean.
+#
+# The point of step 1 is to construct the set of worker tasks for step 2.
+# All of the information needed by build-piece.exp is contained in the name
+# of the generated .worker file.
+@GMAKE_TRUE@build-perf: $(abs_builddir)/site.exp
+@GMAKE_TRUE@	rm -rf gdb.perf/workers
+@GMAKE_TRUE@	mkdir -p gdb.perf/workers
+@GMAKE_TRUE@	@: Step 1: Generate the build .worker files.
+@GMAKE_TRUE@	$(DO_RUNTEST) --status --directory=gdb.perf --outdir  
gdb.perf/workers GDB_PARALLEL=gdb.perf $(RUNTESTFLAGS)  
GDB_PERFTEST_MODE=compile GDB_PERFTEST_SUBMODE=gen-workers
+@GMAKE_TRUE@	@: Step 2: Compile the pieces.  Here is the build parallelism.
+@GMAKE_TRUE@	$(MAKE) $$(cd gdb.perf && echo workers/*/*.worker)
+@GMAKE_TRUE@	@: Step 3: Do the final link.
+@GMAKE_TRUE@	$(DO_RUNTEST) --status --directory=gdb.perf --outdir gdb.perf  
GDB_PARALLEL=gdb.perf $(RUNTESTFLAGS) GDB_PERFTEST_MODE=compile  
GDB_PERFTEST_SUBMODE=final
+
+# The default is to both compile and run the tests.
+GDB_PERFTEST_MODE = both
+
  check-perf: all $(abs_builddir)/site.exp
  	@if test ! -d gdb.perf; then mkdir gdb.perf; fi
-	$(DO_RUNTEST) --directory=gdb.perf --outdir gdb.perf  
GDB_PERFTEST_MODE=both $(RUNTESTFLAGS)
+	$(DO_RUNTEST) --directory=gdb.perf --outdir gdb.perf  
GDB_PERFTEST_MODE=$(GDB_PERFTEST_MODE) $(RUNTESTFLAGS)

  force:;

@@ -245,6 +277,7 @@ clean mostlyclean:
  	-rm -f core.* *.tf *.cl tracecommandsscript copy1.txt zzz-gdbscript
  	-rm -f *.dwo *.dwp
  	-rm -rf outputs temp cache
+	-rm -rf gdb.perf/workers gdb.perf/outputs gdb.perf/temp gdb.perf/cache
  	-rm -f read1.so expect-read1
  	if [ x"${ALL_SUBDIRS}" != x ] ; then \
  	    for dir in ${ALL_SUBDIRS}; \
diff --git a/gdb/testsuite/lib/build-piece.exp  
b/gdb/testsuite/lib/build-piece.exp
new file mode 100644
index 0000000..a81530c
--- /dev/null
+++ b/gdb/testsuite/lib/build-piece.exp
@@ -0,0 +1,39 @@
+# Copyright (C) 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Utility to bootstrap building a piece of a performance test in a
+# parallel build.
+# See testsuite/Makefile.in:workers/%.worker.
+# WORKER is set by the makefile and is
+# "{program_name}/{program_name}-{worker_nr}".
+
+regexp "^\(.+\)/\(.+\)-\(\[0-9\]+\)$" $WORKER entire_match PROGRAM_NAME  
pname2 WORKER_NR
+
+if { ![info exists entire_match] || $entire_match != $WORKER } {
+    error "Bad value for WORKER: $WORKER"
+}
+if { $PROGRAM_NAME != $pname2 } {
+    error "Bad value for WORKER: $WORKER"
+}
+
+# $subdir is set to "lib", because that is where this file lives,
+# which is not what tests expect.
+set subdir "gdb.perf"
+
+# $gdb_test_file_name is set to this file, build-piece, which is not what
+# tests expect.
+set gdb_test_file_name $PROGRAM_NAME
+
+source $srcdir/$subdir/${gdb_test_file_name}.exp
diff --git a/gdb/testsuite/lib/cache.exp b/gdb/testsuite/lib/cache.exp
index 8df04b9..9565b39 100644
--- a/gdb/testsuite/lib/cache.exp
+++ b/gdb/testsuite/lib/cache.exp
@@ -35,7 +35,7 @@ proc gdb_do_cache {name} {
      }

      if {[info exists GDB_PARALLEL]} {
-	set cache_filename [file join $objdir cache $cache_name]
+	set cache_filename [file join $objdir $GDB_PARALLEL cache $cache_name]
  	if {[file exists $cache_filename]} {
  	    set fd [open $cache_filename]
  	    set gdb_data_cache($cache_name) [read -nonewline $fd]
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 0805de9..42638d6 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -3954,7 +3954,7 @@ proc standard_output_file {basename} {
      global objdir subdir gdb_test_file_name GDB_PARALLEL

      if {[info exists GDB_PARALLEL]} {
-	set dir [file join $objdir outputs $subdir $gdb_test_file_name]
+	set dir [file join $objdir $GDB_PARALLEL outputs $subdir  
$gdb_test_file_name]
  	file mkdir $dir
  	return [file join $dir $basename]
      } else {
@@ -3968,7 +3968,7 @@ proc standard_temp_file {basename} {
      global objdir GDB_PARALLEL

      if {[info exists GDB_PARALLEL]} {
-	return [file join $objdir temp $basename]
+	return [file join $objdir $GDB_PARALLEL temp $basename]
      } else {
  	return $basename
      }
@@ -5084,7 +5084,10 @@ if {[info exists GDB_PARALLEL]} {
      if {[is_remote host]} {
  	unset GDB_PARALLEL
      } else {
-	file mkdir outputs temp cache
+	file mkdir \
+	    [file join $GDB_PARALLEL outputs] \
+	    [file join $GDB_PARALLEL temp] \
+	    [file join $GDB_PARALLEL cache]
      }
  }

Re: [PATCH 5/8] gen-pert-test: parallel build support

[Patrick Palka]

On Tue, Jul 21, 2015 at 9:44 AM, Doug Evans <dje@google.com> wrote:
> Hi.
>
> This patch adds parallel build support for perf testcases.
> To use the existing machinery, GDB_PARALLEL now contains
> the subdirectory in which to put the parallel builds.
>
> 2015-07-20  Doug Evans  <dje@google.com>
>
>         * Makefile.in (check/%.exp): Pass directory for GDB_PARALLEL.
>         (workers/%.worker, build-perf): New rule.
>         (GDB_PERFTEST_MODE): New variable.
>         (check-perf): Use it.
>         (clean): Clean up gdb.perf parallel build subdirs.
>         * lib/build-piece.exp: New file.
>         * lib/cache.exp (gdb_do_cache): Include $GDB_PARALLEL in path name.
>         * lib/gdb.exp (standard_output_file): Include $GDB_PARALLEL in path
>         name.
>         (standard_temp_file): Ditto.
>         (GDB_PARALLEL handling): Make outputs,temp,cache directories as
> subdirs
>         of $GDB_PARALLEL.

This patch seems to have caused a number of regressions:

http://gdb-build.sergiodj.net/builders/Fedora-x86_64-m64/builds/1502
http://gdb-build.sergiodj.net/builders/Fedora-x86_64-m32/builds/1501
http://gdb-build.sergiodj.net/builders/Fedora-i686/builds/1510

(possibly among others)

Reverting this patch locally makes the regressions disappear for me,
at least on x86_64-m64.

Re: [PATCH 5/8] gen-pert-test: parallel build support

[Doug Evans]

On Sat, Jul 25, 2015 at 9:44 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> On Tue, Jul 21, 2015 at 9:44 AM, Doug Evans <dje@google.com> wrote:
>> Hi.
>>
>> This patch adds parallel build support for perf testcases.
>> To use the existing machinery, GDB_PARALLEL now contains
>> the subdirectory in which to put the parallel builds.
>>
>> 2015-07-20  Doug Evans  <dje@google.com>
>>
>>         * Makefile.in (check/%.exp): Pass directory for GDB_PARALLEL.
>>         (workers/%.worker, build-perf): New rule.
>>         (GDB_PERFTEST_MODE): New variable.
>>         (check-perf): Use it.
>>         (clean): Clean up gdb.perf parallel build subdirs.
>>         * lib/build-piece.exp: New file.
>>         * lib/cache.exp (gdb_do_cache): Include $GDB_PARALLEL in path name.
>>         * lib/gdb.exp (standard_output_file): Include $GDB_PARALLEL in path
>>         name.
>>         (standard_temp_file): Ditto.
>>         (GDB_PARALLEL handling): Make outputs,temp,cache directories as
>> subdirs
>>         of $GDB_PARALLEL.
>
> This patch seems to have caused a number of regressions:
>
> http://gdb-build.sergiodj.net/builders/Fedora-x86_64-m64/builds/1502
> http://gdb-build.sergiodj.net/builders/Fedora-x86_64-m32/builds/1501
> http://gdb-build.sergiodj.net/builders/Fedora-i686/builds/1510
>
> (possibly among others)
>
> Reverting this patch locally makes the regressions disappear for me,
> at least on x86_64-m64.

I can't repro this.
It's odd that this patch would cause these particular regressions:

http://gdb-build.sergiodj.net/builders/Fedora-x86_64-m32/builds/1501/steps/regressions/logs/regressions

http://gdb-build.sergiodj.net/builders/Fedora-x86_64-m64/builds/1502/steps/regressions/logs/regressions

Also, I don't understand this one:

http://gdb-build.sergiodj.net/builders/Debian-x86_64-m64/builds/1420

I can imagine makefile hacking for the perf tests breaking normal make check,
but then I'd expect the damage to be far more extensive, but the patch
reported in 1420 cannot have caused those regressions.

Re: [PATCH 5/8] gen-pert-test: parallel build support

[Patrick Palka]

On Sat, Jul 25, 2015 at 2:25 PM, Doug Evans <xdje42@gmail.com> wrote:
> On Sat, Jul 25, 2015 at 9:44 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
>> On Tue, Jul 21, 2015 at 9:44 AM, Doug Evans <dje@google.com> wrote:
>>> Hi.
>>>
>>> This patch adds parallel build support for perf testcases.
>>> To use the existing machinery, GDB_PARALLEL now contains
>>> the subdirectory in which to put the parallel builds.
>>>
>>> 2015-07-20  Doug Evans  <dje@google.com>
>>>
>>>         * Makefile.in (check/%.exp): Pass directory for GDB_PARALLEL.
>>>         (workers/%.worker, build-perf): New rule.
>>>         (GDB_PERFTEST_MODE): New variable.
>>>         (check-perf): Use it.
>>>         (clean): Clean up gdb.perf parallel build subdirs.
>>>         * lib/build-piece.exp: New file.
>>>         * lib/cache.exp (gdb_do_cache): Include $GDB_PARALLEL in path name.
>>>         * lib/gdb.exp (standard_output_file): Include $GDB_PARALLEL in path
>>>         name.
>>>         (standard_temp_file): Ditto.
>>>         (GDB_PARALLEL handling): Make outputs,temp,cache directories as
>>> subdirs
>>>         of $GDB_PARALLEL.
>>
>> This patch seems to have caused a number of regressions:
>>
>> http://gdb-build.sergiodj.net/builders/Fedora-x86_64-m64/builds/1502
>> http://gdb-build.sergiodj.net/builders/Fedora-x86_64-m32/builds/1501
>> http://gdb-build.sergiodj.net/builders/Fedora-i686/builds/1510
>>
>> (possibly among others)
>>
>> Reverting this patch locally makes the regressions disappear for me,
>> at least on x86_64-m64.
>
> I can't repro this.
> It's odd that this patch would cause these particular regressions:
>
> http://gdb-build.sergiodj.net/builders/Fedora-x86_64-m32/builds/1501/steps/regressions/logs/regressions
>
> http://gdb-build.sergiodj.net/builders/Fedora-x86_64-m64/builds/1502/steps/regressions/logs/regressions
>
> Also, I don't understand this one:
>
> http://gdb-build.sergiodj.net/builders/Debian-x86_64-m64/builds/1420
>
> I can imagine makefile hacking for the perf tests breaking normal make check,
> but then I'd expect the damage to be far more extensive, but the patch
> reported in 1420 cannot have caused those regressions.

In this case, the regressions listed for build 1420 originated from build 1417:

http://gdb-build.sergiodj.net/builders/Debian-x86_64-m64/builds/1417

The baseline for checking for regressions is not the build before, but
rather it's a "rolling" baseline or something like that.

Re: [PATCH 5/8] gen-pert-test: parallel build support

[Doug Evans]

On Sat, Jul 25, 2015 at 11:25 AM, Doug Evans <xdje42@gmail.com> wrote:
> On Sat, Jul 25, 2015 at 9:44 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
>> On Tue, Jul 21, 2015 at 9:44 AM, Doug Evans <dje@google.com> wrote:
>>> Hi.
>>>
>>> This patch adds parallel build support for perf testcases.
>>> To use the existing machinery, GDB_PARALLEL now contains
>>> the subdirectory in which to put the parallel builds.
>>>
>>> 2015-07-20  Doug Evans  <dje@google.com>
>>>
>>>         * Makefile.in (check/%.exp): Pass directory for GDB_PARALLEL.
>>>         (workers/%.worker, build-perf): New rule.
>>>         (GDB_PERFTEST_MODE): New variable.
>>>         (check-perf): Use it.
>>>         (clean): Clean up gdb.perf parallel build subdirs.
>>>         * lib/build-piece.exp: New file.
>>>         * lib/cache.exp (gdb_do_cache): Include $GDB_PARALLEL in path name.
>>>         * lib/gdb.exp (standard_output_file): Include $GDB_PARALLEL in path
>>>         name.
>>>         (standard_temp_file): Ditto.
>>>         (GDB_PARALLEL handling): Make outputs,temp,cache directories as
>>> subdirs
>>>         of $GDB_PARALLEL.
>>
>> This patch seems to have caused a number of regressions:
>>
>> http://gdb-build.sergiodj.net/builders/Fedora-x86_64-m64/builds/1502
>> http://gdb-build.sergiodj.net/builders/Fedora-x86_64-m32/builds/1501
>> http://gdb-build.sergiodj.net/builders/Fedora-i686/builds/1510
>>
>> (possibly among others)
>>
>> Reverting this patch locally makes the regressions disappear for me,
>> at least on x86_64-m64.
>
> I can't repro this.
> It's odd that this patch would cause these particular regressions:
>
> http://gdb-build.sergiodj.net/builders/Fedora-x86_64-m32/builds/1501/steps/regressions/logs/regressions
>
> http://gdb-build.sergiodj.net/builders/Fedora-x86_64-m64/builds/1502/steps/regressions/logs/regressions
>
> Also, I don't understand this one:
>
> http://gdb-build.sergiodj.net/builders/Debian-x86_64-m64/builds/1420
>
> I can imagine makefile hacking for the perf tests breaking normal make check,
> but then I'd expect the damage to be far more extensive, but the patch
> reported in 1420 cannot have caused those regressions.

Ok, I think I see it now.
I'll revert the patch and submit a modified one.

Re: [PATCH 5/8] gen-pert-test: parallel build support

[Sergio Durigan Junior]

On Saturday, July 25 2015, Patrick Palka wrote:

>>> This patch seems to have caused a number of regressions:
>>>
>>> http://gdb-build.sergiodj.net/builders/Fedora-x86_64-m64/builds/1502
>>> http://gdb-build.sergiodj.net/builders/Fedora-x86_64-m32/builds/1501
>>> http://gdb-build.sergiodj.net/builders/Fedora-i686/builds/1510
>>>
>>> (possibly among others)
>>>
>>> Reverting this patch locally makes the regressions disappear for me,
>>> at least on x86_64-m64.
>>
>> I can't repro this.
>> It's odd that this patch would cause these particular regressions:
>>
>> http://gdb-build.sergiodj.net/builders/Fedora-x86_64-m32/builds/1501/steps/regressions/logs/regressions
>>
>> http://gdb-build.sergiodj.net/builders/Fedora-x86_64-m64/builds/1502/steps/regressions/logs/regressions
>>
>> Also, I don't understand this one:
>>
>> http://gdb-build.sergiodj.net/builders/Debian-x86_64-m64/builds/1420
>>
>> I can imagine makefile hacking for the perf tests breaking normal make check,
>> but then I'd expect the damage to be far more extensive, but the patch
>> reported in 1420 cannot have caused those regressions.
>
> In this case, the regressions listed for build 1420 originated from build 1417:
>
> http://gdb-build.sergiodj.net/builders/Debian-x86_64-m64/builds/1417
>
> The baseline for checking for regressions is not the build before, but
> rather it's a "rolling" baseline or something like that.

Sorry, as it turns out I found a bug in the way BuildBot calculated the
regressions for the current build.  Unfortunately it wasn't correctly
updating the previous_gdb.sum file, which is used to directly calculate
the regressions against the last build.  The bug has been fixed now; it
has been introduced a few days ago, so not many builds have been
compromised.  Also, the regressions reported *were* valid; the only
problem was that they were being reported over and over for every build.

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/