From 16760c3e9a242f21538c2f4da1aad96953cafc37 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Fri, 21 Jun 2019 18:12:14 +0200 Subject: [PATCH] Persist filesystem timestamp resolution and allow manual configuration To enable persisting filesystem timestamp resolution per FileStore add a new config section to the user global git configuration: - Config section is "filesystem" - Config subsection is concatenation of - Java vendor (system property "java.vm.vendor") - runtime version (system property "java.vm.version") - FileStore's name - separated by '|' e.g. "AdoptOpenJDK|1.8.0_212-b03|/dev/disk1s1" The prefix is needed since some Java versions do not expose the full timestamp resolution of the underlying filesystem. This may also depend on the underlying operating system hence concrete key values may not be portable. - Config key for timestamp resolution is "timestampResolution" as a time value, supported time units are those supported by DefaultTypedConfigGetter#getTimeUnit If timestamp resolution is already configured for a given FileStore the configured value is used instead of measuring the resolution. When timestamp resolution was measured it is persisted in the user global git configuration. Example: [filesystem "AdoptOpenJDK|1.8.0_212-b03|/dev/disk1s1"] timestampResolution = 1 seconds If locking the git config file fails retry saving the resolution up to 5 times in order to workaround races with another thread. In order to avoid stack overflow use the fallback filesystem timestamp resolution when loading FileBasedConfig which creates itself a FileSnapshot to help checking if the config changed. Note: - on some OSes Java 8,9 truncate to milliseconds or seconds, see https://bugs.openjdk.java.net/browse/JDK-8177809, fixed in Java 10 - UnixFileAttributes up to Java 12 truncates timestamp resolution to microseconds when converting the internal representation to FileTime exposed in the API, see https://bugs.openjdk.java.net/browse/JDK-8181493 - WindowsFileAttributes also provides only microsecond resolution up to Java 12 Hence do not attempt to manually configure a higher timestamp resolution than supported by the Java version being used at runtime. Bug: 546891 Bug: 548188 Change-Id: Iff91b8f9e6e5e2295e1463f87c8e95edf4abbcf8 Signed-off-by: Matthias Sohn --- org.eclipse.jgit/.settings/.api_filters | 22 ++++ .../eclipse/jgit/internal/JGitText.properties | 2 + .../org/eclipse/jgit/internal/JGitText.java | 2 + .../internal/storage/file/FileSnapshot.java | 40 ++++++- .../org/eclipse/jgit/lib/ConfigConstants.java | 12 +++ .../src/org/eclipse/jgit/lib/Constants.java | 11 ++ .../jgit/storage/file/FileBasedConfig.java | 4 +- .../src/org/eclipse/jgit/util/FS.java | 101 ++++++++++++++++-- 8 files changed, 183 insertions(+), 11 deletions(-) diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index d4c40788d6..d313e92a05 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -22,6 +22,28 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index 0a2f7029ed..9af6cce59d 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -104,6 +104,7 @@ cannotReadObjectsPath=Cannot read {0}/{1}: {2} cannotReadTree=Cannot read tree {0} cannotRebaseWithoutCurrentHead=Can not rebase without a current HEAD cannotResolveLocalTrackingRefForUpdating=Cannot resolve local tracking ref {0} for updating. +cannotSaveConfig=Cannot save config file ''{0}'' cannotSquashFixupWithoutPreviousCommit=Cannot {0} without previous commit. cannotStoreObjects=cannot store objects cannotResolveUniquelyAbbrevObjectId=Could not resolve uniquely the abbreviated object ID @@ -557,6 +558,7 @@ pushIsNotSupportedForBundleTransport=Push is not supported for bundle transport pushNotPermitted=push not permitted pushOptionsNotSupported=Push options not supported; received {0} rawLogMessageDoesNotParseAsLogEntry=Raw log message does not parse as log entry +readConfigFailed=Reading config file ''{0}'' failed readerIsRequired=Reader is required readingObjectsFromLocalRepositoryFailed=reading objects from local repository failed: {0} readTimedOut=Read timed out after {0} ms diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index bcd6d5ce27..79133d203c 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -165,6 +165,7 @@ public class JGitText extends TranslationBundle { /***/ public String cannotReadTree; /***/ public String cannotRebaseWithoutCurrentHead; /***/ public String cannotResolveLocalTrackingRefForUpdating; + /***/ public String cannotSaveConfig; /***/ public String cannotSquashFixupWithoutPreviousCommit; /***/ public String cannotStoreObjects; /***/ public String cannotResolveUniquelyAbbrevObjectId; @@ -618,6 +619,7 @@ public class JGitText extends TranslationBundle { /***/ public String pushNotPermitted; /***/ public String pushOptionsNotSupported; /***/ public String rawLogMessageDoesNotParseAsLogEntry; + /***/ public String readConfigFailed; /***/ public String readerIsRequired; /***/ public String readingObjectsFromLocalRepositoryFailed; /***/ public String readTimedOut; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java index 0019c5f090..2c874ff594 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java @@ -43,6 +43,7 @@ package org.eclipse.jgit.internal.storage.file; +import static org.eclipse.jgit.lib.Constants.FALLBACK_TIMESTAMP_RESOLUTION; import java.io.File; import java.io.IOException; import java.nio.file.attribute.BasicFileAttributes; @@ -122,6 +123,22 @@ public class FileSnapshot { return new FileSnapshot(path); } + /** + * Record a snapshot for a specific file path without using config file to + * get filesystem timestamp resolution. + *

