[2/2] GDB/testsuite: Add/correct gdb.reverse timeout tweaks

Message ID alpine.DEB.1.10.1407231529220.16254@tp.orcam.me.uk
State Committed
Headers

Commit Message

Maciej W. Rozycki July 24, 2014, 10:39 p.m. UTC
  Hi,

 There are three cases in two scripts in the gdb.reverse subset that take 
a particularly long time.  Two of them are already attempted to take care 
of by extending the timeout from the default.  The remaining one has no 
precautions taken.  The timeout extension is ineffective though, it is 
done by adding a constant rather than by scaling and as a result while it 
may work for target boards that get satisfied with the detault test 
timeout of 10s, it does not serve its purpose for slower ones.

 Here are indicative samples of execution times (in seconds) observed for 
these cases respectively, for an ARMv7 Panda board running Linux and a 
`-march=armv5te' multilib:

PASS: gdb.reverse/sigall-reverse.exp: continue to signal exit
elapsed: 385
PASS: gdb.reverse/until-precsave.exp: run to end of main
elapsed: 4440
PASS: gdb.reverse/until-precsave.exp: save process recfile
elapsed: 965

for the same board and a `-mthumb -march=armv5te' multilib:

PASS: gdb.reverse/sigall-reverse.exp: continue to signal exit
elapsed: 465
PASS: gdb.reverse/until-precsave.exp: run to end of main
elapsed: 4191
PASS: gdb.reverse/until-precsave.exp: save process recfile
elapsed: 669

and for QEMU in the system emulation mode and a `-march=armv4t' 
multilib:

PASS: gdb.reverse/sigall-reverse.exp: continue to signal exit
elapsed: 45
PASS: gdb.reverse/until-precsave.exp: run to end of main
elapsed: 433
PASS: gdb.reverse/until-precsave.exp: save process recfile
elapsed: 104

Based on the performance of other tests these two test configurations have 
their default timeout set to 450s and 60s respectively.

 The remaining two multilibs (`-mthumb -march=armv4t' and `-mthumb 
-march=armv7-a') do not produce test results usable enough to have data 
available for these cases.

 Based on these results I have tweaked timeouts for these cases as 
follows.  This, together with a suitable board timeout setting, removes 
timeouts for these cases.  Note that for the default timeout of 10s the 
new setting for the first case in gdb.reverse/until-precsave.exp is 
compatible with the old one, just a bit higher to keep the convention of 
longer timeouts to remain multiples of 30s.  The second case there does 
not need such a high setting so I have lowered it a bit to avoid an
unnecessary delay where this test case genuinely times out.

 Tested with arm-linux-gnueabi, as described above.  OK to apply?

2014-07-24  Maciej W. Rozycki  <macro@codesourcery.com>

	gdb/testsuite/
	* gdb.reverse/sigall-reverse.exp: Increase the timeout by a factor 
	of 2 for a slow test case.  Take the `gdb,timeout' target setting
	into account for this calculation.
	* gdb.reverse/until-precsave.exp: Increase the timeout by a factor
	of 15 and 3 respectively rather than adding 120 for a pair of slow 
	test cases.  Take the `gdb,timeout' target setting into account 
	for this calculation.

  Maciej

gdb-test-reverse-timeout.diff
  

Comments

Pedro Alves July 29, 2014, 12:40 p.m. UTC | #1
Looks good to me.

I wonder though, whether:

On 07/24/2014 11:39 PM, Maciej W. Rozycki wrote:
> +set savedtimeout $timeout
> +if { [target_info exists gdb,timeout]
> +     && $timeout < [target_info gdb,timeout] } {
> +    set oldtimeout [target_info gdb,timeout]
> +} else {
> +    set oldtimeout $timeout
> +}
> +set timeout [expr $oldtimeout * 2]

... this pattern can be somewhat factored into a
procedure? That'd also serve the duty of being the
simple place we document it.

>  gdb_test "continue" "\[process \[0-9\]+ .*" "continue to signal exit" \


Thanks,
Pedro Alves
  
