Introduce core.trustPackedRefsStat config

Currently, we always read packed-refs file when 'trustFolderStat'
is false. Introduce a new config 'trustPackedRefsStat' which takes
precedence over 'trustFolderStat' when reading packed refs. Possible
values for this new config are:

* always: Trust packed-refs file attributes
* after_open: Same as 'always', but refresh the file attributes of
              packed-refs before trusting it
* never: Always read the packed-refs file
* unset: Fallback to 'trustFolderStat' to determine if the file
  attributes of packed-refs can be trusted

Folks whose repositories are on NFS and have traditionally been
setting 'trustFolderStat=false' can now get some performance improvement
with 'trustPackedRefsStat=after_open' as it refreshes the file
attributes of packed-refs (at least on some NFS clients) before
considering it.

For example, consider a repository on NFS with ~500k packed-refs. Here
are some stats which illustrate the improvement with this new config
when reading packed refs on NFS:

trustFolderStat=true trustPackedRefsStat=unset: 0.2ms
trustFolderStat=false trustPackedRefsStat=unset: 155ms
trustFolderStat=false trustPackedRefsStat=after_open: 1.5ms

Change-Id: I00da88e4cceebbcf3475be0fc0011ff65767c111
Signed-off-by: Kaushik Lingarkar <quic_kaushikl@quicinc.com>

