FileReftableStack: use FileSnapshot to detect modification

Reading file attributes is faster than reading file content hence use
FileSnapshot to speedup detecting if FileReftableStack is up-to-date.

Introduce new option "core.trustTablesListStat" allowing to configure if
we can trust file attributes of the "tables.list" file to speedup
detection of file modifications. This file is used to store the list of
filenames of the files storing Reftables in FileReftableDatabase.

If this option is set to "ALWAYS" we trust File attributes and use them
to speedup detection of file modifications.

If set to "NEVER" the content of the "tables.list" file is always read
unconditionally. This can help to avoid caching issues on some
filesystems.

If set to "AFTER_OPEN" we will open a FileInputStream to refresh File
attributes of the "tables.list" file before relying on the refreshed
File attributes to detect modifications. This works on some NFS
filesystems and is faster than using "NEVER".

Change-Id: I3e288d90fb07edf4fa2a03c707a333b26f0c458d

4 files changed, 71 insertions, 4 deletions

diff --git a/Documentation/config-options.md b/Documentation/config-options.md
index 1dfe95f850..4dde8f8c15 100644
--- a/Documentation/config-options.md
+++ b/Documentation/config-options.md
@@ -58,9 +58,10 @@ For details on native git options see also the official [git config documentatio
 | ~~`core.trustFolderStat`~~ | `true` | ⃞ | __Deprecated__, use `core.trustStat` instead. If set to `true` translated to `core.trustStat=always`, if `false` translated to `core.trustStat=never`, see below. If both `core.trustFolderStat` and `core.trustStat` are configured then `trustStat` takes precedence and `trustFolderStat` is ignored. |
 | `core.trustLooseRefStat` | `inherit` | ⃞ | Whether to trust the file attributes of loose refs and its fan-out parent directory. See `core.trustStat` for possible values. If `inherit`, JGit will use the behavior configured in `trustStat`. |
 | `core.trustPackedRefsStat` | `inherit` | ⃞ | Whether to trust the file attributes of the packed-refs file. See `core.trustStat` for possible values. If `inherit`, JGit will use the behavior configured in `core.trustStat`. |
+| `core.trustTablesListStat` | `inherit` | ⃞ | Whether to trust the file attributes of the `tables.list` file used by the reftable ref storage backend to store the list of reftable filenames. See `core.trustStat` for possible values. If `inherit`, JGit will use the behavior configured in `core.trustStat`.  The reftable backend is used if `extensions.refStorage = reftable`. |
 | `core.trustLooseObjectStat` | `inherit` | ⃞ | Whether to trust the file attributes of the loose object file and its fan-out parent directory. See `core.trustStat` for possible values. If `inherit`, JGit will use the behavior configured in `core.trustStat`. |
 | `core.trustPackStat` | `inherit` | ⃞ | Whether to trust the file attributes of the `objects/pack` directory. See `core.trustStat` for possible values. If `inherit`, JGit will use the behavior configured in `core.trustStat`. |
-| `core.trustStat` | `always` | ⃞ | Global option to configure whether to trust file attributes (Java equivalent of stat command on Unix) of files storing git objects. Can be overridden for specific files by configuring `core.trustLooseRefStat, core.trustPackedRefsStat, core.trustLooseObjectStat, core.trustPackStat`. If `never` JGit will ignore the file attributes of the file and always read it. If `always` JGit will trust the file attributes and will only read it if a file attribute has changed. `after_open` behaves the same as `always`, but file attributes are only considered *after* the file itself and any transient parent directories have been opened and closed. An open/close of the file/directory is known to refresh its file attributes, at least on some NFS clients. |
+| `core.trustStat` | `always` | ⃞ | Global option to configure whether to trust file attributes (Java equivalent of stat command on Unix) of files storing git objects. Can be overridden for specific files by configuring `core.trustLooseRefStat, core.trustPackedRefsStat, core.trustLooseObjectStat, core.trustPackStat,core.trustTablesListStat`. If `never` JGit will ignore the file attributes of the file and always read it. If `always` JGit will trust the file attributes and will only read it if a file attribute has changed. `after_open` behaves the same as `always`, but file attributes are only considered *after* the file itself and any transient parent directories have been opened and closed. An open/close of the file/directory is known to refresh its file attributes, at least on some NFS clients. |
 | `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. |
 
 ## __fetch__ options
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java
index 0f5ff0f9f7..b2c88922b8 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java
@@ -18,8 +18,10 @@ import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
 import java.nio.file.StandardCopyOption;
 import java.security.SecureRandom;
 import java.util.ArrayList;
@@ -27,6 +29,7 @@ import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
@@ -39,6 +42,8 @@ import org.eclipse.jgit.internal.storage.reftable.ReftableConfig;
 import org.eclipse.jgit.internal.storage.reftable.ReftableReader;
 import org.eclipse.jgit.internal.storage.reftable.ReftableWriter;
 import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.CoreConfig;
+import org.eclipse.jgit.lib.CoreConfig.TrustStat;
 import org.eclipse.jgit.util.FileUtils;
 import org.eclipse.jgit.util.SystemReader;
 
@@ -59,6 +64,9 @@ public class FileReftableStack implements AutoCloseable {
 
 	private List<StackEntry> stack;
 
+	private AtomicReference<FileSnapshot> snapshot = new AtomicReference<>(
+			FileSnapshot.DIRTY);
+
 	private long lastNextUpdateIndex;
 
 	private final File stackPath;
@@ -98,6 +106,8 @@ public class FileReftableStack implements AutoCloseable {
 
 	private final CompactionStats stats;
 
+	private final TrustStat trustTablesListStat;
+
 	/**
 	 * Creates a stack corresponding to the list of reftables in the argument
 	 *
@@ -126,6 +136,8 @@ public class FileReftableStack implements AutoCloseable {
 		reload();
 
 		stats = new CompactionStats();
+		trustTablesListStat = configSupplier.get().get(CoreConfig.KEY)
+				.getTrustTablesListStat();
 	}
 
 	CompactionStats getStats() {
@@ -272,8 +284,9 @@ public class FileReftableStack implements AutoCloseable {
 	}
 
 	private List<String> readTableNames() throws IOException {
+		FileSnapshot old;
 		List<String> names = new ArrayList<>(stack.size() + 1);
-
+		old = snapshot.get();
 		try (BufferedReader br = new BufferedReader(
 				new InputStreamReader(new FileInputStream(stackPath), UTF_8))) {
 			String line;
@@ -282,8 +295,10 @@ public class FileReftableStack implements AutoCloseable {
 					names.add(line);
 				}
 			}
+			snapshot.compareAndSet(old, FileSnapshot.save(stackPath));
 		} catch (FileNotFoundException e) {
 			// file isn't there: empty repository.
+			snapshot.compareAndSet(old, FileSnapshot.MISSING_FILE);
 		}
 		return names;
 	}
@@ -294,9 +309,28 @@ public class FileReftableStack implements AutoCloseable {
 	 *             on IO problem
 	 */
 	boolean isUpToDate() throws IOException {
-		// We could use FileSnapshot to avoid reading the file, but the file is
-		// small so it's probably a minor optimization.
 		try {
+			switch (trustTablesListStat) {
+			case NEVER:
+				break;
+			case AFTER_OPEN:
+				try (InputStream stream = Files
+						.newInputStream(stackPath.toPath())) {
+					// open the tables.list file to refresh attributes (on some
+					// NFS clients)
+				} catch (FileNotFoundException | NoSuchFileException e) {
+					// ignore
+				}
+				//$FALL-THROUGH$
+			case ALWAYS:
+				if (!snapshot.get().isModified(stackPath)) {
+					return true;
+				}
+				break;
+			case INHERIT:
+				// only used in CoreConfig internally
+				throw new IllegalStateException();
+			}
 			List<String> names = readTableNames();
 			if (names.size() != stack.size()) {
 				return false;
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 f62bf161a7..c4550329d3 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java
@@ -1045,6 +1045,14 @@ public final class ConfigConstants {
 	 * @since 7.2
 	 */
 	public static final String CONFIG_KEY_TRUST_LOOSE_OBJECT_STAT = "trustLooseObjectStat";
+
+	/**
+	 * The "trustTablesListStat" key
+	 *
+	 * @since 7.2
+	 */
+	public static final String CONFIG_KEY_TRUST_TABLESLIST_STAT = "trustTablesListStat";
+
 	/**
 	 * The "pack.preserveOldPacks" key
 	 *
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 e43c9653dd..0e27b2743c 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/CoreConfig.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/CoreConfig.java
@@ -215,6 +215,8 @@ public class CoreConfig {
 
 	private final TrustStat trustLooseObjectStat;
 
+	private final TrustStat trustTablesListStat;
+
 	/**
 	 * Options for symlink handling
 	 *
@@ -268,6 +270,7 @@ public class CoreConfig {
 		trustLooseRefStat = parseTrustLooseRefStat(rc);
 		trustPackStat = parseTrustPackFileStat(rc);
 		trustLooseObjectStat = parseTrustLooseObjectFileStat(rc);
+		trustTablesListStat = parseTablesListStat(rc);
 	}
 
 	private static TrustStat parseTrustStat(Config rc) {
@@ -318,6 +321,13 @@ public class CoreConfig {
 		return t == TrustStat.INHERIT ? trustStat : t;
 	}
 
+	private TrustStat parseTablesListStat(Config rc) {
+		TrustStat t = rc.getEnum(ConfigConstants.CONFIG_CORE_SECTION, null,
+				ConfigConstants.CONFIG_KEY_TRUST_TABLESLIST_STAT,
+				TrustStat.INHERIT);
+		return t == TrustStat.INHERIT ? trustStat : t;
+	}
+
 	/**
 	 * Get the compression level to use when storing loose objects
 	 *
@@ -418,4 +428,18 @@ public class CoreConfig {
 	public TrustStat getTrustLooseObjectStat() {
 		return trustLooseObjectStat;
 	}
+
+	/**
+	 * Get how far we can trust file attributes of the "tables.list" file which
+	 * is used to store the list of filenames of the files storing
+	 * {@link org.eclipse.jgit.internal.storage.reftable.Reftable}s in
+	 * {@link org.eclipse.jgit.internal.storage.file.FileReftableDatabase}.
+	 *
+	 * @return how far we can trust file attributes of the "tables.list" file.
+	 *
+	 * @since 7.2
+	 */
+	public TrustStat getTrustTablesListStat() {
+		return trustTablesListStat;
+	}
 }