[5/8] gen-pert-test: parallel build support
Commit Message
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.
} 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]
}
}
Comments
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.
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.
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.
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.
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.
@@ -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}; \
b/gdb/testsuite/lib/build-piece.exp
new file mode 100644
@@ -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
@@ -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]
@@ -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]