From 19c43806890a5b7556d4d12b87f7ba2bb878cbf8 Mon Sep 17 00:00:00 2001 From: Juergen Denner Date: Tue, 15 Jan 2019 13:22:54 +0100 Subject: [PATCH] Reduce contention on PackFile.idx() function. In case of concurrent pack file access, threads may wait on the idx() function even for already open files. This happens especially with a slow file system. Performance numbers are listed in the bug report. Bug: 543739 Change-Id: Iff328d347fa65ae07ecce3267d44184161248978 Signed-off-by: Juergen Denner Signed-off-by: Matthias Sohn --- .../jgit/internal/storage/file/PackFile.java | 59 +++++++++++-------- 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java index d28c04fd6..3e5ef21cc 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackFile.java @@ -139,7 +139,7 @@ public int compare(final PackFile a, final PackFile b) { private byte[] packChecksum; - private PackIndex loadedIdx; + private volatile PackIndex loadedIdx; private PackReverseIndex reverseIdx; @@ -174,35 +174,44 @@ public PackFile(final File packFile, int extensions) { length = Long.MAX_VALUE; } - private synchronized PackIndex idx() throws IOException { - if (loadedIdx == null) { - if (invalid) - throw new PackInvalidException(packFile); + private PackIndex idx() throws IOException { + PackIndex idx = loadedIdx; + if (idx == null) { + synchronized (this) { + idx = loadedIdx; + if (idx == null) { + if (invalid) { + throw new PackInvalidException(packFile); + } + try { + idx = PackIndex.open(extFile(INDEX)); - try { - final PackIndex idx = PackIndex.open(extFile(INDEX)); - - if (packChecksum == null) { - packChecksum = idx.packChecksum; - } else if (!Arrays.equals(packChecksum, idx.packChecksum)) { - throw new PackMismatchException(MessageFormat.format( - JGitText.get().packChecksumMismatch, - packFile.getPath(), - ObjectId.fromRaw(packChecksum).name(), - ObjectId.fromRaw(idx.packChecksum).name())); + if (packChecksum == null) { + packChecksum = idx.packChecksum; + } else if (!Arrays.equals(packChecksum, + idx.packChecksum)) { + throw new PackMismatchException(MessageFormat + .format(JGitText.get().packChecksumMismatch, + packFile.getPath(), + ObjectId.fromRaw(packChecksum) + .name(), + ObjectId.fromRaw(idx.packChecksum) + .name())); + } + loadedIdx = idx; + } catch (InterruptedIOException e) { + // don't invalidate the pack, we are interrupted from + // another thread + throw e; + } catch (IOException e) { + invalid = true; + throw e; + } } - loadedIdx = idx; - } catch (InterruptedIOException e) { - // don't invalidate the pack, we are interrupted from another thread - throw e; - } catch (IOException e) { - invalid = true; - throw e; } } - return loadedIdx; + return idx; } - /** @return the File object which locates this pack on disk. */ public File getPackFile() { return packFile;