testsuite: Of all the timeouts provided always select the largest

testsuite: Of all the timeouts provided always select the largest

[Maciej W. Rozycki]

Hello,

 Some targets require long time to perform certain operations.  An example 
is a hardware board (i.e. non-simulated one) driven through a serial 
interface of some kind or the target itself may be slow.  Board 
description files can provide their own timeout setting, overriding 
defaults, like the global one or the tool-specific one.  Unfortunately 
this does not apply to explicit timeouts used with gdb_expect here and 
there.  I have seen cases of test suite failures resulting from that.

 To rectify I have modified gdb_expect so that one timeout can never lower 
any other one.  A patch follows.  No regressions with the 
mipsisa32-sde-elf target using the mips-sim-sde32/-EB/-march=mips32r2 
board.  It fixes several timeouts seen with the mips-fs2-sdelib board in 
some configurations though.

2008-02-25  Maciej W. Rozycki  <macro@mips.com>

	* lib/gdb.exp (gdb_expect): Of all the timeouts provided always
	select the largest.

 OK to apply?

  Maciej

14614.diff
Index: gdb/src/gdb/testsuite/lib/gdb.exp
===================================================================
--- gdb.orig/src/gdb/testsuite/lib/gdb.exp	2007-02-13 16:40:36.000000000 +0000
+++ gdb/src/gdb/testsuite/lib/gdb.exp	2007-02-13 16:40:45.000000000 +0000
@@ -1697,34 +1697,44 @@
 
 proc gdb_expect { args } {
     if { [llength $args] == 2  && [lindex $args 0] != "-re" } {
-	set gtimeout [lindex $args 0];
+	set atimeout [lindex $args 0];
 	set expcode [list [lindex $args 1]];
     } else {
-	upvar timeout timeout;
-
 	set expcode $args;
-	if [target_info exists gdb,timeout] {
-	    if [info exists timeout] {
-		if { $timeout < [target_info gdb,timeout] } {
-		    set gtimeout [target_info gdb,timeout];
-		} else {
-		    set gtimeout $timeout;
-		}
-	    } else {
+    }
+
+    upvar timeout timeout;
+
+    if [target_info exists gdb,timeout] {
+	if [info exists timeout] {
+	    if { $timeout < [target_info gdb,timeout] } {
 		set gtimeout [target_info gdb,timeout];
+	    } else {
+		set gtimeout $timeout;
 	    }
+	} else {
+	    set gtimeout [target_info gdb,timeout];
 	}
+    }
 
+    if ![info exists gtimeout] {
+	global timeout;
+	if [info exists timeout] {
+	    set gtimeout $timeout;
+	}
+    }
+
+    if [info exists atimeout] {
+	if { ![info exists gtimeout] || $gtimeout < $atimeout } {
+	    set $gtimeout $atimeout;
+	}
+    } else {
 	if ![info exists gtimeout] {
-	    global timeout;
-	    if [info exists timeout] {
-		set gtimeout $timeout;
-	    } else {
-		# Eeeeew.
-		set gtimeout 60;
-	    }
+	    # Eeeeew.
+	    set gtimeout 60;
 	}
     }
+
     global suppress_flag;
     global remote_suppress_flag;
     if [info exists remote_suppress_flag] {

Re: testsuite: Of all the timeouts provided always select the largest

[Jim Blandy]

I've often thought that what would be coolest would be for gdb_expect
to accept a factor to be applied to the ambient timeout, rather than a
new absolute number of seconds.  After all, the only thing a specific
use of gdb_expect knows is that the response it's waiting for will
take longer (or shorter?) than usual to arrive; it doesn't know
anything about the absolute performance of the system under test.

Re: testsuite: Of all the timeouts provided always select the largest

[Daniel Jacobowitz]

On Tue, Feb 26, 2008 at 02:35:55PM +0000, Maciej W. Rozycki wrote:
> 2008-02-25  Maciej W. Rozycki  <macro@mips.com>
> 
> 	* lib/gdb.exp (gdb_expect): Of all the timeouts provided always
> 	select the largest.
> 
>  OK to apply?

OK.

-- 
Daniel Jacobowitz
CodeSourcery

Re: testsuite: Of all the timeouts provided always select the largest

[Maciej W. Rozycki]

On Tue, 26 Feb 2008, Jim Blandy wrote:

> I've often thought that what would be coolest would be for gdb_expect
> to accept a factor to be applied to the ambient timeout, rather than a
> new absolute number of seconds.  After all, the only thing a specific
> use of gdb_expect knows is that the response it's waiting for will
> take longer (or shorter?) than usual to arrive; it doesn't know
> anything about the absolute performance of the system under test.

 All the callers would have to be updated and unfortunately the dependency 
may not necessarily be linear.  For example for a fast target accessed 
through a slow port the factor for transferring data through, such as 
downloading a program image, would have to be higher than for a slow 
target accessed through a fast port.  Contrariwise the factor applied to 
actions involving execution on the respective systems.  In general I do 
agree it is a good idea, but that is a lot of work that best be well 
thought before commencing.

  Maciej

Re: testsuite: Of all the timeouts provided always select the largest

[Jim Blandy]

On Tue, Feb 26, 2008 at 8:09 AM, Maciej W. Rozycki <macro@mips.com> wrote:
>   All the callers would have to be updated and unfortunately the dependency
>  may not necessarily be linear.  For example for a fast target accessed
>  through a slow port the factor for transferring data through, such as
>  downloading a program image, would have to be higher than for a slow
>  target accessed through a fast port.  Contrariwise the factor applied to
>  actions involving execution on the respective systems.  In general I do
>  agree it is a good idea, but that is a lot of work that best be well
>  thought before commencing.

All true, except for the "all callers" part, I think: couldn't
gdb_expect recognize a '-timeout-factor' flag as its first argument?
Then we could update call sites as we came across them.

It's certainly not linear, but linear is better than constant, surely.
 For the sake of setting timeouts, we only need an upper bound.

Re: testsuite: Of all the timeouts provided always select the largest

[Maciej W. Rozycki]

On Tue, 26 Feb 2008, Jim Blandy wrote:

> >   All the callers would have to be updated and unfortunately the dependency
> >  may not necessarily be linear.  For example for a fast target accessed
> >  through a slow port the factor for transferring data through, such as
> >  downloading a program image, would have to be higher than for a slow
> >  target accessed through a fast port.  Contrariwise the factor applied to
> >  actions involving execution on the respective systems.  In general I do
> >  agree it is a good idea, but that is a lot of work that best be well
> >  thought before commencing.
> 
> All true, except for the "all callers" part, I think: couldn't
> gdb_expect recognize a '-timeout-factor' flag as its first argument?
> Then we could update call sites as we came across them.
> 
> It's certainly not linear, but linear is better than constant, surely.
>  For the sake of setting timeouts, we only need an upper bound.

 Hmm, I have given it a little bit more thought and I believe it may be 
reasonable for boards to provide their own factors for classes of 
operations, let's say two for a start, corresponding to tranfer and 
execution latencies as noted above.  Then each of calling sites might use 
one of them according to operation performed, together, possibly, with a 
factor specific to the requested operation which the class factor would be 
multiplied by.  I am not sure if that is not too complicated, but it would 
better match the reality.  All the factors might default to 1 for 
simplicity as could call sites with no class annotation (your flag might 
become '-timeout-class').

 Any thoughts?

  Maciej

Re: testsuite: Of all the timeouts provided always select the largest

[Daniel Jacobowitz]

On Wed, Feb 27, 2008 at 03:38:54PM +0000, Maciej W. Rozycki wrote:
>  Hmm, I have given it a little bit more thought and I believe it may be 
> reasonable for boards to provide their own factors for classes of 
> operations, let's say two for a start, corresponding to tranfer and 
> execution latencies as noted above.  Then each of calling sites might use 
> one of them according to operation performed, together, possibly, with a 
> factor specific to the requested operation which the class factor would be 
> multiplied by.  I am not sure if that is not too complicated, but it would 
> better match the reality.  All the factors might default to 1 for 
> simplicity as could call sites with no class annotation (your flag might 
> become '-timeout-class').
> 
>  Any thoughts?

Yes - do you think this extra complexity is really useful?  I'm dubious.

-- 
Daniel Jacobowitz
CodeSourcery