You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Bert Huijben <be...@qqmail.nl> on 2014/08/02 11:02:41 UTC
[Patch] R/W lock slowness on Windows (Was: Windows R/W lock comment / Reader Writer lock performance on Windows)
Hi,
With this mail I would like to ping the original problem with the second
patch I send to the list. This patch doesn't have the likely original
problems of potentially changing the lock behavior to a spin lock, but
provides all the performance improvements anyway.
It just replaces the Windows inter-proces mutex in the apr code with the
already existing apr-mutex implementation, which is usually implemented as
the far more efficient Windows in-process Critical section.
This makes the implementation 10* to 140* times more efficient on Windows
and more similar to the performance on other platforms. A performance fix
like this is critical for Subversion, or we would have to implement our own
r/w locking implementation for our caching infrastructure.
Bert
Re: [Patch] R/W lock slowness on Windows (Was: Windows R/W lock
comment / Reader Writer lock performance on Windows)
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 27 August 2014 17:34, Bert Huijben <be...@qqmail.nl> wrote:
>
>
>> -----Original Message-----
>> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
>> Sent: woensdag 27 augustus 2014 15:29
>> To: Bert Huijben
>> Subject: Re: [Patch] R/W lock slowness on Windows (Was: Windows R/W lock
>> comment / Reader Writer lock performance on Windows)
>>
>> Hi Bert!
>>
>> Did you get any feedback about this from APR developers? What do you
>> think about my suggestion that I've posted on apr@ mailing list?
>
> I didn't get any feedback except yours :(
>
> 1)
> In general apr doesn't promise anything about its output arguments on error cases, so I'm not sure if we should start look at this specifically here.
> (Other platforms don't care and I see at least one that don't set it on all error cases)
>
Yes, I know that general practice do not initialize output arguments
on error, but I think it worth to keep old behavior because this is
very sensitive area (deadlocks and etc).
> 2)
> Same reasoning: Other platforms differ here, so I don't see any requirement to keep Windows strictly as it was.
Ok.
--
Ivan Zhakov
RE: [Patch] R/W lock slowness on Windows (Was: Windows R/W lock comment / Reader Writer lock performance on Windows)
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
> Sent: woensdag 27 augustus 2014 15:29
> To: Bert Huijben
> Subject: Re: [Patch] R/W lock slowness on Windows (Was: Windows R/W lock
> comment / Reader Writer lock performance on Windows)
>
> Hi Bert!
>
> Did you get any feedback about this from APR developers? What do you
> think about my suggestion that I've posted on apr@ mailing list?
I didn't get any feedback except yours :(
1)
In general apr doesn't promise anything about its output arguments on error cases, so I'm not sure if we should start look at this specifically here.
(Other platforms don't care and I see at least one that don't set it on all error cases)
2)
Same reasoning: Other platforms differ here, so I don't see any requirement to keep Windows strictly as it was.
Bert
>
> On 4 August 2014 02:27, Ivan Zhakov <iv...@visualsvn.com> wrote:
> > On 2 August 2014 13:02, Bert Huijben <be...@qqmail.nl> wrote:
> >> Hi,
> >>
> >> With this mail I would like to ping the original problem with the second
> >> patch I send to the list. This patch doesn't have the likely original
> >> problems of potentially changing the lock behavior to a spin lock, but
> >> provides all the performance improvements anyway.
> >>
> >> It just replaces the Windows inter-proces mutex in the apr code with the
> >> already existing apr-mutex implementation, which is usually implemented
> as
> >> the far more efficient Windows in-process Critical section.
> >>
> >> This makes the implementation 10* to 140* times more efficient on
> Windows
> >> and more similar to the performance on other platforms. A performance
> fix
> >> like this is critical for Subversion, or we would have to implement our own
> >> r/w locking implementation for our caching infrastructure.
> >>
> > Hi Bert,
> >
> > Comments on your patch:
> > 1. It seems behavior of apr_thread_rwlock_create() functions changes a
> > bit: code in trunk intializes output *rwlock pointer to NULL in case
> > of error, while your patch returns pointer to partially initialized
> > object. It seems that apr_thread_rwlock_create() doesn't have API
> > promise to initalize output to NULL, but I think it's better to leave
> > current behavior.
> >
> > 2. The apr_thread_rwlock_unlock() currently uses checked mutex
> > behavior for its logic. This is possible reason why current rwlock
> > implementation uses OS mutexes instead of cirtical sections. I don't
> > see problem with proposed change though.
> >
> >
> > --
> > Ivan Zhakov
>
>
>
> --
> Ivan Zhakov
> CTO | VisualSVN | http://www.visualsvn.com
Re: [Patch] R/W lock slowness on Windows (Was: Windows R/W lock
comment / Reader Writer lock performance on Windows)
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 2 August 2014 13:02, Bert Huijben <be...@qqmail.nl> wrote:
> Hi,
>
> With this mail I would like to ping the original problem with the second
> patch I send to the list. This patch doesn't have the likely original
> problems of potentially changing the lock behavior to a spin lock, but
> provides all the performance improvements anyway.
>
> It just replaces the Windows inter-proces mutex in the apr code with the
> already existing apr-mutex implementation, which is usually implemented as
> the far more efficient Windows in-process Critical section.
>
> This makes the implementation 10* to 140* times more efficient on Windows
> and more similar to the performance on other platforms. A performance fix
> like this is critical for Subversion, or we would have to implement our own
> r/w locking implementation for our caching infrastructure.
>
Hi Bert,
Comments on your patch:
1. It seems behavior of apr_thread_rwlock_create() functions changes a
bit: code in trunk intializes output *rwlock pointer to NULL in case
of error, while your patch returns pointer to partially initialized
object. It seems that apr_thread_rwlock_create() doesn't have API
promise to initalize output to NULL, but I think it's better to leave
current behavior.
2. The apr_thread_rwlock_unlock() currently uses checked mutex
behavior for its logic. This is possible reason why current rwlock
implementation uses OS mutexes instead of cirtical sections. I don't
see problem with proposed change though.
--
Ivan Zhakov