Improve performance when used with a local git remote that has a large working tree
authorJoey Hess <joeyh@joeyh.name>
Wed, 10 Sep 2025 16:05:55 +0000 (12:05 -0400)
committerJoey Hess <joeyh@joeyh.name>
Wed, 10 Sep 2025 16:08:11 +0000 (12:08 -0400)
git write-tree was being run once per file git-annex acts on when eg,
getting files, which is slow when the remote repository has a large
tree.

onLocal calls quiesce after each action, and quiesce closes the keys db
since [[!commit ba7ecbc6a9c]]. Which has a relevant comment about
performance. I have not addressed that, the keys db still gets closed and
reopened after each file.

Turns out that, since git write-tree was run by each call to
reconcileStaged, the .git/annex/keysdb.cache value was never the
same as the git index's inode. Because git write-tree updates the index's
mtime even when no changes have been made.

And so, when the database got closed and reopened, reconcileStaged would
see a changed index, and run git write-tree again. Over and over.

I considered writing the index's new inodecache after write-tree to the
keysdb.cache, but that would be vulnerable to a race, if the index was
changed just after write-tree.

The fix was to stop using keysb.cache at all. When the database is closed
and later reopened by the same process, avoid re-doing reconcileStaged.

Now that .git/annex/keysdb.cache is no longer used. It could be removed,
but the time overhead of removing it would be more than the space overhead
of keeping it. Defferred removal to the v11 upgrade.

Sponsored-by: unqueued
Annex/Locations.hs
CHANGELOG
Database/Keys.hs
Database/Keys/Handle.hs
Test.hs
doc/bugs/get_from_local_git_remote_slow_because_reconcileStaged_runs_for_each_file.mdwn
doc/todo/v11_changes.mdwn

