Tcl Source Code

View Ticket
Login
Ticket UUID: 1202178
Title: Change [time] to return a float for count>1
Type: Bug Version: obsolete: 8.5a3
Submitter: das Created on: 2005-05-15 02:43:34
Subsystem: 18. Commands M-Z Assigned To: das
Priority: 6 Severity:
Status: Closed Last Modified: 2005-05-26 02:27:42
Resolution: Fixed Closed By: hobbs
    Closed on: 2005-05-25 19:27:42
Description:
When [time]ing a script of short duration for many counts, a lot of 
measured precision is lost because [time] only returns an integer 
number of microseconds.

The attached very simple patch changes [time] to return a list with 
a string rep identical to the current result string, except that the 
returned time is a DoubleObj for count>1 (and an IntObj for 
count==1)

For things like tclbench that do [lindex 0] on the return value of 
[time] this transparently avoids string conversion of the 
microseconds value...

I'm not sure if this change would require a TIP? it does change 
the public API of [time] but in a way that seems innocous...
User Comments: hobbs added on 2005-05-26 02:27:42:
Logged In: YES 
user_id=72656

Fixed the cast in 8.4 and head

vincentdarley added on 2005-05-26 00:05:28:
Logged In: YES 
user_id=32170

This change breaks compilation with symbols on Windows:

>nmake -f makefile.vc OPTS=symbols
...
tclCmdMZ.c
..\generic\tclCmdMZ.c(2924) : error C2220: warning treated
as error - no object
file generated
..\generic\tclCmdMZ.c(2924) : warning C4244: 'function' :
conversion from 'double ' to 'int ', possible loss of data
NMAKE : fatal error U1077: 'cl' : return code '0x2'
Stop.

I think a cast is needed in there...

das added on 2005-05-24 11:24:42:
Logged In: YES 
user_id=90580

committed to core-8-4-branch

das added on 2005-05-24 03:31:28:
Logged In: YES 
user_id=90580

committed to HEAD

dgp added on 2005-05-19 22:49:45:
Logged In: YES 
user_id=80530


my objection was based on a misunderstanding.
The patch is fine with me.

dkf added on 2005-05-19 20:07:29:
Logged In: YES 
user_id=79902

I'm just trying to think of things that might possibly go
wrong before rolling such a change into production and
getting howls from people whose code it breaks. Trying to
spot problems before they happen, as opposed to trying to
make your life hard. :^)

Of course, one way round your original problem is to time
your code on a slower computer... ;^)

das added on 2005-05-19 19:47:59:
Logged In: YES 
user_id=90580

Donal, I'm not sure I understand your objection.
For count==1, my point was that the string rep of the list that the 
patched [time] returns is exactly the same as the stringObj currently 
returned (constructed via sprintf).
As long as nobody checks the Tcl_ObjType of the return value (which 
they shouldn't), there is no way to distingush between this list and the 
currently returned string, AFAICT

I have certainly tested tclbench against this, the patch was indeed 
primarily motivated by the desire to get more stable results from tclbench 
for tests with small [time] results. Everything works unchanged except 
that all timing results are now floats

Don, the motivation of using an IntObj for the count==1 case was to 
preserve the current return string format exactly when it is not necessary 
to change it (since the measured time can never be a float for 
count==1). I am certainly happy to always make it a float, this idea may 
indeed be trying to be to smart for its own good...

OTOH, I don't understand why you say the patch returns information 
embodied in the Tcl_ObjType, everything would work just the same if I 
returned just the string rep of the list I construct as a StrinObj... (except 
that there could be truncation due to $tcl_precision)

In any case, I'd even be ok to retain the current sprintf way of 
constructing the result as long as we switch the %f.0 in the format string 
to something with more decimals.

dkf added on 2005-05-16 15:01:42:
Logged In: YES 
user_id=79902

[time] returns a string, not a list or an int. It just
happens to be easy to use [lindex] and [expr] to work with
the output.

The primary consumer of [time] output as numeric data is
probably tclbench; any change should be checked against
what's going on there before committing.

dgp added on 2005-05-16 09:34:22:
Logged In: YES 
user_id=80530


That sounds suspiciously like using
the Tcl_ObjType to embody a value
with meaning beyond its string rep.
We're working on getting away from
that in the few regrettable places
where it's been done.  I think it's a
mistake to make new abuse of
ObjType's like that.

One internals change kbk and
I are considering is to set a
Tcl_ObjType to "double" only
if its string rep is not one that
"LooksLikeInt".  The intent is
to simplify lots of places that
are now forced to check whether
a "double" Tcl_Obj is "pure"
before making use of the cached
double.  Sounds like your trick
would conflict with that.

On the other hand, I don't think I
have any objection to a proposal
to change [time] to return a
double instead of an int.  Rather
than sneak around the issue, can
we just embrace the change?

das added on 2005-05-15 09:43:35:

File Added - 134511: float-time.diff

Attachments: