From: Joey Hess Date: Wed, 10 Sep 2025 16:05:55 +0000 (-0400) Subject: Improve performance when used with a local git remote that has a large working tree X-Git-Tag: archive/raspbian/10.20251029-1+rpi1~1^2~3^2~123 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=4fd71c125e6e144508c5e76a9b69e896914a57f8;p=git-annex.git Improve performance when used with a local git remote that has a large working tree 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 --- diff --git a/Annex/Locations.hs b/Annex/Locations.hs index 6d1d8804cc..9ce7a70a94 100644 --- a/Annex/Locations.hs +++ b/Annex/Locations.hs @@ -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 diff --git a/CHANGELOG b/CHANGELOG index 9549c28264..be8cdacf1a 100644 --- 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 Fri, 29 Aug 2025 12:34:06 -0400 diff --git a/Database/Keys.hs b/Database/Keys.hs index 93e659f445..f17d07dbe7 100644 --- a/Database/Keys.hs +++ b/Database/Keys.hs @@ -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 diff --git a/Database/Keys/Handle.hs b/Database/Keys/Handle.hs index 1e4a85427b..70e28ab441 100644 --- a/Database/Keys/Handle.hs +++ b/Database/Keys/Handle.hs @@ -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 43a15f8952..6de37709dc 100644 --- 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" diff --git a/doc/bugs/get_from_local_git_remote_slow_because_reconcileStaged_runs_for_each_file.mdwn b/doc/bugs/get_from_local_git_remote_slow_because_reconcileStaged_runs_for_each_file.mdwn index efdd32bd62..1d49b66bdf 100644 --- a/doc/bugs/get_from_local_git_remote_slow_because_reconcileStaged_runs_for_each_file.mdwn +++ b/doc/bugs/get_from_local_git_remote_slow_because_reconcileStaged_runs_for_each_file.mdwn @@ -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]] diff --git a/doc/todo/v11_changes.mdwn b/doc/todo/v11_changes.mdwn index 79f5647d1b..434952a8fa 100644 --- a/doc/todo/v11_changes.mdwn +++ b/doc/todo/v11_changes.mdwn @@ -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.