Menu
▾
▴
#6 Better Obj-aware screen distances (Tcl Patch #102471)
Sorry, I have overlooked something in
the accepted Tcl Patch #102471 I submitted.
The invariant of the old behaviour was, that every
MMRep had a valid string representation, therefor
updateStringProc was not needed. This is with my
patch not the case any more. You can test this with
> tclsh8.4
load libtk8.4.so
canvas .c
pack .c
set qx [expr {1.+1.}]
# qx has type double and no string representation
.c scale all $qx 0 1. 1.
# qx has now type MMRep and no string representation
puts "qqq: $qx"
result is:
UpdateStringProc should not be invoked for type mm
Aborted
Therefor we need the updateStringProc.
Second there were build problems under Windows, because
of the use of "&tclDoubleObj"
Here is the new patch. Which, hopefully, is right
now. It is against the original tk8.4a2 source.
Peter
Discussion
None
The direct access through the typePtr of the Tcl_Obj is a bit icky. What about querying Tcl for the pointer to the "double" type structure and storing this in a Tk global ? Fast comparison without having to use 'tclDoubleType' directly. We simply get it through the approved API and cache that.
You mean something like
... SetMMFromAny( ... )
{
static Tcl_ObjTyp *doubleObj(
You mean something like
... SetMMFromAny( ... )
{
static Tcl_ObjTyp *doubleObj = Tcl_GetObjType("double");
...
if( objPrt == doubleObj )
...
that's definitely better!
Should "*doubleObj" be function local, file scope or global?
I don't want to make it global, because I want to have I
small patch, which is hopefully accepted faster.
But if I get a hint, what would be accepted, I make
a new patch.
After a short email discussion with Andreas Kupries:
here is the revised patch, which only uses the
tcl_Obj-interface.
A short benchmark shows, that the difference in
execution time is low. So better use the clean
interface.
Hmm. It is probably safe to cache the value of Tcl_GetObjType("double") in a local variable; I don't anticipate it changing over the life of any program.
Has anyone given any thought to what happens if I pass in an integer object instead?
Some of the comments talk about caching of the object
type pointer, but the patch #103327 I see registered
doesn't do any caching. Am I seeing an outdated
version of the patch?
Anyhow, the semantics for Tcl_RegisterObjType(typePtr)
do not support the caching of object types by extensions.
From Tcl_RegisterObjType(3):
...If the type table already containes a type with
the same name as in typePtr, it is replaced with the
new type. ...
If we wanted the Tcl core to support caching of object
types by extensions, we would need to change
Tcl_RegisterObjType() so that it reports an error
(panics?) on the second attempt to register an object
type under the same name.
As is, Tcl_RegisterObjType lets extensions replace
object types with customized replacements. (Though
in that case the extension has the responsibility of
providing a compatible replacement -- much like an
extension which replaces one of Tcl's built-in
commands has the responsibility to provide a
compatible replacement.)
Looks like the choice is between letting extensions
cache object type pointers (efficiency) and letting
extensions provide customized versions of built-in
object types (extensibility) and that choice has
already been made in favor of extensibility.
So, the patch looks fine to me now, but the caching
idea won't fly.
> Am I seeing an outdated version of the patch?
No. I wanted the patch to be as small and as clean
as possible, to be accepted.
The cached variant (in C not C++!):
[...]
static Tcl_ObjTyp *doubleObj = NULL;
if( doubleObj == NULL )
doubleObj = Tcl_GetObjType("double");
[...]
Is a bit ugly and doesn't buy us much speed enhancement.
So I submitted the "clean" variant.
If we do caching, we should probably do it in
tk 9.0 and tk_init().
> Has anyone given any thought to what happens if I pass
> in an integer object instead?
It is converted to a string and then to a double.
One could do another if-branche, but I think for most
application where speed is important, the coordiantes
are the result of some calculation (e.g. expr) and therefor
double. As I said, I wanted the patch to be as small
as possible.
There are several ways that screen distances might be created. While one is undoubtedly as the result of an [expr]ession evaluation, another which certainly happens quite a bit in my own code is by direct use of an integer interator used in a [for] loop, and your patch would make that case shimmer most horribly. Adding in support for directly accessing an integer will not expand the size of the patch that much, and will make a substantial difference in that particular area.
I've assigned this to myself to apply next time I reach my devel box.
Logged In: YES
user_id=72656
It is actually valuable to cache the results, as there are
mutex locks that occur around the Tcl_GetObjType. As to
the ability to change those with another
Tcl_RegisterObjType - it seems like an interesting design
idea that never was reconciled in code. There are numerous
uses of cached object types in Tcl. I have modified the
patch and uploaded it. It also includes a check for int
types, because I found enough code that would hit it, as
well as a test case that exposes the bug in the original
version (with no UpdateStringOfMM proc).
Revised patch with extra test cases
Logged In: YES
user_id=72656
Further testing shows that you can't do the same for Ints,
because the UpdateStringOfMM doesn't return it to a valid
int. This happens when the objPtr is an int without a
valid string rep. Added a test case to show this, and one
more line to correct it. All in all though, when adding an
example in tclbench, I'm not seeing a lot of speedup. At
~30 item creations, it's insignificant. From 200+ you get
between 10 and 15%. Was there an example with greater
performance gain?
Logged In: YES
user_id=72656
Further testing shows that you can't do the same for Ints,
because the UpdateStringOfMM doesn't return it to a valid
int. This happens when the objPtr is an int without a
valid string rep. Added a test case to show this, and one
more line to correct it. All in all though, when adding an
example in tclbench, I'm not seeing a lot of speedup. At
~30 item creations, it's insignificant. From 200+ you get
between 10 and 15%. Was there an example with greater
performance gain?
Revised patch with extra test cases
Logged In: YES
user_id=102364
I benchmarked my hole application, where the coordinates are
calculated by a C++-package. I got:
# 2 * Pentium III 500 MHz 128 MByte, XFree 4.0
# Linux 2.2.17, tcl 8.4a2
#draw: 173001 microseconds per iteration
# 2 * Pentium III 500 MHz 128 MByte, XFree 4.0
# Linux 2.2.17, tcl 8.4a2 gepatcht: SetMMFromAny
#draw: 80230 microseconds per iteration
The effect should be reproducible by:
canvas .c
pack .c
update idletasks
set height [winfo reqheight .c]
set width [winfo reqwidth .c]
set coords {}
set noCoords 1024
for { set x 0 } { $x < $noCoords } { incr x } {
lappend coords $x
lappend coords \
[expr {$height/3*sin(4*3.14*$x/$width)+$height/2}]
}
.c create line $coords
time { .c coords $coords; update idletasks } 10
But I have formated my harddisk one month ago, and therefor
no 8.4a2 available here at the moment :-(.
Logged In: YES
user_id=72656
added to 8.4a2 cvs.
Logged In: YES
user_id=72656
uploaded final patch.