On Fri, Aug 27, 2021 at 4:49 PM Neeraj K. Singh via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote: > > Thanks to everyone for review so far! I've responded to the previous > feedback and changed the patch series a bit. > > Changes since v1: > > * Switch from futimes(2) to futimens(2), which is in POSIX.1-2008. Contrary > to dscho's suggestion, I'm still implementing the Windows version in the > same patch and I'm not doing autoconf detection since this is a POSIX > function. > > * Introduce a separate preparatory patch to the bulk-checkin infrastructure > to separate the 'plugged' variable and rename the 'state' variable, as > suggested by dscho. > > * Add performance numbers to the commit message of the main bulk fsync > patch, as suggested by dscho. > > * Add a comment about the non-thread-safety of the bulk-checkin > infrastructure, as suggested by avarab. > > * Rename the experimental mode to core.fsyncobjectfiles=batch, as suggested > by dscho and avarab and others. > > * Add more details to Documentation/config/core.txt about the various > settings and their intended effects, as suggested by avarab. > > * Switch to the string-list API to hold the rename state, as suggested by > avarab. > > * Create a separate update-index patch to use bulk-checkin as suggested by > dscho. > > * Add Windows support in the upstream git. This is done in a way that > should not conflict with git-for-windows. > > * Add new performance tests that shows the delta based on fsync mode. > > NOTE: Based on Christoph Hellwig's comments, the 'batch' mode is not correct > on Linux, since sync_file_range does not provide data integrity guarantees. > There is currently no kernel interface suitable to achieve disk flush > batching as is, but he suggested that he might implement a 'syncfs' variant > on top of this patchset. This code is still useful on macOS and Windows, and > the config documentation makes that clear. > > Neeraj Singh (6): > object-file: use futimens rather than utime > bulk-checkin: rename 'state' variable and separate 'plugged' boolean > core.fsyncobjectfiles: batched disk flushes > core.fsyncobjectfiles: add windows support for batch mode > update-index: use the bulk-checkin infrastructure > core.fsyncobjectfiles: performance tests for add and stash > > Documentation/config/core.txt | 26 ++++++-- > Makefile | 6 ++ > builtin/add.c | 3 +- > builtin/update-index.c | 3 + > bulk-checkin.c | 92 +++++++++++++++++++++++++---- > bulk-checkin.h | 4 +- > cache.h | 8 ++- > compat/mingw.c | 53 +++++++++++------ > compat/mingw.h | 5 ++ > compat/win32/flush.c | 29 +++++++++ > config.c | 8 ++- > config.mak.uname | 4 ++ > configure.ac | 8 +++ > contrib/buildsystems/CMakeLists.txt | 3 +- > environment.c | 2 +- > git-compat-util.h | 7 +++ > object-file.c | 23 ++------ > t/perf/lib-unique-files.sh | 32 ++++++++++ > t/perf/p3700-add.sh | 43 ++++++++++++++ > t/perf/p3900-stash.sh | 46 +++++++++++++++ > wrapper.c | 40 +++++++++++++ > write-or-die.c | 2 +- > 22 files changed, 389 insertions(+), 58 deletions(-) > create mode 100644 compat/win32/flush.c > create mode 100644 t/perf/lib-unique-files.sh > create mode 100755 t/perf/p3700-add.sh > create mode 100755 t/perf/p3900-stash.sh > > > base-commit: 225bc32a989d7a22fa6addafd4ce7dcd04675dbf > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1076%2Fneerajsi-msft%2Fneerajsi%2Fbulk-fsync-object-files-v2 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1076/neerajsi-msft/neerajsi/bulk-fsync-object-files-v2 > Pull-Request: https://github.com/git/git/pull/1076 Hello everyone, I'd like to bump this review up in people's inboxes since Patch V2 hasn't gotten any traction in over a week. Thanks in advance for taking a look, - Neeraj Singh Windows Core Filesystems Team
