Erik Bray added the comment:

Hi,

First of all, I should be clear this is not just about CloudABI.  In fact I 
don't even know what CloudABI is.  I'm personally working toward getting a 
working/stable buildbot for Cygwin, so that I can move Python toward supporting 
it again.  Cygwin has not been officially unsupported, but support for it is 
broken due to lack of a buildbot and lack of a maintainer, both of which I'm 
willing to offer.  I can't get a stable buildbot though until some issues with 
the build are resolved, and this is a major blocker.  If you prefer I can also 
work on a more formal plan to clarify Python's support for Cygwin which is 
currently in limbo.

Not saying a solution to this needs to be merged ASAP as I can work on a 
branch, but in the meantime it's nice to have an initial fix for the issue so 
that it can be worked around--I'm sure other platforms that have this issue, 
such as the CloudABI folks, will appreciate that as well.

Second of all, the point must be made that I'm not suggesting this be changed 
just in order to support some specific platform.  The fact remains that the 
existing code is misusing the pthread API and is fragile to begin with.  "It 
works on Linux" is not an excuse--not only does it suggest a bias, but the fact 
that it even works on Linux is an accident of implementation. The 
implementation of the pthread_key_t type could change tomorrow, and it would be 
Python's fault if that breaks things (not that I think that's at all likely to 
happen, though I'd be less surprised if OSX suddenly changed :)

As for the performance I agree that PyThread_(get|set)_key_value should be 
fast.  This may be O(n) but how big, typically, is n?  In a normal run of the 
Python interpreter n=1, and the autoTLSkey is always the first in the list.  It 
is only larger if some third-party code is using the PyThread_ API for TLS (and 
in most typical uses this will only be a few keys at the most, though I 
couldn't even find any examples in the wild where third-party code is using 
these functions).

n could also grow on forks, or manual Py_Finialize/Py_Initialize.  This patch 
didn't implement PyThread_ReInitTLS but it could, in which case it would reset 
next_key_id to 0 for each child process.  Likewise, PyThread_ReInitTLS could 
also be called from Py_Initialize, which should be harmless.

So TL;DR I'm not really convinced by the performance argument.  But even if 
that were an issue other implementations are possible; this was just among the 
simplest and least wasteful.

----------

_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue25658>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to