From 014dc63a551dc8acd87e66be880dfd5b8baac441 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Mon, 14 Jun 2021 12:36:55 -0400 Subject: [PATCH] avoid sometimes expensive operations when annex.supportunlocked = false This will mostly just avoid a DB lookup, so things get marginally faster. But in cases where there are many files using the same key, it can be a more significant speedup. Added overhead is one MVar lookup per call, which should be small enough, since this happens after transferring or ingesting a file, which is always a lot more work than that. It would be nice, though, to move getGitConfig to AnnexRead, which there is an open todo about. --- Annex/Content.hs | 22 ++++++++++--------- Annex/Ingest.hs | 17 +++++++------- ..._0d6a37f823cd9cb3ed1e6e90066ebd2c._comment | 9 ++++++++ .../move_readonly_values_to_AnnexRead.mdwn | 10 ++++----- 4 files changed, 35 insertions(+), 23 deletions(-) create mode 100644 doc/bugs/significant_performance_regression_impacting_datal/comment_28_0d6a37f823cd9cb3ed1e6e90066ebd2c._comment diff --git a/Annex/Content.hs b/Annex/Content.hs index 12af39618c..07daa14cce 100644 --- a/Annex/Content.hs +++ b/Annex/Content.hs @@ -340,12 +340,13 @@ moveAnnex key af src = ifM (checkSecureHashes' key) liftIO $ moveFile (fromRawFilePath src) (fromRawFilePath dest) - g <- Annex.gitRepo - fs <- map (`fromTopFilePath` g) - <$> Database.Keys.getAssociatedFiles key - unless (null fs) $ do - ics <- mapM (populatePointerFile (Restage True) key dest) fs - Database.Keys.storeInodeCaches' key [dest] (catMaybes ics) + whenM (annexSupportUnlocked <$> Annex.getGitConfig) $ do + g <- Annex.gitRepo + fs <- map (`fromTopFilePath` g) + <$> Database.Keys.getAssociatedFiles key + unless (null fs) $ do + ics <- mapM (populatePointerFile (Restage True) key dest) fs + Database.Keys.storeInodeCaches' key [dest] (catMaybes ics) ) alreadyhave = liftIO $ R.removeLink src @@ -502,10 +503,11 @@ removeAnnex (ContentRemovalLock key) = withObjectLoc key $ \file -> cleanObjectLoc key $ do secureErase file liftIO $ removeWhenExistsWith R.removeLink file - g <- Annex.gitRepo - mapM_ (\f -> void $ tryIO $ resetpointer $ fromTopFilePath f g) - =<< Database.Keys.getAssociatedFiles key - Database.Keys.removeInodeCaches key + whenM (annexSupportUnlocked <$> Annex.getGitConfig) $ do + g <- Annex.gitRepo + mapM_ (\f -> void $ tryIO $ resetpointer $ fromTopFilePath f g) + =<< Database.Keys.getAssociatedFiles key + Database.Keys.removeInodeCaches key where -- Check associated pointer file for modifications, and reset if -- it's unmodified. diff --git a/Annex/Ingest.hs b/Annex/Ingest.hs index 7c0d6f449c..b8d98f1084 100644 --- a/Annex/Ingest.hs +++ b/Annex/Ingest.hs @@ -225,14 +225,15 @@ finishIngestUnlocked' key source restage = do {- Copy to any unlocked files using the same key. -} populateUnlockedFiles :: Key -> KeySource -> Restage -> Annex () -populateUnlockedFiles key source restage = do - obj <- calcRepo (gitAnnexLocation key) - g <- Annex.gitRepo - ingestedf <- flip fromTopFilePath g - <$> inRepo (toTopFilePath (keyFilename source)) - afs <- map (`fromTopFilePath` g) <$> Database.Keys.getAssociatedFiles key - forM_ (filter (/= ingestedf) afs) $ - populatePointerFile restage key obj +populateUnlockedFiles key source restage = + whenM (annexSupportUnlocked <$> Annex.getGitConfig) $ do + obj <- calcRepo (gitAnnexLocation key) + g <- Annex.gitRepo + ingestedf <- flip fromTopFilePath g + <$> inRepo (toTopFilePath (keyFilename source)) + afs <- map (`fromTopFilePath` g) <$> Database.Keys.getAssociatedFiles key + forM_ (filter (/= ingestedf) afs) $ + populatePointerFile restage key obj cleanCruft :: KeySource -> Annex () cleanCruft source = when (contentLocation source /= keyFilename source) $ diff --git a/doc/bugs/significant_performance_regression_impacting_datal/comment_28_0d6a37f823cd9cb3ed1e6e90066ebd2c._comment b/doc/bugs/significant_performance_regression_impacting_datal/comment_28_0d6a37f823cd9cb3ed1e6e90066ebd2c._comment new file mode 100644 index 0000000000..f430ed767e --- /dev/null +++ b/doc/bugs/significant_performance_regression_impacting_datal/comment_28_0d6a37f823cd9cb3ed1e6e90066ebd2c._comment @@ -0,0 +1,9 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 28""" + date="2021-06-14T16:31:26Z" + content=""" +@Ilya sure can be skipped when annex.supportunlocked=false. +I've implemented that. (And also for several other cases that have similar +behavior, like dropping a key.) +"""]] diff --git a/doc/todo/move_readonly_values_to_AnnexRead.mdwn b/doc/todo/move_readonly_values_to_AnnexRead.mdwn index b14627df7f..1a2906aabf 100644 --- a/doc/todo/move_readonly_values_to_AnnexRead.mdwn +++ b/doc/todo/move_readonly_values_to_AnnexRead.mdwn @@ -5,8 +5,8 @@ moved to AnnexRead for a performance win and also to make clean how it's used. --[[Joey]] The easy things have been moved now, but some things like Annex.force and -Annex.fast would be good to move. Moving those would involve running -argument processing outside the Annex monad. The main reason argument -processing runs in the Annex monad is to set those values, but there may be -other reasons too, so this will be a large set of changes that need to all -happen together. --[[Joey]] +Annex.fast and Annex.getGitConfig would be good to move. Moving those would +involve running argument processing outside the Annex monad. The main +reason argument processing runs in the Annex monad is to set those values, +but there may be other reasons too, so this will be a large set of changes +that need to all happen together. --[[Joey]] -- 2.30.2