> -----Original Message----- > From: Ivan Zhakov [mailto:i...@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 <i...@visualsvn.com> wrote: > > On 2 August 2014 13:02, Bert Huijben <b...@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