Evgeny Kotkov <evgeny.kotkov_at_visualsvn.com> writes:
> In the meanwhile, apparently, there is an oversight in the core V1 patch
> 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
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
- 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.
Received on 2017-08-23 18:47:28 CEST