Re: [Python-ideas] New PyThread_tss_ C-API for CPython
On Sun, Dec 18, 2016 at 12:10 AM, Masayuki YAMAMOTO
wrote:
> 2016-12-17 18:35 GMT+09:00 Stephen J. Turnbull
> :
>>
>> I don't understand this. I assume that there are no such platforms
>> supported at present. I would think that when such a platform becomes
>> supported, code supporting "key" functions becomes unsupportable
>> without #ifdefs on that platform, at least directly. So you should
>> either (1) raise UnimplementedError, or (2) provide the API as a
>> wrapper over the new API by making the integer keys indexes into a
>> table of TSS'es, or some such device. I don't understand how (3)
>> "make it a no-op" can be implemented for PyThread_create_key -- return
>> 0 or -1? That would only work if there's a failure return status like
>> 0 or -1, and it seems really dangerous to me since in general a lot of
>> code doesn't check status even though it should. Even for code
>> checking the status, the error message will be suboptimal ("creation
>> failed" vs. "unimplemented").
>
>
> PyThread_create_key has required user to check the return value since when
> key creation fails, returns -1 instead of valid key value. Therefore, my
> patch changes PyThread_create_key that always return -1 on platforms that
> cannot cast key to int safely and current API never return valid key value
> to these platforms. Its advantage to not change function specifications and
> no effect on supported platforms. Hence, this is reason that doesn't raise
> any exception on the API.
>
> (2) of ideas can enable current API on specific-platforms. If it's simple,
> I'd have liked to select it. However, work that brings current API using
> native TLS to specific-platforms brings duplication implementation that
> manages keys, and it's ugly (same reason for Erik's draft, the last item of
> Rejected Ideas). Thus, I gave up to keep feature and decided to implement
> "no-op", delegate error handling to API users.
Yep--I think it speaks to the sensibleness of that decision that I
pretty much read your mind :)
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/
Re: [Python-ideas] New PyThread_tss_ C-API for CPython
On Sat, Dec 17, 2016 at 8:21 AM, Stephen J. Turnbull
wrote:
> Erik Bray writes:
>
> > Abstract
> >
> >
> > The proposal is to add a new Thread Local Storage (TLS) API to CPython
> > which would supersede use of the existing TLS API within the CPython
> > interpreter, while deprecating the existing API.
>
> Thank you for the analysis!
And thank *you* for the feedback!
> Question:
>
> > Further, the old PyThread_*_key* functions will be marked as
> > deprecated.
>
> Of course, but:
>
> > Additionally, the pthread implementations of the old
> > PyThread_*_key* functions will either fail or be no-ops on
> > platforms where sizeof(pythead_t) != sizeof(int).
>
> Typo "pythead_t" in last line.
Thanks, yes, that was suppose to be pthread_key_t of course. I think
I had a few other typos too.
> I don't understand this. I assume that there are no such platforms
> supported at present. I would think that when such a platform becomes
> supported, code supporting "key" functions becomes unsupportable
> without #ifdefs on that platform, at least directly. So you should
> either (1) raise UnimplementedError, or (2) provide the API as a
> wrapper over the new API by making the integer keys indexes into a
> table of TSS'es, or some such device. I don't understand how (3)
> "make it a no-op" can be implemented for PyThread_create_key -- return
> 0 or -1? That would only work if there's a failure return status like
> 0 or -1, and it seems really dangerous to me since in general a lot of
> code doesn't check status even though it should. Even for code
> checking the status, the error message will be suboptimal ("creation
> failed" vs. "unimplemented").
Masayuki already explained this downthread I think, but I could have
probably made that section more precise. The point was that
PyThread_create_key should immediately return -1 in this case. This
is just a subtle difference over the current situation, which is that
PyThread_create_key succeeds, but the key is corrupted by being cast
to an int, so that later calls to PyThread_set_key_value and the like
fail unexpectedly. The point is that PyThread_create_key (and we're
only talking about the pthread implementation thereof, to be clear)
must fail immediately if it can't work correctly.
#ifdefs on the platform would not be necessary--instead, Masayuki's
patch adds a feature check in configure.ac for sizeof(int) ==
sizeof(pthread_key_t). It should be noted that even this check is not
100% perfect, as on Linux pthread_key_t is an unsigned int, and so
technically can cause Python's signed int key to overflow, but there's
already an explicit check for that (which would be kept), and it's
also a very unlikely scenario.
> I gather from references to casting pthread_key_t to unsigned int and
> back that there's probably code that does this in ways making (2) too
> dangerous to support. If true, perhaps that should be mentioned here.
It's not necessarily too dangerous, so much as not worth the trouble,
IMO. Simpler to just provide, and immediately use the new API and
make the old one deprecated and explicitly not supported on those
platforms where it can't work.
Thanks,
Erik
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/
Re: [Python-ideas] New PyThread_tss_ C-API for CPython
On 17 December 2016 at 03:51, Antoine Pitrou wrote: > On Fri, 16 Dec 2016 13:07:46 +0100 > Erik Bray wrote: > > Greetings all, > > > > I wanted to bring attention to an issue that's been languishing on the > > bug tracker since last year, which I think would best be addressed by > > changes to CPython's C-API. The original issue is at > > http://bugs.python.org/issue25658, but I have made an effort below in > > a sort of proto-PEP to summarize the problem and the proposed > > solution. > > > > I haven't written this up in the proper PEP format because I want to > > see if the idea has some broader support first, and it's also not > > clear to me whether C-API changes (especially to undocumented APIs) > > even require their own PEP. > > This is a nice detailed write-up and I'm in favour of the proposal. > Likewise - we know the status quo isn't right, and the proposed change addresses that. In reviewing the patch on the tracker, the one downside I've found is that due to "pthread_key_t" being an opaque type with no defined sentinel, the consuming code in _tracemalloc.c and pystate.c needed to add separate boolean flag variables to track whether or not the key had been created. (The pthread examples at http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_key_create.html use pthread_once for a similar effect) I don't see any obvious way around that either, as even using a small struct for native pthread TLS keys would still face the problem of how to initialise the pthread_key_t field. Cheers, Nick. -- Nick Coghlan | ncogh...@gmail.com | Brisbane, Australia ___ Python-ideas mailing list Python-ideas@python.org https://mail.python.org/mailman/listinfo/python-ideas Code of Conduct: http://python.org/psf/codeofconduct/
Re: [Python-ideas] New PyThread_tss_ C-API for CPython
On Mon, Dec 19, 2016 at 1:11 PM, Nick Coghlan wrote: > On 17 December 2016 at 03:51, Antoine Pitrou wrote: >> >> On Fri, 16 Dec 2016 13:07:46 +0100 >> Erik Bray wrote: >> > Greetings all, >> > >> > I wanted to bring attention to an issue that's been languishing on the >> > bug tracker since last year, which I think would best be addressed by >> > changes to CPython's C-API. The original issue is at >> > http://bugs.python.org/issue25658, but I have made an effort below in >> > a sort of proto-PEP to summarize the problem and the proposed >> > solution. >> > >> > I haven't written this up in the proper PEP format because I want to >> > see if the idea has some broader support first, and it's also not >> > clear to me whether C-API changes (especially to undocumented APIs) >> > even require their own PEP. >> >> This is a nice detailed write-up and I'm in favour of the proposal. > > > Likewise - we know the status quo isn't right, and the proposed change > addresses that. In reviewing the patch on the tracker, the one downside I've > found is that due to "pthread_key_t" being an opaque type with no defined > sentinel, the consuming code in _tracemalloc.c and pystate.c needed to add > separate boolean flag variables to track whether or not the key had been > created. (The pthread examples at > http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_key_create.html > use pthread_once for a similar effect) > > I don't see any obvious way around that either, as even using a small struct > for native pthread TLS keys would still face the problem of how to > initialise the pthread_key_t field. Hmm...fair point that it's not pretty. One way around it, albeit requiring more work/complexity, would be to extend this proposal to add a new function analogous to pthread_once--say--PyThread_call_once, and an associated Py_once_flag_t ___ Python-ideas mailing list Python-ideas@python.org https://mail.python.org/mailman/listinfo/python-ideas Code of Conduct: http://python.org/psf/codeofconduct/
Re: [Python-ideas] New PyThread_tss_ C-API for CPython
On Mon, Dec 19, 2016 at 3:45 PM, Erik Bray wrote: > On Mon, Dec 19, 2016 at 1:11 PM, Nick Coghlan wrote: >> On 17 December 2016 at 03:51, Antoine Pitrou wrote: >>> >>> On Fri, 16 Dec 2016 13:07:46 +0100 >>> Erik Bray wrote: >>> > Greetings all, >>> > >>> > I wanted to bring attention to an issue that's been languishing on the >>> > bug tracker since last year, which I think would best be addressed by >>> > changes to CPython's C-API. The original issue is at >>> > http://bugs.python.org/issue25658, but I have made an effort below in >>> > a sort of proto-PEP to summarize the problem and the proposed >>> > solution. >>> > >>> > I haven't written this up in the proper PEP format because I want to >>> > see if the idea has some broader support first, and it's also not >>> > clear to me whether C-API changes (especially to undocumented APIs) >>> > even require their own PEP. >>> >>> This is a nice detailed write-up and I'm in favour of the proposal. >> >> >> Likewise - we know the status quo isn't right, and the proposed change >> addresses that. In reviewing the patch on the tracker, the one downside I've >> found is that due to "pthread_key_t" being an opaque type with no defined >> sentinel, the consuming code in _tracemalloc.c and pystate.c needed to add >> separate boolean flag variables to track whether or not the key had been >> created. (The pthread examples at >> http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_key_create.html >> use pthread_once for a similar effect) >> >> I don't see any obvious way around that either, as even using a small struct >> for native pthread TLS keys would still face the problem of how to >> initialise the pthread_key_t field. > > Hmm...fair point that it's not pretty. One way around it, albeit > requiring more work/complexity, would be to extend this proposal to > add a new function analogous to pthread_once--say--PyThread_call_once, > and an associated Py_once_flag_t Oops--fat-fingered a 'send' command before I finished. So workaround would be to add a PyThread_call_once function, analogous to pthread_once. Yet another interface one needs to implement for a native thread implementation, but not too hard either. For pthreads there's already an obvious analogue that can be wrapped directly. For other platforms that don't have a direct analogue a (naive) implementation is still fairly simple: All you need in Py_once_flag_t is a boolean flag with an associated mutex, and a sentinel value analogous to PTHREAD_ONCE_INIT. Best, Erik ___ Python-ideas mailing list Python-ideas@python.org https://mail.python.org/mailman/listinfo/python-ideas Code of Conduct: http://python.org/psf/codeofconduct/
