On 1/20/22 5:14 PM, Yann Ylavic wrote:
> On Thu, Jan 20, 2022 at 2:41 PM Ruediger Pluem <rpl...@apache.org> wrote:
>>
>> On 1/20/22 2:24 PM, Yann Ylavic wrote:
>>
>>>
>>> All good points, thanks Rüdiger, should be fixed in r1897250.
>>
>> Great. I guess next we need to think what we do for 2.4.x.
>> Even when 1.8.x is released, we cannot demand it for 2.4.x (for trunk we
>> could).
>> I guess we have two general choices:
>>
>> 1. We implement it differently on 2.4.x also using TLS when available, but
>> not requiring APR 1.8.x.
>> 2. We recommend that people who switch over to PCRE2 to use APR 1.8.x for
>> performance reasons.
>>
>> As using PCRE2 make some sense and should be encouraged 2. would make it
>> difficult for people tied to older APR versions like some
>> distributions.
>>
>> For 1. my rough idea would be that in case of a threaded MPM we could store
>> the apr_thread_t pointer of a worker_thread in a TLS.
>> That would solve the performance issue in most cases.
>
> How about the attached patch?
> For APR < 1.8 it defines ap_thread_create()/ap_thread_current() as
> wrappers around the APR ones plus storing/loading the current
> apr_thread_t* from the TLS as APR 1.8 does (if implemented by the
> compiler still). Then it applies a simple
> s/apr_thread_create/ap_thread_create/g in the code base..
> Finally ap_regex is adapted to use that if available at compile time
> and runtime, otherwise it falls back to allocation/free.
>
>>
>> BTW: I think the current code does not work well in the case of
>> !APR_HAS_THREADS, but in this case we could just a static variable
>> to store the pointer, correct?
>
> AP[R]_HAS_THREAD_LOCAL are defined on if APR_HAS_THREADS, so it should
> be fine for the compilation.
> I'm not sure we should optimize for the !APR_HAS_THREADS case with a
> static though, but I could be convinced..
Rethinking as well and I guess even users of prefork will likely compile on
systems with APR_HAS_THREADS and probably
AP[R]_HAS_THREAD_LOCAL. Hence it might be a very small group of users that
would benefit from this. Hence lets park this
for a while and resurrect it if my assumption on the amount of users benefiting
from this was wrong.
Regards
Rüdiger
