From c8e7eb6d47e9cd382589a64d04200b61344351eb Mon Sep 17 00:00:00 2001 From: dkf Date: Sat, 29 Nov 2008 12:15:30 +0000 Subject: Improvements to the general readability of the TSD implementation. --- ChangeLog | 54 ++++---- generic/tclThreadStorage.c | 309 +++++++++++++++++++++++++++------------------ 2 files changed, 220 insertions(+), 143 deletions(-) diff --git a/ChangeLog b/ChangeLog index be37c31..9e5f936 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,15 +1,22 @@ +2008-11-29 Donal K. Fellows + + * generic/tclThreadStorage.c: General revisions to make code clearer + and more like the style used in the rest of the core. Includes adding + more comments and explanation of what is going on. + 2008-11-27 Alexandre Ferrieux - * generic/tcl.h: Alternate fix for[Bug 2251175]: missing - * generic/tclCompile.c backslash substitution on expanded literals. - * generic/tclParse.c - * generic/tclTest.c - * tests/parse.test + * generic/tcl.h: Alternate fix for [Bug 2251175]: missing + * generic/tclCompile.c: backslash substitution on expanded literals. + * generic/tclParse.c: + * generic/tclTest.c: + * tests/parse.test: 2008-11-26 Jan Nijtmans * generic/tclIndexObj.c: Eliminate warning: unused variable - * generic/tclTest.c: A few more (harmless) Tcl_SetResult eliminations + * generic/tclTest.c: A few more (harmless) Tcl_SetResult + eliminations. 2008-11-26 Kevin B. Kenny @@ -22,19 +29,20 @@ * generic/tclIndexObj.c: Eliminate 3 calls to Tcl_SetResult, as * generic/tclIO.c examples how it should have been done. - * generic/tclTestObj.c purpose: contribute in the TIP #340 discussion. + * generic/tclTestObj.c purpose: contribute in the TIP #340 + discussion. 2008-11-25 Andreas Kupries * generic/tclIO.c (TclFinalizeIOSubsystem): Applied Alexandre - Ferrieux's patch for [Bug 2270477] to prevent infinite looping - during finalization of channels not bound to interpreters. + Ferrieux's patch for [Bug 2270477] to prevent infinite looping during + finalization of channels not bound to interpreters. 2008-11-25 Jan Nijtmans - * generic/tclTest.c: Don't assume that Tcl_SetResult - sets interp->result, especially not in a dstring test, - in preparation for TIP #340 + * generic/tclTest.c: Don't assume that Tcl_SetResult sets + interp->result, especially not in a dstring test, in preparation for + TIP #340 2008-11-24 Donal K. Fellows @@ -1048,17 +1056,17 @@ 2008-08-14 Daniel Steffen - * generic/tclBasic.c (TclNREvalObjv, Tcl_NRCallObjProc): DTrace probes - * generic/tclProc.c (TclNRInterpProcCore, InterpProcNR2): for NRE. - [Bug 2017160] + * generic/tclBasic.c (TclNREvalObjv, Tcl_NRCallObjProc): + * generic/tclProc.c (TclNRInterpProcCore, InterpProcNR2): + DTrace probes for NRE. [Bug 2017160] * generic/tclBasic.c (TclDTraceInfo): Add two extra arguments to * generic/tclCompile.h: DTrace 'info' probes for tclOO * generic/tclDTrace.d: method & class/object info. - * generic/tclCompile.h: Add support for debug logging of DTrace - * generic/tclBasic.c: 'proc', 'cmd' and 'inst' probes (does - _not_ require a platform with DTrace). + * generic/tclCompile.h: Add support for debug logging of DTrace + * generic/tclBasic.c: 'proc', 'cmd' and 'inst' probes (does _not_ + require a platform with DTrace). * generic/tclCmdIL.c (TclInfoFrame): Check fPtr->line before dereferencing as line info may @@ -1285,11 +1293,11 @@ 2008-08-01 Don Porter - * generic/tclBasic.c: Revised timing of the CmdFrame stack management - * tests/info.test: in TclEvalEx so that the CmdFrame will still - be on the stack at the time Tcl_LogCommandInfo is called to append - another level of -errorinfo information. Sets the stage to add file - and line data to the stack trace. Added test to check that [info + * generic/tclBasic.c: Revised timing of the CmdFrame stack + * tests/info.test: management in TclEvalEx so that the CmdFrame + will still be on the stack at the time Tcl_LogCommandInfo is called to + append another level of -errorinfo information. Sets the stage to add + file and line data to the stack trace. Added test to check that [info frame] functioning remains unchanged by the revision. 2008-07-31 Miguel Sofer diff --git a/generic/tclThreadStorage.c b/generic/tclThreadStorage.c index d0fdeec..71fa1e9 100644 --- a/generic/tclThreadStorage.c +++ b/generic/tclThreadStorage.c @@ -1,78 +1,117 @@ /* * tclThreadStorage.c -- * - * This file implements platform independent thread storage operations. + * This file implements platform independent thread storage operations to + * work around system limits on the number of thread-specific variables. * * Copyright (c) 2003-2004 by Joe Mistachkin * Copyright (c) 2008 by George Peter Staplin * - * The primary idea is that we create one platform-specific TSD slot, and - * use it for storing a table pointer. - * - * Each Tcl_ThreadDataKey has an offset into the table of TSD values. - * - * We don't use more than 1 platform-specific TSD slot, because there is - * a hard limit on the number of TSD slots. - * - * Valid key offsets are > 0. 0 is for the initialized Tcl_ThreadDataKey. - * * See the file "license.terms" for information on usage and redistribution of * this file, and for a DISCLAIMER OF ALL WARRANTIES. * - * RCS: @(#) $Id: tclThreadStorage.c,v 1.16 2008/05/09 04:58:54 georgeps Exp $ + * RCS: @(#) $Id: tclThreadStorage.c,v 1.17 2008/11/29 12:15:30 dkf Exp $ */ #include "tclInt.h" + +#ifdef TCL_THREADS #include -#if defined(TCL_THREADS) +/* + * IMPLEMENTATION NOTES: + * + * The primary idea is that we create one platform-specific TSD slot, and use + * it for storing a table pointer. Each Tcl_ThreadDataKey has an offset into + * the table of TSD values. We don't use more than 1 platform-specific TSD + * slot, because there is a hard limit on the number of TSD slots. Valid key + * offsets are greater than 0; 0 is for the initialized Tcl_ThreadDataKey. + */ + +/* + * The master collection of information about TSDs. This is shared across the + * whole process, and includes the mutex used to protect it. + */ + +static struct TSDMaster { + void *key; /* Key into the system TSD structure. The + * collection of Tcl TSD values for a + * particular thread will hang off the + * back-end of this. */ + sig_atomic_t counter; /* The number of different Tcl TSDs used + * across *all* threads. This is a strictly + * increasing value. */ + Tcl_Mutex mutex; /* Protection for the rest of this structure, + * which holds per-process data. */ +} tsdMaster = { NULL, 0 }; -static void *tclTsdKey = NULL; -static Tcl_Mutex tclTsdMutex; -static sig_atomic_t tclTsdCounter = 0; +/* + * The type of the data held per thread in a system TSD. + */ typedef struct TSDTable { - sig_atomic_t allocated; - void **table; + ClientData *tablePtr; /* The table of Tcl TSDs. */ + sig_atomic_t allocated; /* The size of the table in the current + * thread. */ } TSDTable; -typedef union { - void *ptr; +/* + * The actual type of Tcl_ThreadDataKey. + */ + +typedef union TSDUnion { volatile sig_atomic_t offset; + /* The type is really an offset into the + * thread-local table of TSDs, which is this + * field. */ + void *ptr; /* For alignment purposes only. Not actually + * accessed through this. */ } TSDUnion; -static TSDTable *TSDTableCreate(void); -static void TSDTableDelete(TSDTable *t); -static void TSDTableGrow(TSDTable *t, sig_atomic_t atLeast); +/* + * Forward declarations of functions in this file. + */ +static TSDTable * TSDTableCreate(void); +static void TSDTableDelete(TSDTable *tsdTablePtr); +static void TSDTableGrow(TSDTable *tsdTablePtr, + sig_atomic_t atLeast); +/* + * Allocator and deallocator for a TSDTable structure. + */ + static TSDTable * -TSDTableCreate(void) { - TSDTable *t; +TSDTableCreate(void) +{ + TSDTable *tsdTablePtr; sig_atomic_t i; - t = TclpSysAlloc(sizeof *t, 0); - if (NULL == t) { + tsdTablePtr = TclpSysAlloc(sizeof(TSDTable), 0); + if (tsdTablePtr == NULL) { Tcl_Panic("unable to allocate TSDTable"); } - t->allocated = 8; - t->table = TclpSysAlloc(sizeof (*(t->table)) * t->allocated, 0); - if (NULL == t->table) { + tsdTablePtr->allocated = 8; + tsdTablePtr->tablePtr = + TclpSysAlloc(sizeof(void *) * tsdTablePtr->allocated, 0); + if (tsdTablePtr->tablePtr == NULL) { Tcl_Panic("unable to allocate TSDTable"); } - for (i = 0; i < t->allocated; ++i) { - t->table[i] = NULL; + for (i = 0; i < tsdTablePtr->allocated; ++i) { + tsdTablePtr->tablePtr[i] = NULL; } - return t; + return tsdTablePtr; } - + static void -TSDTableDelete(TSDTable *t) { - TclpSysFree(t->table); - TclpSysFree(t); +TSDTableDelete( + TSDTable *tsdTablePtr) +{ + TclpSysFree(tsdTablePtr->tablePtr); + TclpSysFree(tsdTablePtr); } /* @@ -80,34 +119,43 @@ TSDTableDelete(TSDTable *t) { * * TSDTableGrow -- * - * This procedure makes the passed TSDTable grow to fit the atLeast value. + * This procedure makes the passed TSDTable grow to fit the atLeast + * value. * - * Side effects: The table is enlarged. + * Results: + * None. + * + * Side effects: + * The table is enlarged. * *---------------------------------------------------------------------- */ + static void -TSDTableGrow(TSDTable *t, sig_atomic_t atLeast) { - sig_atomic_t newAllocated = t->allocated * 2; - void **newTablePtr; +TSDTableGrow( + TSDTable *tsdTablePtr, + sig_atomic_t atLeast) +{ + sig_atomic_t newAllocated = tsdTablePtr->allocated * 2; + ClientData *newTablePtr; sig_atomic_t i; if (newAllocated <= atLeast) { newAllocated = atLeast + 10; } - newTablePtr = TclpSysRealloc(t->table, sizeof (*newTablePtr) * newAllocated); - - if (NULL == newTablePtr) { + newTablePtr = TclpSysRealloc(tsdTablePtr->tablePtr, + sizeof(ClientData) * newAllocated); + if (newTablePtr == NULL) { Tcl_Panic("unable to reallocate TSDTable"); } - - for (i = t->allocated; i < newAllocated; ++i) { + + for (i = tsdTablePtr->allocated; i < newAllocated; ++i) { newTablePtr[i] = NULL; } - - t->allocated = newAllocated; - t->table = newTablePtr; + + tsdTablePtr->allocated = newAllocated; + tsdTablePtr->tablePtr = newTablePtr; } /* @@ -117,25 +165,28 @@ TSDTableGrow(TSDTable *t, sig_atomic_t atLeast) { * * This procedure gets the value associated with the passed key. * - * Results: A pointer value associated with the Tcl_ThreadDataKey or NULL. + * Results: + * A pointer value associated with the Tcl_ThreadDataKey or NULL. + * + * Side effects: + * None. * *---------------------------------------------------------------------- */ + void * -TclThreadStorageKeyGet(Tcl_ThreadDataKey *dataKeyPtr) { - TSDTable *t = TclpThreadGetMasterTSD(tclTsdKey); - void *resultPtr = NULL; - TSDUnion *keyPtr = (TSDUnion *)dataKeyPtr; +TclThreadStorageKeyGet( + Tcl_ThreadDataKey *dataKeyPtr) +{ + TSDTable *tsdTablePtr = TclpThreadGetMasterTSD(tsdMaster.key); + ClientData resultPtr = NULL; + TSDUnion *keyPtr = (TSDUnion *) dataKeyPtr; sig_atomic_t offset = keyPtr->offset; - - if (t == NULL) { - return NULL; - } - if (offset && offset > 0 && offset < t->allocated) { - resultPtr = t->table[offset]; + if ((tsdTablePtr != NULL) && (offset > 0) + && (offset < tsdTablePtr->allocated)) { + resultPtr = tsdTablePtr->tablePtr[offset]; } - return resultPtr; } @@ -144,53 +195,62 @@ TclThreadStorageKeyGet(Tcl_ThreadDataKey *dataKeyPtr) { * * TclThreadStorageKeySet -- * - * This procedure set an association of value with the key passed. - * The associated value may be retrieved with TclThreadDataKeyGet(). + * This procedure set an association of value with the key passed. The + * associated value may be retrieved with TclThreadDataKeyGet(). * - * Side effects: The thread-specific table may be created or reallocated. + * Results: + * None. + * + * Side effects: + * The thread-specific table may be created or reallocated. * *---------------------------------------------------------------------- */ void -TclThreadStorageKeySet(Tcl_ThreadDataKey *dataKeyPtr, void *value) { - TSDTable *t = TclpThreadGetMasterTSD(tclTsdKey); - TSDUnion *keyPtr = (TSDUnion *)dataKeyPtr; +TclThreadStorageKeySet( + Tcl_ThreadDataKey *dataKeyPtr, + void *value) +{ + TSDTable *tsdTablePtr = TclpThreadGetMasterTSD(tsdMaster.key); + TSDUnion *keyPtr = (TSDUnion *) dataKeyPtr; - if (NULL == t) { - t = TSDTableCreate(); - TclpThreadSetMasterTSD(tclTsdKey, t); + if (tsdTablePtr == NULL) { + tsdTablePtr = TSDTableCreate(); + TclpThreadSetMasterTSD(tsdMaster.key, tsdTablePtr); } - Tcl_MutexLock(&tclTsdMutex); + /* + * Get the lock while we check if this TSD is new or not. Note that this + * is the only place where Tcl_ThreadDataKey values are set. + */ - if (0 == keyPtr->offset) { + Tcl_MutexLock(&tsdMaster.mutex); + if (keyPtr->offset == 0) { /* - * The Tcl_ThreadDataKey hasn't been used yet. + * The Tcl_ThreadDataKey hasn't been used yet. Make a new one. */ - ++tclTsdCounter; - - if (tclTsdCounter >= t->allocated) { - TSDTableGrow(t, tclTsdCounter); - } - - keyPtr->offset = tclTsdCounter; - t->table[tclTsdCounter] = value; - } else { + keyPtr->offset = ++tsdMaster.counter; + } + Tcl_MutexUnlock(&tsdMaster.mutex); - if (keyPtr->offset >= t->allocated) { - /* - * This is the first time this Tcl_ThreadDataKey has been - * used with the current thread. - */ - TSDTableGrow(t, keyPtr->offset); - } + /* + * Check if this is the first time this Tcl_ThreadDataKey has been used + * with the current thread. Note that we don't need to hold a lock when + * doing this, as we are *definitely* the only point accessing this + * tsdTablePtr right now; it's thread-local. + */ - t->table[keyPtr->offset] = value; + if (keyPtr->offset >= tsdTablePtr->allocated) { + TSDTableGrow(tsdTablePtr, keyPtr->offset); } - Tcl_MutexUnlock(&tclTsdMutex); + /* + * Set the value in the Tcl thread-local variable. + */ + + tsdTablePtr->tablePtr[keyPtr->offset] = value; } /* @@ -198,23 +258,26 @@ TclThreadStorageKeySet(Tcl_ThreadDataKey *dataKeyPtr, void *value) { * * TclFinalizeThreadDataThread -- * - * This procedure finalizes the data for a single thread. - * + * This procedure finalizes the data for a single thread. + * + * Results: + * None. + * + * Side effects: + * The TSDTable is deleted/freed. * - * Side effects: The TSDTable is deleted/freed. *---------------------------------------------------------------------- */ + void -TclFinalizeThreadDataThread(void) { - TSDTable *t = TclpThreadGetMasterTSD(tclTsdKey); +TclFinalizeThreadDataThread(void) +{ + TSDTable *tsdTablePtr = TclpThreadGetMasterTSD(tsdMaster.key); - if (NULL == t) { - return; + if (tsdTablePtr != NULL) { + TSDTableDelete(tsdTablePtr); + TclpThreadSetMasterTSD(tsdMaster.key, NULL); } - - TSDTableDelete(t); - - TclpThreadSetMasterTSD(tclTsdKey, NULL); } /* @@ -222,14 +285,22 @@ TclFinalizeThreadDataThread(void) { * * TclInitializeThreadStorage -- * - * This procedure initializes the TSD subsystem with per-platform - * code. This should be called before any Tcl threads are created. + * This procedure initializes the TSD subsystem with per-platform code. + * This should be called before any Tcl threads are created. + * + * Results: + * None. + * + * Side effects: + * Allocates a system TSD. * *---------------------------------------------------------------------- */ + void -TclInitThreadStorage(void) { - tclTsdKey = TclpThreadCreateKey(); +TclInitThreadStorage(void) +{ + tsdMaster.key = TclpThreadCreateKey(); } /* @@ -237,27 +308,26 @@ TclInitThreadStorage(void) { * * TclFinalizeThreadStorage -- * - * This procedure cleans up the thread storage data key for all - * threads. - * - * IMPORTANT: All Tcl threads must be finalized before calling this! + * This procedure cleans up the thread storage data key for all threads. + * IMPORTANT: All Tcl threads must be finalized before calling this! * * Results: - * None. + * None. * * Side effects: - * Releases the thread data key. + * Releases the thread data key. * *---------------------------------------------------------------------- */ + void -TclFinalizeThreadStorage(void) { - TclpThreadDeleteKey(tclTsdKey); - tclTsdKey = NULL; +TclFinalizeThreadStorage(void) +{ + TclpThreadDeleteKey(tsdMaster.key); + tsdMaster.key = NULL; } -#else /* !defined(TCL_THREADS) */ - +#else /* !TCL_THREADS */ /* * Stub functions for non-threaded builds */ @@ -276,8 +346,7 @@ void TclFinalizeThreadStorage(void) { } - -#endif /* defined(TCL_THREADS) && defined(USE_THREAD_STORAGE) */ +#endif /* TCL_THREADS */ /* * Local Variables: -- cgit v0.12