Merge branch 'stable-6.10' into stable-7.0 * stable-6.10: Do not always refresh packed-refs during ref updates Change-Id: I4cdc1b80841481f83b64b800ef54da0b8ef8753a
diff --git a/Documentation/config-options.md b/Documentation/config-options.md index 78930e6..b30b958 100644 --- a/Documentation/config-options.md +++ b/Documentation/config-options.md
@@ -55,7 +55,7 @@ | `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, packed-refs file's and loose-objects folder'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. When looking for loose objects, if `false` and if a loose object is not found, JGit will open and close a stream to `.git/objects` folder (which can refresh its directory listing, at least on some NFS clients) and retry looking for that loose object. 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.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`. Note: since 6.10.2, this setting applies during both ref reads and ref updates, but previously only applied during reads.| | `core.trustLooseRefStat` | `always` | ⃞ | Whether to trust the file attributes (Java equivalent of stat command on *nix) of the loose ref. If `always` JGit will trust the file attributes of the loose ref and its parent directories. `after_open` behaves similar to `always`, except that all parent directories of the loose ref up to the repository root are opened and closed before its file attributes are considered. An open/close of these parent directories is known to refresh the 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. |
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java index 5584f13..4b5566e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java
@@ -172,7 +172,7 @@ public void execute(RevWalk walk, ProgressMonitor monitor, } packedRefsLock = refdb.lockPackedRefsOrThrow(); - PackedRefList oldPackedList = refdb.refreshPackedRefs(); + PackedRefList oldPackedList = refdb.getLockedPackedRefs(packedRefsLock); RefList<Ref> newRefs = applyUpdates(walk, oldPackedList, pending); if (newRefs == null) { return;
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 a9f0605..cc48176 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
@@ -702,7 +702,7 @@ void delete(RefDirectoryUpdate update) throws IOException { PackedRefList packed = getPackedRefs(); if (packed.contains(name)) { // Force update our packed-refs snapshot before writing - packed = refreshPackedRefs(); + packed = getLockedPackedRefs(lck); int idx = packed.find(name); if (0 <= idx) { commitPackedRefs(lck, packed.remove(idx), packed, true); @@ -770,7 +770,7 @@ private void pack(Collection<String> refs, try { LockFile lck = lockPackedRefsOrThrow(); try { - PackedRefList oldPacked = refreshPackedRefs(); + PackedRefList oldPacked = getLockedPackedRefs(lck); RefList<Ref> newPacked = oldPacked; // Iterate over all refs to be packed @@ -953,6 +953,15 @@ else if (0 <= (idx = packed.find(dst.getName()))) } PackedRefList getPackedRefs() throws IOException { + return refreshPackedRefsIfNeeded(); + } + + PackedRefList getLockedPackedRefs(LockFile packedRefsFileLock) throws IOException { + packedRefsFileLock.requireLock(); + return refreshPackedRefsIfNeeded(); + } + + PackedRefList refreshPackedRefsIfNeeded() throws IOException { PackedRefList curList = packedRefs.get(); if (!curList.shouldRefresh()) { return curList; @@ -967,23 +976,29 @@ private PackedRefsRefresher getPackedRefsRefresher(PackedRefList curList) return refresher; } // This synchronized is NOT needed for correctness. Instead it is used - // as a throttling mechanism to ensure that only one "read" thread does - // the work to refresh the file. In order to avoid stalling writes which - // must already be serialized and tend to be a bottleneck, - // the refreshPackedRefs() need not be synchronized. + // as a mechanism to try to avoid parallel reads of the same file content + // since repeating work, even in parallel, hurts performance. + // Unfortunately, this approach can still lead to some unnecessary re-reads + // during the "racy" window of the snapshot timestamp. synchronized (this) { if (packedRefsRefresher.get() != refresher) { refresher = packedRefsRefresher.get(); if (refresher != null) { - // Refresher now guaranteed to have been created after the - // current thread entered getPackedRefsRefresher(), even if - // it's currently out of date. + // Refresher now guaranteed to have not started refreshing until + // after the current thread entered getPackedRefsRefresher(), + // even if it's currently out of date. And if the packed-refs + // lock is held before calling this method, then it is also + // guaranteed to not be out-of date even during the "racy" + // window of the snapshot timestamp. return refresher; } } - refresher = createPackedRefsRefresherAsLatest(curList); + refresher = new PackedRefsRefresher(curList); + packedRefsRefresher.set(refresher); } - return runAndClear(refresher); + refresher.run(); + packedRefsRefresher.compareAndSet(refresher, null); + return refresher; } private boolean shouldRefreshPackedRefs(FileSnapshot snapshot) throws IOException { @@ -1012,23 +1027,6 @@ private boolean shouldRefreshPackedRefs(FileSnapshot snapshot) throws IOExceptio return true; } - PackedRefList refreshPackedRefs() throws IOException { - return runAndClear(createPackedRefsRefresherAsLatest(packedRefs.get())) - .getOrThrowIOException(); - } - - private PackedRefsRefresher createPackedRefsRefresherAsLatest(PackedRefList curList) { - PackedRefsRefresher refresher = new PackedRefsRefresher(curList); - packedRefsRefresher.set(refresher); - return refresher; - } - - private PackedRefsRefresher runAndClear(PackedRefsRefresher refresher) { - refresher.run(); - packedRefsRefresher.compareAndSet(refresher, null); - return refresher; - } - private PackedRefList refreshPackedRefs(PackedRefList curList) throws IOException { final PackedRefList newList = readPackedRefs();