Re: [PATCH] Reduce the amount of WriteFile() syscalls when writing to buffered files

On 23 August 2017 at 19:47, Evgeny Kotkov <evgeny.kotkov_at_visualsvn.com> wrote:
> Evgeny Kotkov <evgeny.kotkov_at_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