Tcl Source Code

So far, the CONCAT1 instruction goes through the string reps of all objects, even when all are byte arrays. This compromises performance in heavy byte-juggling code since it means converting back and forth to UTF-8. This is odd since [string range] et al. do have an optimization for the byte array case.

The attached patch (against 8.5.1) fixes that, by first looping through all to-be-concatenated objects to check whether they are all byte arrays (or empty strings).
In that case, all operations are done at byte array level, while keeping the very same logic regarding in-place operation if possible (unshared first object).

Moreover, the inclusion of the empty string case allows for the "K-free K" idiom $x[set x {}], which in the end allows the following in-place byte array growing operation:

      set x $x[set x {}]$y

Test suite OK, and stepped through gdb to check most branches.

Logged In: YES 
user_id=496139
Originator: YES

Test suite updated with a relative speed test (detect overhead of conversions).

Logged In: YES 
user_id=496139
Originator: YES

Committed to HEAD.
Will now prepare test and tclbench cases.

File Added - 283079: binconcat2.patch

Logged In: YES 
user_id=496139
Originator: YES

Thanks for the green light.

The just attached update fixes the warnings.
FWIW, these warnings don't appear with msys/mingw, based on gcc 3.4.2.
Maybe you compiled on unix with a more modern gcc ?

Will now do my fist solo flight with non-anon cvs :-)
File Added: binconcat2.patch

Logged In: YES 
user_id=148712
Originator: NO

This looks OK for commit to HEAD after fixing those warnings; Alex, please do. 

It would be nice to insure that the testsuite covers any possible gotcha, eg different combinations of concat'ing bytearrays, empties and other things at first/other places.

It'd be also nice to check if tclbench will expose the improvement, and to write a small bench to include there if not.

Logged In: YES 
user_id=80530
Originator: NO


when applied to HEAD, the compiler has
some gripes about this patch:

/home/dgp/cvs/tcl/unix/../generic/tclExecute.c: In function ‘TclExecuteByteCode’:
/home/dgp/cvs/tcl/unix/../generic/tclExecute.c:2221: warning: pointer targets in assignment differ in signedness
/home/dgp/cvs/tcl/unix/../generic/tclExecute.c:2224: warning: pointer targets in assignment differ in signedness
/home/dgp/cvs/tcl/unix/../generic/tclExecute.c:2230: warning: pointer targets in assignment differ in signedness
/home/dgp/cvs/tcl/unix/../generic/tclExecute.c:2244: warning: pointer targets in assignment differ in signedness

Logged In: YES 
user_id=496139
Originator: YES

Miguel, reassigning this to you on Donal's advice.
Hope you don't mind :-}

File Added - 276219: binconcat.patch