On 19 December 2014 at 19:26, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
>
> On 19 December 2014 at 18:55, Branko Čibej <brane_at_wandisco.com> wrote:
> > On 19.12.2014 15:37, Ivan Zhakov wrote:
> >> Originally FSFS caching implementation was using apr_thread_mutex_t to
> >> serialize access to shared data. In r1346122 [1] implementation was
> >> switched to apr_thread_rw_lock_t to improve performance. This change
> >> was released in Subversion 1.8.0
> >>
> >> Unfortunately current apr_thread_rwlock_t implementation is very slow
> >> on Windows. As quick workaround FSFS code was patched to select
> >> apr_thread_mutex_t or apr_thread_rwlock_t() depending of platform on
> >> compile time (r1611380 [2]).
> >>
> >> In further investigation Bert found why apr_thread_rwlock_t() so slow
> >> on Windows: implementation uses kernel level mutex object instead of
> >> lightweight critical section (critical sections are also used in
> >> apr_thread_mutex_t in most cases).
> >>
> >> The simple patch to switch apr_thread_rwlock_t() implementation to use
> >> critical sections (through apr_thread_mutex_t) was proposed on APR
> >> development mailing list [3], but patch was not commited nor reviewed.
> >>
> >> So the current situation is:
> >> 1. APR has performance problem on Windows that hurts Subversion 1.8.x and trunk
> >> 2. Subversion trunk has workaround for this specific problem at compile time
> >> 3. Patch proposed to APR mailing list, without any reaction for three months
> >>
> >> From my point of view the proposed patch is straightforward and Bert
> >> stated that it makes apr_thread_rwlock_t 10 to 140 times more
> >> efficient on Windows. So the best option will be to commit Bert's
> >> patch, but I cannot do this since I'm not APR committer :(
> >>
> >> Thoughts?
> >
> >
> >
> > Thanks for the reminder ... I remember that thread on apr-dev, but it
> > slipped my mind. If I don't review and commit Bert's patch in the next
> > week or so, feel free to send another nagging e-mail. :)
> >
> Will do!
>
> > FWIW, committing to APR trunk isn't enough: should be back-ported to 1.6
> > and 1.5, too.
> >
> Yes, you are right. We need this to be backported to 1.6 and 1.5 APR branches.
>
Btw it's not the only problem with APR read/write lock on Windows and
OS/2: rwlock sometimes allows a writer to take the lock while a reader
has the lock. The detailed explanation of possible race-condition
given in issue 45455 [1].
Subversion 1.8.x is affected by this issue and FSFS membuffer cache
may be corrupted. I'm thinking about backporting r1611380 to 1.8.x or
switch back to simple mutex for FSFS caches for all platforms. Any
thoughts will be very appreciated.
[1] https://issues.apache.org/bugzilla/show_bug.cgi?id=45455
--
Ivan Zhakov
Received on 2014-12-22 14:09:59 CET