Menu
▾
▴
[Tcl-bugs] [ tcl-Patches-731356 ] Win32 non-TLS thread storage patch...
|
From: SourceForge.net <no...@so...> - 2003-11-14 22:28:00
|
Patches item #731356, was opened at 2003-05-02 10:03 Message generated for change (Comment added) made by dgp You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=310894&aid=731356&group_id=10894 Category: 47. Threading Group: None Status: Open Resolution: Remind Priority: 9 Submitted By: Nobody/Anonymous (nobody) >Assigned to: Donal K. Fellows (dkf) Summary: Win32 non-TLS thread storage patch... Initial Comment: This patch largely eliminates the need to use the Win32 TLS functions, which have serious limitations on Windows 95 and NT 4.0. This patch does NOT modify the "AllocCache" related functions to not use Win32 TLS functions. However, it could be extended to do so. ---------------------------------------------------------------------- >Comment By: Don Porter (dgp) Date: 2003-11-14 17:28 Message: Logged In: YES user_id=80530 Updated TIP 138 patch includes documentation. Please review, correct, and when satisfied, apply to HEAD. ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2003-11-14 16:52 Message: Logged In: YES user_id=80530 Corrected TIP 138 patch for Unix. ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2003-11-14 16:21 Message: Logged In: YES user_id=80530 Smaller patch that just implements TIP 138 (?) someone please review/confirm. ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2003-11-14 16:15 Message: Logged In: YES user_id=80530 new patch updated to current HEAD. ---------------------------------------------------------------------- Comment By: Donal K. Fellows (dkf) Date: 2003-11-11 13:19 Message: 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!) ---------------------------------------------------------------------- Comment By: Joe Mistachkin (mistachkin) Date: 2003-11-11 03:13 Message: 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. ---------------------------------------------------------------------- Comment By: Donal K. Fellows (dkf) Date: 2003-10-29 05:17 Message: Logged In: YES user_id=79902 This is your reminder call. I want to get as many Accepted TIPs as possible to be Final... ---------------------------------------------------------------------- Comment By: Joe Mistachkin (mistachkin) Date: 2003-08-25 18:16 Message: 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. ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2003-08-25 14:30 Message: Logged In: YES user_id=80530 TIP 138 is Accepted. Is this the implementation patch? ---------------------------------------------------------------------- Comment By: Joe Mistachkin (mistachkin) Date: 2003-06-05 23:57 Message: 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. ---------------------------------------------------------------------- Comment By: Jeffrey Hobbs (hobbs) Date: 2003-05-11 12:51 Message: 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? ---------------------------------------------------------------------- Comment By: Joe Mistachkin (mistachkin) Date: 2003-05-11 08:07 Message: 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. ---------------------------------------------------------------------- Comment By: Joe Mistachkin (mistachkin) Date: 2003-05-11 07:41 Message: 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. ---------------------------------------------------------------------- Comment By: Joe Mistachkin (mistachkin) Date: 2003-05-10 21:42 Message: 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. ---------------------------------------------------------------------- Comment By: Kevin B KENNY (kennykb) Date: 2003-05-10 13:25 Message: 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. ---------------------------------------------------------------------- Comment By: Joe Mistachkin (mistachkin) Date: 2003-05-05 02:31 Message: Logged In: YES user_id=113501 A few minor changes pursuant to discussions with jyl. ---------------------------------------------------------------------- Comment By: Joe Mistachkin (mistachkin) Date: 2003-05-05 00:34 Message: 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). ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=310894&aid=731356&group_id=10894 |