Tcl Source Code

File Added - 49359: tls2.zip

File Deleted - 49359: 



File Added - 49534: tls4.zip

Logged In: YES 
user_id=113501

Updated to optionally (via TCL_NO_TLS) remove calls to 
ALL win32 TLS functions, including in the AllocCache 
related functions.  Several leaks fixed (from HEAD, not 
my previous patch).

File Deleted - 49534: 



File Added - 49541: tls5.zip

Logged In: YES 
user_id=113501

A few minor changes pursuant to discussions with jyl.

Logged In: YES 
user_id=99768

This is starting to look good!

Just a few comments before it gets committed:

(1) I'd be a bit more comfortable if STORAGE_CACHE_SLOTS
    were a prime number.  Consider that on at least one
    version of Windows, the thread ID is always a multiple
    of four; this means that the position in the cache will
    also be a multiple of four, giving us only 25 effective
    slots instead of 100. Could we perhaps make it 97
    instead?  Another point is that we might want to
    consider making it a variable instead of a constant. The
    advantage to this is that we could compile tcltest with
    a very, very small value (2 perhaps) and thus make sure
    of test coverage in the cache miss logic.

(2) Several new Tcl* functions have been added. It's
    generally considered good practice to put them in
    tclInt.decls -- but opinions vary about the use of the
    internal stubs table, so I'll be willing to be convinced
    otherwise.

(3) Several functions in the new code lack prologue
    comments.  It would be nice if these could get written
    before committing.

(4) The management scheme that we've arrived at for thread
    specific data is actually platform neutral. Given that
    we've already encountered one platform with an awkward
    hard limit on thread specific data blocks, I'd not be
    willing to lay long odds that there is no other. I
    wonder if we should move the platform-neutral TSD
    management logic to generic/tclThread.c instead of
    calling it Windows code.

(5) What's the difference between TCL_NO_TLS and
    TCL_THREAD_ALLOC_NO_TLS? Do we need both flags?
    (Actually, I wonder, given how badly Windows does this,
    whether we need either, or whether we just want to use
    our own system exclusively.)

Nice job - I'm tempted to say that we should just use it
for ALL our TSD management, irrespective of platform.

Logged In: YES 
user_id=113501

Let me address your comments one by one:

(1) I think you are totally right about this. It should be a 
variable that is defaulted to a prime number.

(2) I believe they should go into tclInt.decls, it slipped 
my mind at the time.

(3) The functions in question are most likely the alloc 
cache functions, which strangely never had any 
prologue comments. I believe I added one function to 
this set of functions and I figured "why comment?" if 
there were no other comments regarding these 
particular functions.  All the other functions I added 
should have prologue comments.

(4) I agree about this too.  However, some work will 
need to be done to make this cross-platform.  I have 
proposed to JeffH that we create a new file for this 
subsystem and call it "generic/tclThreadStorage.c".

(5) The difference between these flags is very important, 
actually.  The TCL_NO_TLS flag prevents TLS from being 
used for "high level" thread local storage operations 
(TclpThreadDataKeyInit/TclpThreadDataKeyGet/TclpThre
adDataKeySet).  The TCL_THREAD_ALLOC_NO_TLS 
forces the threaded memory allocator (which is MUCH 
more low level) to NOT use TLS.  These flags should 
definitely stay separate.

Did you notice that I made some changes to tclHash.c 
(the ability to use TclpSysAlloc/TclpSysFree instead of 
ckalloc/ckfree for the non-entry parts of the table that 
need to be dynamically allocated) and tcl.h (a new hash 
key type flag)?  It is my hope that these changes will 
NOT require a TIP, because without them, the threaded 
memory allocator CANNOT use hash tables (due to 
circular dependency issues).

In conclusion, I don't think this patch should be 
committed as is.  I'm still working on some ideas and I 
think that we should make the "new" code cross-
platform. Oh, and I think we should leave in support for 
the "legacy" thread local storage stuff that's in there 
now.

Logged In: YES 
user_id=113501

I've made some progress on making this patch 
for "generic".  However, I have a couple questions.  I 
need a way to initialize/finalizing a mutex WITHOUT 
having ckalloc/ckfree being called.  I still need 
InitializeCriticalSection/DeleteCriticalSection (or their 
Unix/Mac equivalents) to be called for the mutex.  The 
threaded memory allocator currently uses 
TclpNewAllocMutex (a very puzzling function) and then 
never finalizes the mutex.  The TclpNewAllocMutex is 
poorly documented and in the case of TCL_THREADS 
without USE_THREAD_ALLOC, unavailable.  I feel there 
should be new platform specific functions added to the 
tcl{Unix|Win|Mac}Thrd.c files, such as 
TclpInitMutex/TclpDeleteMutex.  For platforms that do 
not require these steps, the functions can be empty.

Logged In: YES 
user_id=113501

Once I get this patch completed, I would really like to 
benchmark and stress test it.  I haven't been able to get 
tclBench to cooperate as of yet.  Any help on this is 
appreciated.

Logged In: YES 
user_id=72656

tclbench has a little support for threaded tests, but it may not 
be quite what you want.  What isn't working?

Logged In: YES 
user_id=113501

Further research reveals this problem is much worse than I 
originally thought.  Running even the most basic Tcl and/or Tk 
scripts requires a LOT of TLS indexes.  JUST to start wish 
requires 26 TLS indexes.  To start wish and then run a script 
appears to require at least 33 to 40 TLS indexes. The more 
subsystems of Tcl or Tk that are required, the more TLS 
indexes are required.

Logged In: YES 
user_id=80530


TIP 138 is Accepted.  Is this
the implementation patch?

Logged In: YES 
user_id=113501

Not exactly. This patch includes more than just the TIP #138 
changes. When I get back home, I will create a new patch for 
HEAD with just the TIP #138 changes.

Logged In: YES 
user_id=79902

This is your reminder call.  I want to get as many Accepted
TIPs as possible to be Final...

Logged In: YES 
user_id=113501

This is not the TIP #138 patch, although this does 
incorporate the changes talked about in TIP #138. I will 
create a pure TIP #138 patch as soon as possible, however, I 
will need help making the doc changes.

Logged In: YES 
user_id=79902

I might be able to help with docs.  No promises though.

(When you attach the patch that is purely the TIP impl,
please change the Group field to TIP Implementation.  Thanks!)

File Added - 67528: 731356.patch

Logged In: YES 
user_id=80530


new patch updated to current HEAD.

File Added - 67530: tip138.patch

Logged In: YES 
user_id=80530


Smaller patch that just
implements TIP 138 (?)

someone please review/confirm.

File Deleted - 67530: 



File Added - 67533: tip138.patch

Logged In: YES 
user_id=80530


Corrected TIP 138 patch
for Unix.

File Deleted - 67533: 



File Added - 67547: tip138.patch

Logged In: YES 
user_id=80530


Updated TIP 138 patch
includes documentation.

Please review, correct, and
when satisfied, apply to HEAD.

Logged In: YES 
user_id=79902

Pretty good.  Docs not quite there and we need something to
force this code to be exercised in 'make test'.

Logged In: YES 
user_id=79902

Implemented.