Re: Thread local storage

[Ludovic Courtès]

Hello,

Neil Jerram <address@hidden> writes:

> address@hidden (Ludovic Courtès) writes:
>
>> I looked again at how/whether we could improve thread-local storage
>> access, using compiler support (the ‘__thread’ storage class).
>
> I worry a bit about having another build option, that might end up not
> used and so bitrotting.

Right, it increases the configuration space, and in practice most people
will end up using ‘__thread’.  OTOH I think the change is quite
isolated, since we have only one thread-local variable.

> Also I wonder, if #1 is always better than the
> current implementation of pthread_getspecific, why the implementation of
> pthread_getspecific isn't rewritten so that it uses #1 inside.  Maybe
> the signature of the pthread_getspecific API doesn't allow that.

pthread_getspecific(3) is a function, whereas ‘__thread’ requires
support from the compiler and dynamic linker.

>> So #3 is appealing (~8% speedup on ‘gcbench’, ~10% on 30 iterations of
>> ‘nboyer’).  Unfortunately it’s not generally applicable to libguile
>> since libguile may be dlopened (e.g., the XChat-Guile plug-in).
>
> (I presume it's actually the plug-in that gets dlopened, and the plug-in
> links to libguile.  I also presume that that doesn't make any difference
> in practice.)

Yes.

>> The relevant work in ‘wip-tls’:
>>
>>   commit 8346727c49c51a9668f10b507daff62dd889850a
>>   Author: Ludovic Courtès <address@hidden>
>>   Date:   Fri Oct 2 15:02:52 2009 +0200
>>
>>       Deprecate `scm_mask_ints'.
>
> Is there any replacement, or is this something that there was never any
> reason to expose?  It would be good for the deprecation comment to say a
> bit more to justify the deprecation.

I didn’t understand the point of this macro, and it’s not documented,
which is why the deprecation message doesn’t suggest any replacement.

Actually, it seems that it was used as an lvalue to mask block asyncs:

  http://google.com/codesearch?q=scm_mask_ints&hl=en&btnG=Search+Code

In that case, I don’t know how we could provide a useful compatibility
layer.

Should we worry?

>>   commit b9619bff4ddff267149e7e869ef3c2bcb9c4f4b4
>>   Author: Ludovic Courtès <address@hidden>
>>   Date:   Fri Oct 2 15:28:29 2009 +0200
>>
>>       Arrange so that `SCM_I_CURRENT_THREAD' is not accessed outside of 
>> libguile.
>
> This is great.  I recommend writing the NEWS soon too, so as not to
> build up a backlog of such items that have been removed from the API.

Well, Andy has been very good at it so far.  ;-)

Besides, ‘SCM_I_CURRENT_THREAD’ is *not* part of the API.

> +    [AC_COMPILE_IFELSE([AC_LANG_PROGRAM([static __thread int tls_integer;],
> +                         [tls_integer = 123;])],
>
> Since we don't use static for scm_i_current_thread, I guess it would be
> a more accurate test not to use static here.

Yes, why not.

> +  pf ("/* Define the 1 if the compiler supports the "
> +      "`__thread' storage class.  */\n");
>
> s/Define the/Define as/

Noted.

Thanks,
Ludo’.