classification
Title: thread_nt.c update
Type: enhancement Stage:
Components: Versions: Python 3.0
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: kristjan.jonsson Nosy List: amaury.forgeotdarc, eckhardt, kristjan.jonsson, loewis
Priority: normal Keywords: patch

Created on 2008-08-17 21:00 by kristjan.jonsson, last changed 2009-01-19 10:31 by kristjan.jonsson. This issue is now closed.

Files
File name Uploaded Description Edit
thread_nt.patch kristjan.jonsson, 2008-08-17 21:00
Messages (13)
msg71289 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2008-08-17 21:00
Here is a suggested update to thread_nt.c.  It has two significant 
changes:
1) it uses the new and safer _beginthreadex API to start a thread
2) it implements native TLS functions on NT, which are certain to be as 
fast as possible.
msg79179 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2009-01-05 18:12
- Isn't CreateThread even better?

- Using TLS functions is a good idea, beware though that the number of
TLS slots cannot exceed 1088 on Windows. This should at least be documented.
OTOH since there is only one call to PyThread_create_key in the codebase
(for the GIL), and I found only one more through the Google Code Search,
it's probably not worth the trouble.
msg79180 - (view) Author: Ulrich Eckhardt (eckhardt) Date: 2009-01-05 18:25
No, CreateThread() is not a suitable replacement, because it lacks 
some initialisations of the C library, as explained in the MSDN: 
http://msdn.microsoft.com/en-us/library/ms682453(VS.85).aspx
msg79188 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2009-01-05 19:25
You are right. 
(My dream is someday to completely get rid of this msvcrt stuff. Only
pure Win32 API)
msg79236 - (view) Author: Ulrich Eckhardt (eckhardt) Date: 2009-01-06 08:39
I do have an issue with the patch's startup code. The prototype for 
the thread entry should be "DWORD WINAPI function(PVOID);". The 
important distinction is the WINAPI part, which resolves to __stdcall 
but doesn't have to. I know that some CE targets actually #define 
__cdecl __stdcall or vice versa, but using WINAPI always works. I'd 
then also change the comment to "...to adapt between our function 
signature and the one used by _createthreadex", as the internally used 
one doesn't mention __cdecl.
msg79238 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2009-01-06 09:03
No, the startup function for beginthreadex() must be __stdcall.  From 
the docs:

start_address
Start address of a routine that begins execution of a new thread. For 
_beginthread, the calling convention is either __cdecl or __clrcall; 
for _beginthreadex, it is either __stdcall or __clrcall.

(__clrcall is for managed code which doesn't apply).

Do not confuse beginthreadex() with CreateThread even though they have 
similar arguments.  The former is for use with programs using the crt.

See http://msdn.microsoft.com/en-us/library/kdzttdcb.aspx for details.

Amaury, Where did you find the value 1088 for maximum TLS slots?  The 
documentation mentions the error condidion TLS_OUT_OF_INDEXES (which I 
should check for, I suppose), but nothing else.
msg79239 - (view) Author: Ulrich Eckhardt (eckhardt) Date: 2009-01-06 09:08
You're right, Kristján, sorry for the noise. Shouldn't make claims 
before being fully awake....
msg79499 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2009-01-09 20:04
Committed this as revision: 68455
msg79503 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2009-01-09 20:29
Windows NT (3.1, and a number of later versions) only support 64 TLS
keys. Starting with Windows 2000, they added another page per thread for
TLS, giving an addition 1024 TLS slots, for a total of 1088 TLS slots.
FWIW, Win 9.x supported 80 TLS slots. See

http://www.nynaeve.net/?p=181
http://msdn.microsoft.com/en-us/library/ms686749.aspx
http://bugs.python.org/file11141/thread_nt.patch

TLS slots are typically considered a scarce resource, so that
programming language implementations typically don't allow applications
direct allocation of TLS slots. Instead, most runtimes I know of will
allocate a single TLS slot for themselves, which then is filled with an
array or a dictionary.
msg79653 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2009-01-12 08:25
Please add a Misc/NEWS entry for this change.
msg79656 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2009-01-12 09:19
Note, see defect 4906 for fallout caused by this checkin.
msg79657 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2009-01-12 09:21
Misc/NEWS updated in revision: 68544
msg80154 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2009-01-19 10:31
Note, this has been ported to py3k in http://svn.python.org/view?
view=rev&rev=68543
History
Date User Action Args
2009-01-19 10:31:50kristjan.jonssonsetkeywords: patch, patch
messages: + msg80154
2009-01-12 22:06:41loewissetstatus: open -> closed
keywords: patch, patch
2009-01-12 09:21:01kristjan.jonssonsetkeywords: patch, patch
messages: + msg79657
2009-01-12 09:19:58kristjan.jonssonsetkeywords: patch, patch
messages: + msg79656
2009-01-12 08:25:26loewissetstatus: closed -> open
assignee: kristjan.jonsson
messages: + msg79653
keywords: patch, patch
2009-01-09 20:29:21loewissetkeywords: patch, patch
nosy: + loewis
messages: + msg79503
2009-01-09 20:04:08kristjan.jonssonsetstatus: open -> closed
keywords: patch, patch
resolution: accepted
messages: + msg79499
2009-01-06 09:08:35eckhardtsetmessages: + msg79239
2009-01-06 09:03:51kristjan.jonssonsetkeywords: patch, patch
messages: + msg79238
2009-01-06 08:39:46eckhardtsetmessages: + msg79236
2009-01-05 19:25:09amaury.forgeotdarcsetkeywords: patch, patch
messages: + msg79188
2009-01-05 18:25:39eckhardtsetnosy: + eckhardt
messages: + msg79180
2009-01-05 18:12:41amaury.forgeotdarcsetkeywords: patch, patch
nosy: + amaury.forgeotdarc
messages: + msg79179
2008-08-17 21:00:18kristjan.jonssoncreate