remove supportUnlocked check that is not worth its overhead
authorJoey Hess <joeyh@joeyh.name>
Tue, 15 Jun 2021 13:24:59 +0000 (09:24 -0400)
committerJoey Hess <joeyh@joeyh.name>
Tue, 15 Jun 2021 13:28:56 +0000 (09:28 -0400)
moveAnnex only gets to that check if the object file was not present
before. So in the case where dup files are being added repeatedly,
it will only run the first time, and so there's no significant speedup
from doing it; all it avoids is a single sqlite lookup. Since MVar
accesses do have overhead, it's better to optimise for the common case,
where unlocked files are supported.

removeAnnex is less clear cut, but I think mostly is skipped running on
keys when the object has already been dropped, so similar reasoning
applies.

Annex/Content.hs
doc/bugs/significant_performance_regression_impacting_datal/comment_33_6e5121e066998a303cf68ebc53e9fc15._comment [new file with mode: 0644]

index 07daa14cce61aeee8795cd49db49c7b26b9fd35d..12af39618c0be59a69d2e359c51c55a9e26cb0e7 100644 (file)
@@ -340,13 +340,12 @@ moveAnnex key af src = ifM (checkSecureHashes' key)
                        liftIO $ moveFile
                                (fromRawFilePath src)
                                (fromRawFilePath dest)
-                       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)
+                       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
 
@@ -503,11 +502,10 @@ removeAnnex (ContentRemovalLock key) = withObjectLoc key $ \file ->
        cleanObjectLoc key $ do
                secureErase file
                liftIO $ removeWhenExistsWith R.removeLink file
-               whenM (annexSupportUnlocked <$> Annex.getGitConfig) $ do
-                       g <- Annex.gitRepo 
-                       mapM_ (\f -> void $ tryIO $ resetpointer $ fromTopFilePath f g)
-                               =<< Database.Keys.getAssociatedFiles key
-                       Database.Keys.removeInodeCaches key
+               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/doc/bugs/significant_performance_regression_impacting_datal/comment_33_6e5121e066998a303cf68ebc53e9fc15._comment b/doc/bugs/significant_performance_regression_impacting_datal/comment_33_6e5121e066998a303cf68ebc53e9fc15._comment
new file mode 100644 (file)
index 0000000..deeeec0
--- /dev/null
@@ -0,0 +1,10 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 33"""
+ date="2021-06-15T13:01:04Z"
+ content="""
+Oh, there's a much better solution: If the annex object file already exists
+when ingesting a new file, skip populating other associated files. They
+will have already been populated. moveAnnex has to check if the annex object
+file already exists anyway, so this will have zero overhead.
+"""]]