index 6d1d8804cc2a4d3770bb7c16549541c368c6b211..9ce7a70a94026f33e1d97658d6687f18a0885bae 100644 (file)
@@ -47,7 +47,6 @@ module Annex.Locations (
        gitAnnexUnusedLog,
        gitAnnexKeysDbDir,
        gitAnnexKeysDbLock,
-       gitAnnexKeysDbIndexCache,
        gitAnnexFsckState,
        gitAnnexFsckDbDir,
        gitAnnexFsckDbDirOld,
@@ -411,11 +410,6 @@ gitAnnexKeysDbDir r c =
 gitAnnexKeysDbLock :: Git.Repo -> GitConfig -> OsPath
 gitAnnexKeysDbLock  r c = gitAnnexKeysDbDir r c <> literalOsPath ".lck"
 
-{- Contains the stat of the last index file that was
- - reconciled with the keys database. -}
-gitAnnexKeysDbIndexCache :: Git.Repo -> GitConfig -> OsPath
-gitAnnexKeysDbIndexCache r c = gitAnnexKeysDbDir r c <> literalOsPath ".cache"
-
 {- .git/annex/fsck/uuid/ is used to store information about incremental
  - fscks. -}
 gitAnnexFsckDir :: UUID -> Git.Repo -> Maybe GitConfig -> OsPath
index 9549c28264c2fee55158f70eedfa691cf3547790..be8cdacf1afeba568cbaf7e35db9b3ff1f28b0d4 100644 (file)
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -10,6 +10,8 @@ git-annex (10.20250829) UNRELEASED; urgency=medium
   * Avoid leaking file descriptors to child processes started by git-annex
     in some situations. Note that when not built with the OsPath build
     flag, these leaks can still happen.
+  * Improve performance when used with a local git remote that has a
+    large working tree.
 
  -- Joey Hess <id@joeyh.name>  Fri, 29 Aug 2025 12:34:06 -0400
 
index 93e659f445b611b4910d11e55c42007c3bef008f..f17d07dbe7317f05a4d29ab72f871dd7713f7238 100644 (file)
@@ -47,7 +47,6 @@ import Git
 import Git.FilePath
 import Git.Command
 import Git.Types
-import Git.Index
 import Git.Sha
 import Git.CatFile
 import Git.Branch (writeTreeQuiet, update')
@@ -81,8 +80,8 @@ runReader t a = do
                        else return tableschanged
                v <- a (SQL.ReadHandle qh)
                return (v, DbOpen (qh, tableschanged'))
-       go DbClosed = do
-               st <- openDb False DbClosed
+       go startst@(DbClosed _) = do
+               st <- openDb False startst
                v <- case st of
                        (DbOpen (qh, _)) -> a (SQL.ReadHandle qh)
                        _ -> return mempty
@@ -124,7 +123,11 @@ runWriterIO t a = runWriter t (liftIO . a)
 openDb :: Bool -> DbState -> Annex DbState
 openDb _ st@(DbOpen _) = return st
 openDb False DbUnavailable = return DbUnavailable
-openDb forwrite _ = do
+openDb forwrite (DbClosed wasopen) = openDb' forwrite wasopen
+openDb forwrite DbUnavailable = openDb' forwrite (DbWasOpen False) 
+
+openDb' :: Bool -> DbWasOpen -> Annex DbState
+openDb' forwrite wasopen = do
        lck <- calcRepo' gitAnnexKeysDbLock
        catchPermissionDenied permerr $ withExclusiveLock lck $ do
                dbdir <- calcRepo' gitAnnexKeysDbDir
@@ -144,7 +147,7 @@ openDb forwrite _ = do
        
        open db dbisnew = do
                qh <- liftIO $ H.openDbQueue db SQL.containedTable
-               tc <- reconcileStaged dbisnew qh
+               tc <- reconcileStaged dbisnew qh wasopen
                return $ DbOpen (qh, tc)
 
 {- Closes the database if it was open. Any writes will be flushed to it.
@@ -238,8 +241,8 @@ isInodeKnown i s = or <$> runReaderIO ContentTable
  - This is run with a lock held, so only one process can be running this at
  - a time.
  -
- - To avoid unnecessary work, the index file is statted, and if it's not
- - changed since last time this was run, nothing is done.
+ - If the database gets closed and then reopened by the same process, this
+ - will avoid doing any repeated work.
  -
  - A tree is generated from the index, and the diff between that tree
  - and the last processed tree is examined for changes.
@@ -259,30 +262,19 @@ isInodeKnown i s = or <$> runReaderIO ContentTable
  - So when using getAssociatedFiles, have to make sure the file still
  - is an associated file.
  -}
-reconcileStaged :: Bool -> H.DbQueue -> Annex DbTablesChanged
-reconcileStaged dbisnew qh = ifM isBareRepo
+reconcileStaged :: Bool -> H.DbQueue -> DbWasOpen -> Annex DbTablesChanged
+reconcileStaged _ _ (DbWasOpen True) =
+       return (DbTablesChanged False False)
+reconcileStaged dbisnew qh _ = ifM isBareRepo
        ( return mempty
-       , do
-               gitindex <- inRepo currentIndexFile
-               indexcache <- calcRepo' gitAnnexKeysDbIndexCache
-               withTSDelta (liftIO . genInodeCache gitindex) >>= \case
-                       Just cur -> readindexcache indexcache >>= \case
-                               Nothing -> go cur indexcache =<< getindextree
-                               Just prev -> ifM (compareInodeCaches prev cur)
-                                       ( return mempty
-                                       , go cur indexcache =<< getindextree
-                                       )
-                       Nothing -> return mempty
+       , go =<< getindextree
        )
   where
        lastindexref = Ref "refs/annex/last-index"
 
-       readindexcache indexcache = liftIO $ maybe Nothing readInodeCache
-               <$> catchMaybeIO (readFileString indexcache)
-
        getoldtree = fromMaybe emptyTree <$> inRepo (Git.Ref.sha lastindexref)
        
-       go cur indexcache (Just newtree) = do
+       go (Just newtree) = do
                oldtree <- getoldtree
                when (oldtree /= newtree) $ do
                        fastDebug "Database.Keys" "reconcileStaged start"
@@ -292,7 +284,6 @@ reconcileStaged dbisnew qh = ifM isBareRepo
                                        (Just (fromRef oldtree)) 
                                        (fromRef newtree)
                                        (procdiff mdfeeder)
-                       liftIO $ writeFileString indexcache $ showInodeCache cur
                        -- Storing the tree in a ref makes sure it does not
                        -- get garbage collected, and is available to diff
                        -- against next time.
@@ -309,7 +300,7 @@ reconcileStaged dbisnew qh = ifM isBareRepo
        -- When there is a merge conflict, that will not see the new local
        -- version of the files that are conflicted. So a second diff
        -- is done, with --staged but no old tree.
-       go _ _ Nothing = do
+       go Nothing = do
                fastDebug "Database.Keys" "reconcileStaged start (in conflict)"
                oldtree <- getoldtree
                g <- Annex.gitRepo
index 1e4a85427b3fac065646874e424c45a10b7e5708..70e28ab44144941b61050fe3900085d35d9525d6 100644 (file)
@@ -9,6 +9,7 @@ module Database.Keys.Handle (
        DbHandle,
        newDbHandle,
        DbState(..),
+       DbWasOpen(..),
        withDbState,
        flushDbQueue,
        closeDbHandle,
@@ -30,10 +31,16 @@ newtype DbHandle = DbHandle (MVar DbState)
 
 -- The database can be closed or open, but it also may have been
 -- tried to open (for read) and didn't exist yet or is not readable.
-data DbState = DbClosed | DbOpen (H.DbQueue, DbTablesChanged) | DbUnavailable
+data DbState 
+       = DbClosed DbWasOpen
+       | DbOpen (H.DbQueue, DbTablesChanged)
+       | DbUnavailable
+
+-- Was the database previously opened by this process?
+data DbWasOpen = DbWasOpen Bool
 
 newDbHandle :: IO DbHandle
-newDbHandle = DbHandle <$> newMVar DbClosed
+newDbHandle = DbHandle <$> newMVar (DbClosed (DbWasOpen False))
 
 -- Runs an action on the state of the handle, which can change its state.
 -- The MVar is empty while the action runs, which blocks other users
@@ -65,5 +72,5 @@ closeDbHandle h = withDbState h go
   where
        go (DbOpen (qh, _)) = do
                H.closeDbQueue qh
-               return ((), DbClosed)
+               return ((), DbClosed (DbWasOpen True))
        go st = return ((), st)
diff --git a/Test.hs b/Test.hs
index 43a15f89524bce7a10891ed5a434e0e08fb6a53c..6de37709dca409cbcf4b0a9f1fda4b9599e94c80 100644 (file)
--- a/Test.hs
+++ b/Test.hs
@@ -896,8 +896,6 @@ test_lock_force = intmpclonerepo $ do
                Just k <- Annex.WorkTree.lookupKey (toOsPath annexedfile)
                Database.Keys.removeInodeCaches k
                Database.Keys.closeDb
-               liftIO . removeWhenExistsWith removeFile
-                       =<< Annex.calcRepo' Annex.Locations.gitAnnexKeysDbIndexCache
        writecontent annexedfile "test_lock_force content"
        git_annex_shouldfail "lock" [annexedfile] "lock of modified file should not be allowed"
        git_annex "lock" ["--force", annexedfile] "lock --force of modified file"
index efdd32bd62c9b75989b3d05cdb2da22cd7af8d99..1d49b66bdfaf7afff4fa68a9ab9d8e71d2b55eb5 100644 (file)
@@ -3,3 +3,5 @@ can be slow in large repos.
 
 It should be possible for Remote.Git to cache the state so this doesn't
 happen once per file. --[[Joey]]
+
+> [[fixed|done]] --[[Joey]]
index 79f5647d1baa5ad35321ff127ce0f5c4c3a65223..434952a8fad5d35e385edd4b8606185c6c464250 100644 (file)
@@ -28,3 +28,5 @@ version.
   the same as the bug that the second transfer lock was added to fix.
   Still, it would make sense to put this in a v12 upgrade that is delayed
   some amount of time (eg 1 year) after v11.
+
+* Remove .git/annex/keysdb.cache, which is no longer used.