Re: Why does apr_file_read() with !APR_XTHREAD use mutexes on Windows

[Greg Stein <gs...@gmail.com>]

Applied to trunk in r960671.

I'm unclear on backporting/releases right now, so that hasn't been
done. We should do that before new branch releases.

Thanks,
-g

On Sat, Jun 5, 2010 at 05:57, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On Tue, Apr 20, 2010 at 13:07, Bert Huijben <be...@qqmail.nl> wrote:
>>> -----Original Message-----
>>> From: Bert Huijben [mailto:bert@qqmail.nl]
>>> Sent: vrijdag 16 april 2010 17:28
>>> To: dev@apr.apache.org
>>> Subject: Why does apr_file_read() with !APR_XTHREAD on Windows
>>>
>>>       Hi,
>>>
>>> While profiling subversions file usage performance on Windows, one major
>>> slowdown shows:
>>>
>>> When a file is opened with APR_BUFFERED on Windows all read and file
>>> operations are slowed down by a mutex (or actually a critical section),
>> even
>>> though the function is not documented to be thread safe unless you use
>>> APR_XTHREAD.
>>>
>>> I'm trying to find out why this is implemented this way on Windows, but
>> not
>>> on the other platforms.
>>>
>>> * Are all file operations supposed to be slow but thread safe on Windows?
>>> * Or is this just to cover up old compatibility issues in other systems.
>>> (httpd defaults to threading on Windows)
>>> * Can we change this without breaking backwards compatibility?
>>> (I'm willing to write patches, etc.)
>>> * Or is it a better route to add a new flag that disables this mutex
>> around
>>> the buffering.
>>>
>>> Looking at the subversion history of this mutex, I found that it was
>>> introduced in r60083 (may 2000) and the documentation on why the mutex
>>> was
>>> added is nonexisting.
>>>
>>> Going forward ten years since that revision, we got hyperthreading and
>>> multicore systems and the critical section which used to be fast
>>> (implemented as an interlocked exchange) is now much slower because it
>>> has
>>> to perform actual CPU and caching synchronization.
>>>
>>>
>>> I would like to remove this performance penalty of using the mutexes for
>>> Subversion (which explicitly documents that you can't use its instance
>>> objects in multiple threads at the same time)... What are the recommended
>>> steps I should take?
>>
>> Ping. No response at all in this thread.
>>
>> Any suggestions on what steps to take to reduce the slowdown?
>>
> Any updates? I've prepared patch (see attachment) and made some
> performance testing. Reading file byte-by-byte many times gives the
> following rough numbers:
>
> single core CPU
> ============
> unpatched:   205  (time ticks)
> patched:       72    (time ticks)
> Performance improvement: 64%
>
> 2 core CPU
> =========
> unpatched:   217  (time ticks)
> patched :      92    (time ticks)
>
> Performance improvement: 57%
>
> Any chance to get this committed to trunk and backport to 1.3.x?
>
> --
> Ivan Zhakov
> VisualSVN Team
>

Re: Why does apr_file_read() with !APR_XTHREAD use mutexes on Windows

["William A. Rowe Jr." <wr...@rowe-clan.net>]

On 7/5/2010 4:30 PM, Greg Stein wrote:
> On Mon, Jul 5, 2010 at 16:33, William A. Rowe Jr. <wr...@rowe-clan.net> wrote:
>> On 7/5/2010 2:49 PM, Greg Stein wrote:
>>> Applied to trunk in r960671.
>>>
>>> I'm unclear on backporting/releases right now, so that hasn't been
>>> done. We should do that before new branch releases.
>>
>> I'm 99.5% sure the mutex locking existing before I had added the XTHREAD
>> flag.  The .5% leaves me wondering if we should do this to 1.x.
> 
> We want it in Subversion.
> 
> Who are the other APR users that might be affected by such a thing?
> Which relied on the undocumented behavior?

Since noone has any examples or stated any concerns sounds like this too
could be applied to 1.5 and even 1.4 branches, although such a change to
1.4 seems a little dicier.  At least at 1.5 folks expect arbitrary breakage.

Re: Why does apr_file_read() with !APR_XTHREAD use mutexes on Windows

[Greg Stein <gs...@gmail.com>]

On Mon, Jul 5, 2010 at 16:33, William A. Rowe Jr. <wr...@rowe-clan.net> wrote:
> On 7/5/2010 2:49 PM, Greg Stein wrote:
>> Applied to trunk in r960671.
>>
>> I'm unclear on backporting/releases right now, so that hasn't been
>> done. We should do that before new branch releases.
>
> I'm 99.5% sure the mutex locking existing before I had added the XTHREAD
> flag.  The .5% leaves me wondering if we should do this to 1.x.

We want it in Subversion.

Who are the other APR users that might be affected by such a thing?
Which relied on the undocumented behavior?

Cheers,
-g

Re: Why does apr_file_read() with !APR_XTHREAD use mutexes on Windows

["William A. Rowe Jr." <wr...@rowe-clan.net>]

On 7/5/2010 2:49 PM, Greg Stein wrote:
> Applied to trunk in r960671.
> 
> I'm unclear on backporting/releases right now, so that hasn't been
> done. We should do that before new branch releases.

I'm 99.5% sure the mutex locking existing before I had added the XTHREAD
flag.  The .5% leaves me wondering if we should do this to 1.x.