Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core.fsyncmethod: add 'batch' mode for faster fsyncing of multiple objects #1134

Closed
wants to merge 13 commits into from

Commits on Mar 31, 2022

  1. bulk-checkin: rename 'state' variable and separate 'plugged' boolean

    This commit prepares for adding batch-fsync to the bulk-checkin
    infrastructure.
    
    The bulk-checkin infrastructure is currently used to batch up addition
    of large blobs to a packfile. When a blob is larger than
    big_file_threshold, we unconditionally add it to a pack. If bulk
    checkins are 'plugged', we allow multiple large blobs to be added to a
    single pack until we reach the packfile size limit; otherwise, we simply
    make a new packfile for each large blob. The 'unplug' call tells us when
    the series of blob additions is done so that we can finish the packfiles
    and make their objects available to subsequent operations.
    
    Stated another way, bulk-checkin allows callers to define a transaction
    that adds multiple objects to the object database, where the object
    database can optimize its internal operations within the transaction
    boundary.
    
    Batched fsync will fit into bulk-checkin by taking advantage of the
    plug/unplug functionality to determine the appropriate time to fsync
    and make newly-added objects available in the primary object database.
    
    * Rename 'state' variable to 'bulk_checkin_packfile', since we will
      later be adding 'bulk_fsync_objdir'. This also makes the variable
      easier to find in the debugger, since the name is more unique.
    
    * Rename finish_bulk_checkin to flush_bulk_checkin_packfile and call it
      unconditionally from unplug_bulk_checkin. Internally it will
      conditionally do a flush if there's any work to do.
    
    * Move the 'plugged' data member of 'bulk_checkin_state' into a separate
      static variable. Doing this avoids resetting the variable in
      finish_bulk_checkin when zeroing the 'bulk_checkin_state'. As-is, we
      seem to unintentionally disable the plugging functionality the first
      time a new packfile must be created due to packfile size limits. While
      disabling the plugging state only results in suboptimal behavior for
      the current code, it would be fatal for the bulk-fsync functionality
      later in this patch series.
    
    The net effect of these changes is to make a clear separation between
    the portion of the bulk-checkin infrastructure that is related to the
    packfile (nearly all of it at present) and the part that is related to
    other future optimizations of the ODB.
    
    Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
    neerajsi-msft committed Mar 31, 2022
    Configuration menu
    Copy the full SHA
    3b20234 View commit details
    Browse the repository at this point in the history
  2. bulk-checkin: rebrand plug/unplug APIs as 'odb transactions'

    Make it clearer in the naming and documentation of the plug_bulk_checkin
    and unplug_bulk_checkin APIs that they can be thought of as
    a "transaction" to optimize operations on the object database. These
    transactions may be nested so that subsystems like the cache-tree
    writing code can optimize their operations without caring whether the
    top-level code has a transaction active.
    
    Add a flush_odb_transaction API that will be used in update-index to
    make objects visible even if a transaction is active. The flush call may
    also be useful in future cases if we hold a transaction active around
    calling hooks.
    
    Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
    neerajsi-msft committed Mar 31, 2022
    Configuration menu
    Copy the full SHA
    66cba40 View commit details
    Browse the repository at this point in the history
  3. object-file: pass filename to fsync_or_die

    If we die while trying to fsync a loose object file, pass the actual
    filename we're trying to sync. This is likely to be more helpful for a
    user trying to diagnose the cause of the failure than the former
    'loose object file' string. It also sidesteps any concerns about
    translating the die message differently for loose objects versus
    something else that has a real path.
    
    Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
    neerajsi-msft committed Mar 31, 2022
    Configuration menu
    Copy the full SHA
    fcb5afb View commit details
    Browse the repository at this point in the history
  4. core.fsyncmethod: batched disk flushes for loose-objects

    When adding many objects to a repo with `core.fsync=loose-object`,
    the cost of fsync'ing each object file can become prohibitive.
    
    One major source of the cost of fsync is the implied flush of the
    hardware writeback cache within the disk drive. This commit introduces
    a new `core.fsyncMethod=batch` option that batches up hardware flushes.
    It hooks into the bulk-checkin odb-transaction functionality, takes
    advantage of tmp-objdir, and uses the writeout-only support code.
    
    When the new mode is enabled, we do the following for each new object:
    1a. Create the object in a tmp-objdir.
    2a. Issue a pagecache writeback request and wait for it to complete.
    
    At the end of the entire transaction when unplugging bulk checkin:
    1b. Issue an fsync against a dummy file to flush the log and hardware
       writeback cache, which should by now have seen the tmp-objdir writes.
    2b. Rename all of the tmp-objdir files to their final names.
    3b. When updating the index and/or refs, we assume that Git will issue
       another fsync internal to that operation. This is not the default
       today, but the user now has the option of syncing the index and there
       is a separate patch series to implement syncing of refs.
    
    On a filesystem with a singular journal that is updated during name
    operations (e.g. create, link, rename, etc), such as NTFS, HFS+, or XFS
    we would expect the fsync to trigger a journal writeout so that this
    sequence is enough to ensure that the user's data is durable by the time
    the git command returns. This sequence also ensures that no object files
    appear in the main object store unless they are fsync-durable.
    
    Batch mode is only enabled if core.fsync includes loose-objects. If
    the legacy core.fsyncObjectFiles setting is enabled, but core.fsync does
    not include loose-objects, we will use file-by-file fsyncing.
    
    In step (1a) of the sequence, the tmp-objdir is created lazily to avoid
    work if no loose objects are ever added to the ODB. We use a tmp-objdir
    to maintain the invariant that no loose-objects are visible in the main
    ODB unless they are properly fsync-durable. This is important since
    future ODB operations that try to create an object with specific
    contents will silently drop the new data if an object with the target
    hash exists without checking that the loose-object contents match the
    hash. Only a full git-fsck would restore the ODB to a functional state
    where dataloss doesn't occur.
    
    In step (1b) of the sequence, we issue a fsync against a dummy file
    created specifically for the purpose. This method has a little higher
    cost than using one of the input object files, but makes adding new
    callers of this mechanism easier, since we don't need to figure out
    which object file is "last" or risk sharing violations by caching the fd
    of the last object file.
    
    _Performance numbers_:
    
    Linux - Hyper-V VM running Kernel 5.11 (Ubuntu 20.04) on a fast SSD.
    Mac - macOS 11.5.1 running on a Mac mini on a 1TB Apple SSD.
    Windows - Same host as Linux, a preview version of Windows 11.
    
    Adding 500 files to the repo with 'git add' Times reported in seconds.
    
    object file syncing | Linux | Mac   | Windows
    --------------------|-------|-------|--------
               disabled | 0.06  |  0.35 | 0.61
                  fsync | 1.88  | 11.18 | 2.47
                  batch | 0.15  |  0.41 | 1.53
    
    Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
    neerajsi-msft committed Mar 31, 2022
    Configuration menu
    Copy the full SHA
    64e6340 View commit details
    Browse the repository at this point in the history
  5. cache-tree: use ODB transaction around writing a tree

    Take advantage of the odb transaction infrastructure around writing the
    cached tree to the object database.
    
    Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
    neerajsi-msft committed Mar 31, 2022
    Configuration menu
    Copy the full SHA
    be7ccf9 View commit details
    Browse the repository at this point in the history
  6. builtin/add: add ODB transaction around add_files_to_cache

    The add_files_to_cache function is invoked internally by
    builtin/commit.c and builtin/checkout.c for their flags that stage
    modified files before doing the larger operation. These commands
    can benefit from batched fsyncing.
    
    Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
    neerajsi-msft committed Mar 31, 2022
    Configuration menu
    Copy the full SHA
    72c69ff View commit details
    Browse the repository at this point in the history
  7. update-index: use the bulk-checkin infrastructure

    The update-index functionality is used internally by 'git stash push' to
    setup the internal stashed commit.
    
    This change enables odb-transactions for update-index infrastructure to
    speed up adding new objects to the object database by leveraging the
    batch fsync functionality.
    
    There is some risk with this change, since under batch fsync, the object
    files will be in a tmp-objdir until update-index is complete, so callers
    using the --stdin option will not see them until update-index is done.
    This risk is mitigated by flushing the ODB transaction prior to
    reporting any verbose output so that objects will be visible to callers
    that are synchronizing with update-index by snooping its output.
    
    Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
    neerajsi-msft committed Mar 31, 2022
    Configuration menu
    Copy the full SHA
    288a484 View commit details
    Browse the repository at this point in the history
  8. unpack-objects: use the bulk-checkin infrastructure

    The unpack-objects functionality is used by fetch, push, and fast-import
    to turn the transfered data into object database entries when there are
    fewer objects than the 'unpacklimit' setting.
    
    By enabling an odb-transaction when unpacking objects, we can take advantage
    of batched fsyncs.
    
    Here are some performance numbers to justify batch mode for
    unpack-objects, collected on a WSL2 Ubuntu VM.
    
    Fsync Mode | Time for 90 objects (ms)
    -------------------------------------
           Off | 170
      On,fsync | 760
      On,batch | 230
    
    Note that the default unpackLimit is 100 objects, so there's a 3x
    benefit in the worst case. The non-batch mode fsync scales linearly
    with the number of objects, so there are significant benefits even with
    smaller numbers of objects.
    
    Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
    neerajsi-msft committed Mar 31, 2022
    Configuration menu
    Copy the full SHA
    1b03440 View commit details
    Browse the repository at this point in the history
  9. core.fsync: use batch mode and sync loose objects by default on Windows

    Git for Windows has defaulted to core.fsyncObjectFiles=true since
    September 2017. We turn on syncing of loose object files with batch mode
    in upstream Git so that we can get broad coverage of the new code
    upstream.
    
    We don't actually do fsyncs in the most of the test suite, since
    GIT_TEST_FSYNC is set to 0. However, we do exercise all of the
    surrounding batch mode code since GIT_TEST_FSYNC merely makes the
    maybe_fsync wrapper always appear to succeed.
    
    Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
    neerajsi-msft committed Mar 31, 2022
    Configuration menu
    Copy the full SHA
    16a3aa1 View commit details
    Browse the repository at this point in the history
  10. test-lib-functions: add parsing helpers for ls-files and ls-tree

    Several tests use awk to parse OIDs from the output of 'git ls-files
    --stage' and 'git ls-tree'. Introduce helpers to centralize these uses
    of awk.
    
    Update t5317-pack-objects-filter-objects.sh to use the new ls-files
    helper so that it has some usages to review. Other updates are left for
    the future.
    
    Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
    neerajsi-msft committed Mar 31, 2022
    Configuration menu
    Copy the full SHA
    127d35c View commit details
    Browse the repository at this point in the history
  11. core.fsyncmethod: tests for batch mode

    Add test cases to exercise batch mode for:
     * 'git add'
     * 'git stash'
     * 'git update-index'
     * 'git unpack-objects'
    
    These tests ensure that the added data winds up in the object database.
    
    In this change we introduce a new test helper lib-unique-files.sh. The
    goal of this library is to create a tree of files that have different
    oids from any other files that may have been created in the current test
    repo. This helps us avoid missing validation of an object being added
    due to it already being in the repo.
    
    Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
    neerajsi-msft committed Mar 31, 2022
    Configuration menu
    Copy the full SHA
    cea4df4 View commit details
    Browse the repository at this point in the history
  12. t/perf: add iteration setup mechanism to perf-lib

    Tests that affect the repo in stateful ways are easier to write if we
    can run setup steps outside of the measured portion of perf iteration.
    
    This change adds a "--setup 'setup-script'" parameter to test_perf. To
    make invocations easier to understand, I also moved the prerequisites to
    a new --prereq parameter.
    
    The setup facility will be used in the upcoming perf tests for batch
    mode, but it already helps in some existing tests, like t5302 and t7820.
    
    Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
    neerajsi-msft committed Mar 31, 2022
    Configuration menu
    Copy the full SHA
    e52f1da View commit details
    Browse the repository at this point in the history
  13. core.fsyncmethod: performance tests for batch mode

    Add basic performance tests for git commands that can add data to the
    object database. We cover:
    * git add
    * git stash
    * git update-index (via git stash)
    * git unpack-objects
    * git commit --all
    
    We cover all currently available fsync methods as well.
    
    Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
    neerajsi-msft committed Mar 31, 2022
    Configuration menu
    Copy the full SHA
    44c5297 View commit details
    Browse the repository at this point in the history