+ * This method should be invoked before the file is accessed. It is used by + * FileBasedConfig to avoid endless recursion. + * + * @param path + * the path to later remember. The path's current status + * information is saved. + * @return the snapshot. + */ + public static FileSnapshot saveNoConfig(File path) { + return new FileSnapshot(path); + } + private static Object getFileKey(BasicFileAttributes fileAttributes) { Object fileKey = fileAttributes.fileKey(); return fileKey == null ? MISSING_FILEKEY : fileKey; @@ -177,13 +194,30 @@ public class FileSnapshot { * This method should be invoked before the file is accessed. * * @param path - * the path to later remember. The path's current status + * the path to remember meta data for. The path's current status * information is saved. */ protected FileSnapshot(File path) { + this(path, true); + } + + /** + * Record a snapshot for a specific file path. + *

+ * This method should be invoked before the file is accessed. + * + * @param path + * the path to remember meta data for. The path's current status + * information is saved. + * @param useConfig + * if {@code true} read filesystem time resolution from + * configuration file otherwise use fallback resolution + */ + protected FileSnapshot(File path, boolean useConfig) { this.lastRead = System.currentTimeMillis(); - this.fsTimestampResolution = FS - .getFsTimerResolution(path.toPath().getParent()); + this.fsTimestampResolution = useConfig + ? FS.getFsTimerResolution(path.toPath().getParent()) + : FALLBACK_TIMESTAMP_RESOLUTION; BasicFileAttributes fileAttributes = null; try { fileAttributes = FS.DETECTED.fileAttributes(path); 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 d4a0280da6..4f636d4553 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ConfigConstants.java @@ -432,4 +432,16 @@ public final class ConfigConstants { * @since 4.11 */ public static final String CONFIG_SECTION_LFS = "lfs"; + + /** + * The "filesystem" section + * @since 5.1.9 + */ + public static final String CONFIG_FILESYSTEM_SECTION = "filesystem"; + + /** + * The "timestampResolution" key + * @since 5.1.9 + */ + public static final String CONFIG_KEY_TIMESTAMP_RESOLUTION = "timestampResolution"; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java index 4c55196961..94fc100386 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java @@ -52,6 +52,7 @@ import java.nio.charset.Charset; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.text.MessageFormat; +import java.time.Duration; import org.eclipse.jgit.errors.CorruptObjectException; import org.eclipse.jgit.internal.JGitText; @@ -722,6 +723,16 @@ public final class Constants { */ public static final String LOCK_SUFFIX = ".lock"; //$NON-NLS-1$ + /** + * Fallback filesystem timestamp resolution used when we can't measure the + * resolution. The last modified time granularity of FAT filesystems is 2 + * seconds. + * + * @since 5.1.9 + */ + public static final Duration FALLBACK_TIMESTAMP_RESOLUTION = Duration + .ofMillis(2000); + private Constants() { // Hide the default constructor } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java index 93b3baa61f..3a41643e6e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileBasedConfig.java @@ -153,7 +153,9 @@ public class FileBasedConfig extends StoredConfig { int retries = 0; while (true) { final FileSnapshot oldSnapshot = snapshot; - final FileSnapshot newSnapshot = FileSnapshot.save(getFile()); + // don't use config in this snapshot to avoid endless recursion + final FileSnapshot newSnapshot = FileSnapshot + .saveNoConfig(getFile()); try { final byte[] in = IO.readFully(getFile()); final ObjectId newHash = hash(in); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java index 2b4e5c78d8..687c7a4fdd 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java @@ -44,6 +44,7 @@ package org.eclipse.jgit.util; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.eclipse.jgit.lib.Constants.FALLBACK_TIMESTAMP_RESOLUTION; import java.io.BufferedReader; import java.io.ByteArrayInputStream; @@ -87,9 +88,13 @@ import org.eclipse.jgit.annotations.NonNull; import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.api.errors.JGitInternalException; import org.eclipse.jgit.errors.CommandFailedException; +import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.errors.LockFailedException; import org.eclipse.jgit.internal.JGitText; +import org.eclipse.jgit.lib.ConfigConstants; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.storage.file.FileBasedConfig; import org.eclipse.jgit.treewalk.FileTreeIterator.FileEntry; import org.eclipse.jgit.treewalk.FileTreeIterator.FileModeStrategy; import org.eclipse.jgit.treewalk.WorkingTreeIterator.Entry; @@ -193,11 +198,9 @@ public abstract class FS { } private static final class FileStoreAttributeCache { - /** - * The last modified time granularity of FAT filesystems is 2 seconds. - */ - private static final Duration FALLBACK_TIMESTAMP_RESOLUTION = Duration - .ofMillis(2000); + + private static final Duration UNDEFINED_RESOLUTION = Duration + .ofNanos(Long.MAX_VALUE); private static final Map attributeCache = new ConcurrentHashMap<>(); @@ -209,6 +212,10 @@ public abstract class FS { background.set(async); } + private static final String javaVersionPrefix = System + .getProperty("java.vm.vendor") + '|' //$NON-NLS-1$ + + System.getProperty("java.vm.version") + '|'; //$NON-NLS-1$ + private static Duration getFsTimestampResolution(Path file) { Path dir = Files.isDirectory(file) ? file : file.getParent(); FileStore s; @@ -280,6 +287,10 @@ public abstract class FS { private static Optional measureFsTimestampResolution( FileStore s, Path dir) { + Duration configured = readFileTimeResolution(s); + if (!UNDEFINED_RESOLUTION.equals(configured)) { + return Optional.of(configured); + } Path probe = dir.resolve(".probe-" + UUID.randomUUID()); //$NON-NLS-1$ try { Files.createFile(probe); @@ -296,8 +307,9 @@ public abstract class FS { wait = wait * 2; } } - return Optional - .of(Duration.between(t1.toInstant(), t2.toInstant())); + Duration resolution = Duration.between(t1.toInstant(), t2.toInstant()); + saveFileTimeResolution(s, resolution); + return Optional.of(resolution); } catch (IOException | TimeoutException e) { LOG.error(e.getLocalizedMessage(), e); } catch (InterruptedException e) { @@ -328,6 +340,81 @@ public abstract class FS { } } + private static Duration readFileTimeResolution(FileStore s) { + FileBasedConfig userConfig = SystemReader.getInstance() + .openUserConfig(null, FS.DETECTED); + try { + userConfig.load(); + } catch (IOException e) { + LOG.error(MessageFormat.format(JGitText.get().readConfigFailed, + userConfig.getFile().getAbsolutePath()), e); + } catch (ConfigInvalidException e) { + LOG.error(MessageFormat.format( + JGitText.get().repositoryConfigFileInvalid, + userConfig.getFile().getAbsolutePath(), + e.getMessage())); + } + Duration configured = Duration + .ofNanos(userConfig.getTimeUnit( + ConfigConstants.CONFIG_FILESYSTEM_SECTION, + javaVersionPrefix + s.name(), + ConfigConstants.CONFIG_KEY_TIMESTAMP_RESOLUTION, + UNDEFINED_RESOLUTION.toNanos(), + TimeUnit.NANOSECONDS)); + return configured; + } + + private static void saveFileTimeResolution(FileStore s, + Duration resolution) { + FileBasedConfig userConfig = SystemReader.getInstance() + .openUserConfig(null, FS.DETECTED); + long nanos = resolution.toNanos(); + TimeUnit unit; + if (nanos < 200_000L) { + unit = TimeUnit.NANOSECONDS; + } else if (nanos < 200_000_000L) { + unit = TimeUnit.MICROSECONDS; + } else { + unit = TimeUnit.MILLISECONDS; + } + + final int max_retries = 5; + int retries = 0; + boolean succeeded = false; + long value = unit.convert(nanos, TimeUnit.NANOSECONDS); + while (!succeeded && retries < max_retries) { + try { + userConfig.load(); + userConfig.setString( + ConfigConstants.CONFIG_FILESYSTEM_SECTION, + javaVersionPrefix + s.name(), + ConfigConstants.CONFIG_KEY_TIMESTAMP_RESOLUTION, + String.format("%d %s", //$NON-NLS-1$ + Long.valueOf(value), + unit.name().toLowerCase())); + userConfig.save(); + succeeded = true; + } catch (LockFailedException e) { + // race with another thread, wait a bit and try again + try { + retries++; + Thread.sleep(20); + } catch (InterruptedException e1) { + Thread.interrupted(); + } + } catch (IOException e) { + LOG.error(MessageFormat.format( + JGitText.get().cannotSaveConfig, + userConfig.getFile().getAbsolutePath()), e); + } catch (ConfigInvalidException e) { + LOG.error(MessageFormat.format( + JGitText.get().repositoryConfigFileInvalid, + userConfig.getFile().getAbsolutePath(), + e.getMessage())); + } + } + } + private final @NonNull Duration fsTimestampResolution; @NonNull -- 2.39.5