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:
- float-time.diff [download] added by das on 2005-05-15 09:43:35. [details]