Mailing-List: contact dev-help@apr.apache.org; run by ezmlm
Precedence: bulk
Received-SPF: pass (athena.apache.org: domain of ivan@visualsvn.com designates
 207.126.144.143 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <002c01cae068$d7e5e970$87b1bc50$@qqmail.nl>
References: <AQLX7KeE8mpfsKHvXym/DIv4JpnNjQ==>
	<002c01cae068$d7e5e970$87b1bc50$@qqmail.nl>
Date: Sat, 5 Jun 2010 13:57:39 +0400
Message-ID: <AANLkTim6LwFIqZRhZziW8XwlXDsFMvcy8J80lJ-XPqhC@mail.gmail.com>
Subject: Re: Why does apr_file_read() with !APR_XTHREAD use mutexes on Windows
From: Ivan Zhakov <ivan@visualsvn.com>
To: wrowe@rowe-clan.net, trawick@gmail.com
Cc: dev@apr.apache.org, Bert Huijben <bert@qqmail.nl>
Content-Type: multipart/mixed; boundary=001636c9257d537e75048845783a

--001636c9257d537e75048845783a
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On Tue, Apr 20, 2010 at 13:07, Bert Huijben <bert@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
>>
>> =C2=A0 =C2=A0 =C2=A0 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 recommende=
d
>> 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
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
unpatched:   205  (time ticks)
patched:       72    (time ticks)
Performance improvement: 64%

2 core CPU
=3D=3D=3D=3D=3D=3D=3D=3D=3D
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?

--=20
Ivan Zhakov
VisualSVN Team

--001636c9257d537e75048845783a
Content-Type: text/plain; charset=US-ASCII;
	name="apr-improve-file-io-performance-1.patch.txt"
Content-Disposition: attachment;
	filename="apr-improve-file-io-performance-1.patch.txt"
Content-Transfer-Encoding: base64
X-Attachment-Id: f_ga29ro3p0

SW5kZXg6IGZpbGVfaW8vd2luMzIvcmVhZHdyaXRlLmMNCj09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0NCi0tLSBmaWxlX2lv
L3dpbjMyL3JlYWR3cml0ZS5jCShyZXZpc2lvbiA5NTE2ODcpDQorKysgZmlsZV9pby93aW4zMi9y
ZWFkd3JpdGUuYwkod29ya2luZyBjb3B5KQ0KQEAgLTE4MSwxMiArMTgxLDE2IEBADQogICAgICAg
ICBhcHJfc2l6ZV90IGJsb2Nrc2l6ZTsKICAgICAgICAgYXByX3NpemVfdCBzaXplID0gKmxlbjsK
IAotICAgICAgICBhcHJfdGhyZWFkX211dGV4X2xvY2sodGhlZmlsZS0+bXV0ZXgpOworICAgICAg
ICBpZiAodGhlZmlsZS0+ZmxhZ3MgJiBBUFJfRk9QRU5fWFRIUkVBRCkgeworICAgICAgICAgICAg
YXByX3RocmVhZF9tdXRleF9sb2NrKHRoZWZpbGUtPm11dGV4KTsKKyAgICAgICAgfQogCiAgICAg
ICAgIGlmICh0aGVmaWxlLT5kaXJlY3Rpb24gPT0gMSkgewogICAgICAgICAgICAgcnYgPSBhcHJf
ZmlsZV9mbHVzaCh0aGVmaWxlKTsKICAgICAgICAgICAgIGlmIChydiAhPSBBUFJfU1VDQ0VTUykg
ewotICAgICAgICAgICAgICAgIGFwcl90aHJlYWRfbXV0ZXhfdW5sb2NrKHRoZWZpbGUtPm11dGV4
KTsKKyAgICAgICAgICAgICAgICBpZiAodGhlZmlsZS0+ZmxhZ3MgJiBBUFJfRk9QRU5fWFRIUkVB
RCkgeworICAgICAgICAgICAgICAgICAgICBhcHJfdGhyZWFkX211dGV4X3VubG9jayh0aGVmaWxl
LT5tdXRleCk7CisgICAgICAgICAgICAgICAgfQogICAgICAgICAgICAgICAgIHJldHVybiBydjsK
ICAgICAgICAgICAgIH0KICAgICAgICAgICAgIHRoZWZpbGUtPmJ1ZnBvcyA9IDA7CkBAIC0yMjMs
NyArMjI3LDEwIEBADQogICAgICAgICBpZiAoKmxlbikgewogICAgICAgICAgICAgcnYgPSBBUFJf
U1VDQ0VTUzsKICAgICAgICAgfQotICAgICAgICBhcHJfdGhyZWFkX211dGV4X3VubG9jayh0aGVm
aWxlLT5tdXRleCk7CisKKyAgICAgICAgaWYgKHRoZWZpbGUtPmZsYWdzICYgQVBSX0ZPUEVOX1hU
SFJFQUQpIHsKKyAgICAgICAgICAgIGFwcl90aHJlYWRfbXV0ZXhfdW5sb2NrKHRoZWZpbGUtPm11
dGV4KTsKKyAgICAgICAgfQogICAgIH0gZWxzZSB7ICAKICAgICAgICAgLyogVW5idWZmZXJlZCBp
L28gKi8KICAgICAgICAgYXByX3NpemVfdCBuYnl0ZXM7CkBAIC0yNjAsNyArMjY3LDkgQEANCiAg
ICAgICAgIGFwcl9zaXplX3QgYmxvY2tzaXplOwogICAgICAgICBhcHJfc2l6ZV90IHNpemUgPSAq
bmJ5dGVzOwogCi0gICAgICAgIGFwcl90aHJlYWRfbXV0ZXhfbG9jayh0aGVmaWxlLT5tdXRleCk7
CisgICAgICAgIGlmICh0aGVmaWxlLT5mbGFncyAmIEFQUl9GT1BFTl9YVEhSRUFEKSB7CisgICAg
ICAgICAgICBhcHJfdGhyZWFkX211dGV4X2xvY2sodGhlZmlsZS0+bXV0ZXgpOworICAgICAgICB9
CiAKICAgICAgICAgaWYgKHRoZWZpbGUtPmRpcmVjdGlvbiA9PSAwKSB7CiAgICAgICAgICAgICAv
LyBQb3NpdGlvbiBmaWxlIHBvaW50ZXIgZm9yIHdyaXRpbmcgYXQgdGhlIG9mZnNldCB3ZSBhcmUg
bG9naWNhbGx5IHJlYWRpbmcgZnJvbQpAQCAtMjg2LDcgKzI5NSw5IEBADQogICAgICAgICAgICAg
c2l6ZSAtPSBibG9ja3NpemU7CiAgICAgICAgIH0KIAotICAgICAgICBhcHJfdGhyZWFkX211dGV4
X3VubG9jayh0aGVmaWxlLT5tdXRleCk7CisgICAgICAgIGlmICh0aGVmaWxlLT5mbGFncyAmIEFQ
Ul9GT1BFTl9YVEhSRUFEKSB7CisgICAgICAgICAgICBhcHJfdGhyZWFkX211dGV4X3VubG9jayh0
aGVmaWxlLT5tdXRleCk7CisgICAgICAgIH0KICAgICAgICAgcmV0dXJuIHJ2OwogICAgIH0gZWxz
ZSB7CiAgICAgICAgIGlmICghdGhlZmlsZS0+cGlwZSkgewo=
--001636c9257d537e75048845783a--