apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ivan Zhakov <i...@visualsvn.com>
Subject Re: [PATCH] Reduce the amount of WriteFile() syscalls when writing to buffered files
Date Sat, 26 Aug 2017 15:23:38 GMT
On 23 August 2017 at 19:47, Evgeny Kotkov <evgeny.kotkov@visualsvn.com> wrote:
> Evgeny Kotkov <evgeny.kotkov@visualsvn.com> writes:
>
>> In the meanwhile, apparently, there is an oversight in the core V1 patch
>> (3-reduce-syscalls-for-buffered-writes-v1.patch.txt):
>>
>> If the buffer is not empty, and the caller issues a write with a chunk
>> that slightly exceeds the buffer size, for example, 4100 bytes, it would
>> result in flushing the buffer _and_ writing the remaining couple of bytes
>> with WriteFile().  An appropriate thing to do here would be to flush the
>> buffer, but put the few remaining bytes into the buffer, instead of writing
>> them to disk and thus making an unnecessary syscall.
>>
>> With all this in mind, I will prepare the V2 version of the core patch.
>
> I have attached the V2 version of the core patch.
>
> To solve the aforementioned oversight in the V1 patch, I implemented the
> optimization in a slightly different manner, by keeping the existing loop
> but also handling a condition where we can write the remaining data with
> a single WriteFile() call.  Apart from this, the V2 patch also includes an
> additional test, test_small_and_large_writes_buffered().
>
> Speaking of the discussed idea of adjusting the strategy to avoid a memcpy()
> with the write pattern like
>
>     WriteFile(13)
>     WriteFile(86267)
>     ...
>
> instead of
>
>     WriteFile(4096)
>     WriteFile(82185)
>     ...
>
> I preferred to keep the latter approach which keeps the minimum size of
> the WriteFile() chunk equal to 4096, for the following reasons:
>
>   - My benchmarks do not show that the alternative pattern that also avoids
>     a memcpy() results in a quantifiable performance improvement.
>
>   - The existing code had a property that all WriteFile() calls, except
>     for maybe the last one, were performed in chunks with sizes that are
>     never less than 4096.  Although I can't reproduce this in my environment,
>     it could be that introducing a possibility of writing in smaller chunks
>     would result in the decreased performance with specific hardware, non-
>     file handles or in situations when the OS write caching is disabled.
>     Therefore, currently, I think that it would be better to keep this
>     existing property.
>
>   - Probably, implementing the first approach would result in a bit more
>     complex code, as I think that it would require introducing an additional
>     if-else code path.
>
> The log message is included in the beginning of the patch file.
>
Committed 3-reduce-syscalls-for-buffered-writes-v2.patch.txt patch in
r1806308. Thanks!

-- 
Ivan Zhakov

Mime
View raw message