Maciej W. Rozycki Sept. 9, 2014, 4:32 p.m. UTC | #2
On Tue, 29 Jul 2014, Pedro Alves wrote:

> Looks good to me.

 Thanks, I have applied this change now.

> I wonder though, whether:
> 
> On 07/24/2014 11:39 PM, Maciej W. Rozycki wrote:
> > +set savedtimeout $timeout
> > +if { [target_info exists gdb,timeout]
> > +     && $timeout < [target_info gdb,timeout] } {
> > +    set oldtimeout [target_info gdb,timeout]
> > +} else {
> > +    set oldtimeout $timeout
> > +}
> > +set timeout [expr $oldtimeout * 2]
> 
> ... this pattern can be somewhat factored into a
> procedure? That'd also serve the duty of being the
> simple place we document it.

 Good point, that would be a nice improvement indeed.

 I have been continuously being dragged off to various other stuff 
recently though so if I were to consider this improvement a prerequisite 
to pushing the timeout tweaks I have made here, they may well have not 
ended up integrated within a reasonable time frame.  I have therefore 
committed them as they are, as noted above, so as not to let perfect be 
the enemy of good.  I'll keep in mind that our timeouts would benefit from 
some polishing yet and hopefully get back to it sometime sooner rather 
than later.  Unless someone beats me to it, that is -- I won't mind that, 
not a little bit.

 The stuff is easy to grep for, so the pieces to improve should be easy to 
identify.

  Maciej
  

Patch

Index: gdb-fsf-trunk-quilt/gdb/testsuite/gdb.reverse/sigall-reverse.exp
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/testsuite/gdb.reverse/sigall-reverse.exp	2014-05-30 01:43:19.638974509 +0100
+++ gdb-fsf-trunk-quilt/gdb/testsuite/gdb.reverse/sigall-reverse.exp	2014-05-30 01:45:14.148928207 +0100
@@ -262,9 +262,18 @@  gdb_test "continue" \
     "get signal TERM"
 gdb_test "continue" "Breakpoint.*handle_TERM.*" "send signal TERM"
 
+set savedtimeout $timeout
+if { [target_info exists gdb,timeout]
+     && $timeout < [target_info gdb,timeout] } {
+    set oldtimeout [target_info gdb,timeout]
+} else {
+    set oldtimeout $timeout
+}
+set timeout [expr $oldtimeout * 2]
 gdb_test "continue" "\[process \[0-9\]+ .*" "continue to signal exit" \
     "The next instruction is syscall exit_group.* program...y. or n. " \
     "yes"
+set timeout $savedtimeout
 
 foreach sig [lreverse $signals] {
     test_one_sig_reverse $sig
Index: gdb-fsf-trunk-quilt/gdb/testsuite/gdb.reverse/until-precsave.exp
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/testsuite/gdb.reverse/until-precsave.exp	2014-01-03 21:14:07.078675613 +0000
+++ gdb-fsf-trunk-quilt/gdb/testsuite/gdb.reverse/until-precsave.exp	2014-05-30 01:45:14.148928207 +0100
@@ -49,15 +49,22 @@  gdb_test "break $end_of_main" \
     "BP at end of main"
 
 # This can take awhile.
-set oldtimeout $timeout
-set timeout [expr $oldtimeout + 120]
+set savedtimeout $timeout
+if { [target_info exists gdb,timeout]
+     && $timeout < [target_info gdb,timeout] } {
+    set oldtimeout [target_info gdb,timeout]
+} else {
+    set oldtimeout $timeout
+}
+set timeout [expr $oldtimeout * 15]
 gdb_test "continue" "Breakpoint .* set breakpoint 10a here .*" "run to end of main"
 
 # So can this, against gdbserver, for example.
+set timeout [expr $oldtimeout * 3]
 gdb_test "record save $precsave" \
     "Saved core file $precsave with execution log\."  \
     "save process recfile"
-set timeout $oldtimeout
+set timeout $savedtimeout
 
 gdb_test "kill" "" "Kill process, prepare to debug log file" \
     "Kill the program being debugged\\? \\(y or n\\) " "y"