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