diff --git a/Documentation/config-options.md b/Documentation/config-options.md
index 6551ba82da54c72f7528a89154fd5ef9ada5f3f0..9cb3c62a3538c293792a5e91ac90dd4754e4ecc7 100644 (file)
--- a/Documentation/config-options.md
+++ b/Documentation/config-options.md
@@ -46,6 +46,7 @@ For details on native git options see also the official [git config documentatio
 | `core.supportsAtomicFileCreation` | `true` | ⃞ | Whether the filesystem supports atomic file creation. |
 | `core.symlinks` | Auto detect if filesystem supports symlinks| ✅ | If false, symbolic links are checked out as small plain files that contain the link text. |
 | `core.trustFolderStat` | `true` | ⃞ | Whether to trust the pack folder's and packed-refs file's file attributes (Java equivalent of stat command on *nix). When looking for pack files, if `false` JGit will always scan the `.git/objects/pack` folder and if set to `true` it assumes that pack files are unchanged if the file attributes of the pack folder are unchanged. When getting the list of packed refs, if `false` JGit will always read the packed-refs file and if set to `true` it uses the file attributes of the packed-refs file and will only read it if a file attribute has changed. Setting this option to `false` can help to workaround caching issues on NFS, but reduces performance.|
+| `core.trustPackedRefsStat` | `unset` | ⃞ | Whether to trust the file attributes (Java equivalent of stat command on *nix) of the packed-refs file. If `never` JGit will ignore the file attributes of the packed-refs file and always read it. If `always` JGit will trust the file attributes of the packed-refs file and will only read it if a file attribute has changed. `after_open` behaves the same as `always`, except that the packed-refs file is opened and closed before its file attributes are considered. An open/close of the packed-refs file is known to refresh its file attributes, at least on some NFS clients. If `unset`, JGit will use the behavior described in `trustFolderStat`. |
 | `core.worktree` | Root directory of the working tree if it is not the parent directory of the `.git` directory | ✅ | The path to the root of the working tree. |
 
 ## __gc__ options

diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters
index a6f18310d6a63ba3bd1621e8bdb4982f24cbf0af..0f8433008269f600db2f7b910e912593a35cea8c 100644 (file)
--- a/org.eclipse.jgit/.settings/.api_filters
+++ b/org.eclipse.jgit/.settings/.api_filters
@@ -15,6 +15,20 @@
                 <message_argument value="SHA1_IMPLEMENTATION"/>
             </message_arguments>
         </filter>
+        <filter id="1142947843">
+            <message_arguments>
+                <message_argument value="6.1.1"/>
+                <message_argument value="CONFIG_KEY_TRUST_PACKED_REFS_STAT"/>
+            </message_arguments>
+        </filter>
+    </resource>
+    <resource path="src/org/eclipse/jgit/lib/CoreConfig.java" type="org.eclipse.jgit.lib.CoreConfig$TrustPackedRefsStat">
+        <filter id="1142947843">
+            <message_arguments>
+                <message_argument value="6.1.1"/>
+                <message_argument value="TrustPackedRefsStat"/>
+            </message_arguments>
+        </filter>
     </resource>
     <resource path="src/org/eclipse/jgit/lib/ObjectDatabase.java" type="org.eclipse.jgit.lib.ObjectDatabase">
         <filter id="336695337">

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java
index b46ffe3670532ca7e035ebcf488b3ec94b79acb7..43ebce3918cb0b77f2cb200aa9bf7b7e16d1d0a7 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java
@@ -30,6 +30,7 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.InterruptedIOException;
 import java.nio.file.DirectoryNotEmptyException;
@@ -60,6 +61,7 @@ import org.eclipse.jgit.events.RefsChangedEvent;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.CoreConfig.TrustPackedRefsStat;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectIdRef;
 import org.eclipse.jgit.lib.Ref;
@@ -892,10 +894,37 @@ public class RefDirectory extends RefDatabase {
                boolean trustFolderStat = getRepository().getConfig().getBoolean(
                                ConfigConstants.CONFIG_CORE_SECTION,
                                ConfigConstants.CONFIG_KEY_TRUSTFOLDERSTAT, true);
+               TrustPackedRefsStat trustPackedRefsStat =
+                               getRepository().getConfig().getEnum(
+                                               ConfigConstants.CONFIG_CORE_SECTION,
+                                               null,
+                                               ConfigConstants.CONFIG_KEY_TRUST_PACKED_REFS_STAT,
+                                               TrustPackedRefsStat.UNSET);
 
                final PackedRefList curList = packedRefs.get();
-               if (trustFolderStat && !curList.snapshot.isModified(packedRefsFile)) {
-                       return curList;
+
+               switch (trustPackedRefsStat) {
+               case NEVER:
+                       break;
+               case AFTER_OPEN:
+                       try (InputStream stream = Files
+                                       .newInputStream(packedRefsFile.toPath())) {
+                               // open the file to refresh attributes (on some NFS clients)
+                       } catch (FileNotFoundException e) {
+                               // Ignore as packed-refs may not exist
+                       }
+                       //$FALL-THROUGH$
+               case ALWAYS:
+                       if (!curList.snapshot.isModified(packedRefsFile)) {
+                               return curList;
+                       }
+                       break;
+               case UNSET:
+                       if (trustFolderStat
+                                       && !curList.snapshot.isModified(packedRefsFile)) {
+                               return curList;
+                       }
+                       break;
                }
 
                final PackedRefList newList = readPackedRefs();

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java
index 649bcde25a8db05f36080caa02e9a9571085754e..9398419af9a65354bdb2a0afd09f49038be94f53 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java
@@ -850,4 +850,10 @@ public final class ConfigConstants {
         */
        public static final String CONFIG_KEY_ABBREV = "abbrev";
 
+       /**
+        * The "trustPackedRefsStat" key
+        *
+        * @since 6.1.1
+        */
+       public static final String CONFIG_KEY_TRUST_PACKED_REFS_STAT = "trustPackedRefsStat";
 }

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/CoreConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/CoreConfig.java
index f23c6e08d1cfb38e8f77df8294fb88307e028945..fc82a5fead72d5c787b66ad6929de5d11c1c710a 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/CoreConfig.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/CoreConfig.java
@@ -116,6 +116,27 @@ public class CoreConfig {
                ALWAYS
        }
 
+       /**
+        * Permissible values for {@code core.trustPackedRefsStat}.
+        *
+        * @since 6.1.1
+        */
+       public enum TrustPackedRefsStat {
+               /** Do not trust file attributes of the packed-refs file. */
+               NEVER,
+
+               /** Trust file attributes of the packed-refs file. */
+               ALWAYS,
+
+               /** Open and close the packed-refs file to refresh its file attributes
+                * and then trust it. */
+               AFTER_OPEN,
+
+               /** {@code core.trustPackedRefsStat} defaults to this when it is
+                * not set */
+               UNSET
+       }
+
        private final int compression;
 
        private final int packIndexVersion;