From: SourceForge.net <no...@so...> - 2005-05-19 15:49:47
|
Bugs item #1202178, was opened at 2005-05-14 22:43 Message generated for change (Comment added) made by dgp You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=110894&aid=1202178&group_id=10894 Category: 18. Commands M-Z Group: development: 8.5a3 Status: Open Resolution: None Priority: 6 Submitted By: Daniel A. Steffen (das) Assigned to: Jeffrey Hobbs (hobbs) Summary: Change [time] to return a float for count>1 Initial Comment: 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... ---------------------------------------------------------------------- >Comment By: Don Porter (dgp) Date: 2005-05-19 11:49 Message: Logged In: YES user_id=80530 my objection was based on a misunderstanding. The patch is fine with me. ---------------------------------------------------------------------- Comment By: Donal K. Fellows (dkf) Date: 2005-05-19 09:07 Message: 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... ;^) ---------------------------------------------------------------------- Comment By: Daniel A. Steffen (das) Date: 2005-05-19 08:47 Message: 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. ---------------------------------------------------------------------- Comment By: Donal K. Fellows (dkf) Date: 2005-05-16 04:01 Message: 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. ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2005-05-15 22:34 Message: 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? ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=110894&aid=1202178&group_id=10894 |