Pack: Replace extensions bitset with bitmapIdx PackFile
The only extension that was ever consulted from the bitmap was the
bitmap index. We can simplify the Pack code as well as the code of
all the callers if we focus on just that usage.
Change-Id: I799ddfdee93142af67ce5081d14a430d36aa4c15
Signed-off-by: Nasser Grainawi <quic_nasserg@quicinc.com>
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackTest.java
index 85bd31d..a359654 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/PackTest.java
@@ -261,7 +261,7 @@
new PackIndexWriterV1(f).write(list, footer);
}
- Pack pack = new Pack(packName, PackExt.INDEX.getBit());
+ Pack pack = new Pack(packName, null);
try {
pack.get(wc, b);
fail("expected LargeObjectException.ExceedsByteArrayLimit");
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
index e71a960..170df57 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ObjectDirectory.java
@@ -11,8 +11,8 @@
package org.eclipse.jgit.internal.storage.file;
import static java.nio.charset.StandardCharsets.UTF_8;
-import static org.eclipse.jgit.internal.storage.pack.PackExt.INDEX;
import static org.eclipse.jgit.internal.storage.pack.PackExt.PACK;
+import static org.eclipse.jgit.internal.storage.pack.PackExt.BITMAP_INDEX;
import java.io.BufferedReader;
import java.io.File;
@@ -31,7 +31,6 @@
import org.eclipse.jgit.internal.JGitText;
import org.eclipse.jgit.internal.storage.pack.ObjectToPack;
-import org.eclipse.jgit.internal.storage.pack.PackExt;
import org.eclipse.jgit.internal.storage.pack.PackWriter;
import org.eclipse.jgit.lib.AbbreviatedObjectId;
import org.eclipse.jgit.lib.AnyObjectId;
@@ -216,26 +215,26 @@
* Add a single existing pack to the list of available pack files.
*/
@Override
- public Pack openPack(File pack)
- throws IOException {
- final String p = pack.getName();
- if (p.length() != 50 || !p.startsWith("pack-") || !p.endsWith(".pack")) //$NON-NLS-1$ //$NON-NLS-2$
- throw new IOException(MessageFormat.format(JGitText.get().notAValidPack, pack));
-
- // The pack and index are assumed to exist. The existence of other
- // extensions needs to be explicitly checked.
- //
- int extensions = PACK.getBit() | INDEX.getBit();
- final String base = p.substring(0, p.length() - 4);
- for (PackExt ext : PackExt.values()) {
- if ((extensions & ext.getBit()) == 0) {
- final String name = base + ext.getExtension();
- if (new File(pack.getParentFile(), name).exists())
- extensions |= ext.getBit();
- }
+ public Pack openPack(File pack) throws IOException {
+ PackFile pf;
+ try {
+ pf = new PackFile(pack);
+ } catch (IllegalArgumentException e) {
+ throw new IOException(
+ MessageFormat.format(JGitText.get().notAValidPack, pack),
+ e);
}
- Pack res = new Pack(pack, extensions);
+ String p = pf.getName();
+ // TODO(nasserg): See if PackFile can do these checks instead
+ if (p.length() != 50 || !p.startsWith("pack-") //$NON-NLS-1$
+ || !pf.getPackExt().equals(PACK)) {
+ throw new IOException(
+ MessageFormat.format(JGitText.get().notAValidPack, pack));
+ }
+
+ PackFile bitmapIdx = pf.create(BITMAP_INDEX);
+ Pack res = new Pack(pack, bitmapIdx.exists() ? bitmapIdx : null);
packed.insert(res);
return res;
}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java
index fa938b3..5efd4c5 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/Pack.java
@@ -12,7 +12,6 @@
package org.eclipse.jgit.internal.storage.file;
-import static org.eclipse.jgit.internal.storage.pack.PackExt.BITMAP_INDEX;
import static org.eclipse.jgit.internal.storage.pack.PackExt.INDEX;
import static org.eclipse.jgit.internal.storage.pack.PackExt.KEEP;
@@ -38,6 +37,7 @@
import java.util.zip.DataFormatException;
import java.util.zip.Inflater;
+import org.eclipse.jgit.annotations.Nullable;
import org.eclipse.jgit.errors.CorruptObjectException;
import org.eclipse.jgit.errors.LargeObjectException;
import org.eclipse.jgit.errors.MissingObjectException;
@@ -51,7 +51,6 @@
import org.eclipse.jgit.internal.JGitText;
import org.eclipse.jgit.internal.storage.pack.BinaryDelta;
import org.eclipse.jgit.internal.storage.pack.ObjectToPack;
-import org.eclipse.jgit.internal.storage.pack.PackExt;
import org.eclipse.jgit.internal.storage.pack.PackOutputStream;
import org.eclipse.jgit.lib.AbbreviatedObjectId;
import org.eclipse.jgit.lib.AnyObjectId;
@@ -80,8 +79,6 @@
private final PackFile packFile;
- private final int extensions;
-
private PackFile keepFile;
final int hash;
@@ -105,7 +102,8 @@
private volatile Exception invalidatingCause;
- private boolean invalidBitmap;
+ @Nullable
+ private PackFile bitmapIdxFile;
private AtomicInteger transientErrorCount = new AtomicInteger();
@@ -131,14 +129,14 @@
*
* @param packFile
* path of the <code>.pack</code> file holding the data.
- * @param extensions
- * additional pack file extensions with the same base as the pack
+ * @param bitmapIdxFile
+ * existing bitmap index file with the same base as the pack
*/
- public Pack(File packFile, int extensions) {
+ public Pack(File packFile, @Nullable PackFile bitmapIdxFile) {
this.packFile = new PackFile(packFile);
this.fileSnapshot = PackFileSnapshot.save(packFile);
this.packLastModified = fileSnapshot.lastModifiedInstant();
- this.extensions = extensions;
+ this.bitmapIdxFile = bitmapIdxFile;
// Multiply by 31 here so we can more directly combine with another
// value in WindowCache.hash(), without doing the multiply there.
@@ -1124,26 +1122,28 @@
}
synchronized PackBitmapIndex getBitmapIndex() throws IOException {
- if (invalid || invalidBitmap)
+ if (invalid || bitmapIdxFile == null) {
return null;
- if (bitmapIdx == null && hasExt(BITMAP_INDEX)) {
+ }
+ if (bitmapIdx == null) {
final PackBitmapIndex idx;
try {
- idx = PackBitmapIndex.open(packFile.create(BITMAP_INDEX), idx(),
+ idx = PackBitmapIndex.open(bitmapIdxFile, idx(),
getReverseIdx());
} catch (FileNotFoundException e) {
// Once upon a time this bitmap file existed. Now it
// has been removed. Most likely an external gc has
// removed this packfile and the bitmap
- invalidBitmap = true;
- return null;
+ bitmapIdxFile = null;
+ return null;
}
// At this point, idx() will have set packChecksum.
- if (Arrays.equals(packChecksum, idx.packChecksum))
+ if (Arrays.equals(packChecksum, idx.packChecksum)) {
bitmapIdx = idx;
- else
- invalidBitmap = true;
+ } else {
+ bitmapIdxFile = null;
+ }
}
return bitmapIdx;
}
@@ -1179,10 +1179,6 @@
}
}
- private boolean hasExt(PackExt ext) {
- return (extensions & ext.getBit()) != 0;
- }
-
@SuppressWarnings("nls")
@Override
public String toString() {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java
index 2e68d46..007205e 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackDirectory.java
@@ -421,10 +421,7 @@
continue;
}
- list.add(new Pack(packFile,
- packFilesByExt.containsKey(BITMAP_INDEX)
- ? BITMAP_INDEX.getBit()
- : 0));
+ list.add(new Pack(packFile, packFilesByExt.get(BITMAP_INDEX)));
foundNew = true;
}
