From 82b5aaf7e3e3f881056dd2d4486e02537b0493da Mon Sep 17 00:00:00 2001 From: Kaushik Lingarkar Date: Fri, 2 Dec 2022 13:21:02 -0800 Subject: 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 --- org.eclipse.jgit/.settings/.api_filters | 14 +++++++++ .../jgit/internal/storage/file/RefDirectory.java | 33 ++++++++++++++++++++-- .../src/org/eclipse/jgit/lib/ConfigConstants.java | 6 ++++ .../src/org/eclipse/jgit/lib/CoreConfig.java | 21 ++++++++++++++ 4 files changed, 72 insertions(+), 2 deletions(-) (limited to 'org.eclipse.jgit') diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index a6f18310d6..0f84330082 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -15,6 +15,20 @@ + + + + + + + + + + + + + + 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 b46ffe3670..43ebce3918 100644 --- 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 649bcde25a..9398419af9 100644 --- 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 f23c6e08d1..fc82a5fead 100644 --- 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; -- cgit v1.2.3