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

Author: msohn

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

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileReftableStack.java

@@ -18,15 +18,18 @@
 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;
 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.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 @@ private static class StackEntry {
 
 	private List<StackEntry> stack;
 
+	private AtomicReference<FileSnapshot> snapshot = new AtomicReference<>(
+			FileSnapshot.DIRTY);
+
 	private long lastNextUpdateIndex;
 
 	private final File stackPath;
@@ -98,6 +106,8 @@ static class CompactionStats {
 
 	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 FileReftableStack(File stackPath, File reftableDir,
 		reload();
 
 		stats = new CompactionStats();
+		trustTablesListStat = configSupplier.get().get(CoreConfig.KEY)
+				.getTrustTablesListStat();
 	}
 
 	CompactionStats getStats() {
@@ -272,8 +284,9 @@ public interface Writer {
 	}
 
 	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 @@ private List<String> readTableNames() throws IOException {
 					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 @@ private List<String> readTableNames() throws IOException {
 	 *             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;

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
 	 *

org.eclipse.jgit/src/org/eclipse/jgit/lib/CoreConfig.java

@@ -215,6 +215,8 @@ public enum TrustStat {
 
 	private final TrustStat trustLooseObjectStat;
 
+	private final TrustStat trustTablesListStat;
+
 	/**
 	 * Options for symlink handling
 	 *
@@ -268,6 +270,7 @@ public enum HideDotFiles {
 		trustLooseRefStat = parseTrustLooseRefStat(rc);
 		trustPackStat = parseTrustPackFileStat(rc);
 		trustLooseObjectStat = parseTrustLooseObjectFileStat(rc);
+		trustTablesListStat = parseTablesListStat(rc);
 	}
 
 	private static TrustStat parseTrustStat(Config rc) {
@@ -318,6 +321,13 @@ private TrustStat inheritParseTrustStat(Config rc, String key) {
 		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 TrustStat getTrustPackStat() {
 	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;
+	}
 }