From 683bd09092e90aef5b7cf963355995d76aefa439 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Mon, 9 Nov 2015 13:14:13 -0800 Subject: [PATCH] Make BitmapIndexImpl.CompressedBitmap, CompressedBitmapBuilder static A CompressedBitmap represents a pair (EWAH bit vector, PackIndex assigning bit positions to git objects). The bit vector is a member field and the PackIndex is implicit via the 'this' reference to the outer class. Make this clearer by making CompressedBitmap a static class and replacing the 'this' reference by an explicit field. Likewise for CompressedBitmapBuilder. Change-Id: Id85659fc4fc3ad82034db3370cce4cdbe0c5492c Suggested-by: Terry Parker --- .../storage/file/BitmapIndexImpl.java | 149 +++++++++--------- 1 file changed, 71 insertions(+), 78 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/BitmapIndexImpl.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/BitmapIndexImpl.java index 47adb914f..3873c3728 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/BitmapIndexImpl.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/BitmapIndexImpl.java @@ -90,7 +90,7 @@ public class BitmapIndexImpl implements BitmapIndex { EWAHCompressedBitmap compressed = packIndex.getBitmap(objectId); if (compressed == null) return null; - return new CompressedBitmap(compressed); + return new CompressedBitmap(compressed, this); } public CompressedBitmap toBitmap(PackBitmapIndex i, @@ -101,12 +101,12 @@ public class BitmapIndexImpl implements BitmapIndex { if (b == null) { return null; } - return new CompressedBitmap(b); + return new CompressedBitmap(b, this); } @Override public CompressedBitmapBuilder newBitmapBuilder() { - return new CompressedBitmapBuilder(); + return new CompressedBitmapBuilder(this); } int findPosition(AnyObjectId objectId) { @@ -210,16 +210,22 @@ public class BitmapIndexImpl implements BitmapIndex { } } - private final class CompressedBitmapBuilder implements BitmapBuilder { - private ComboBitset bitset = new ComboBitset(); + private static final class CompressedBitmapBuilder implements BitmapBuilder { + private ComboBitset bitset; + private final BitmapIndexImpl bitmapIndex; + + CompressedBitmapBuilder(BitmapIndexImpl bitmapIndex) { + this.bitset = new ComboBitset(); + this.bitmapIndex = bitmapIndex; + } @Override public boolean add(AnyObjectId objectId, int type) { - int position = findOrInsert(objectId, type); + int position = bitmapIndex.findOrInsert(objectId, type); if (bitset.contains(position)) return false; - Bitmap entry = getBitmap(objectId); + Bitmap entry = bitmapIndex.getBitmap(objectId); if (entry != null) { or(entry); return false; @@ -231,66 +237,45 @@ public class BitmapIndexImpl implements BitmapIndex { @Override public boolean contains(AnyObjectId objectId) { - int position = findPosition(objectId); + int position = bitmapIndex.findPosition(objectId); return 0 <= position && bitset.contains(position); } @Override public BitmapBuilder addObject(AnyObjectId objectId, int type) { - bitset.set(findOrInsert(objectId, type)); + bitset.set(bitmapIndex.findOrInsert(objectId, type)); return this; } @Override public void remove(AnyObjectId objectId) { - int position = findPosition(objectId); + int position = bitmapIndex.findPosition(objectId); if (0 <= position) bitset.remove(position); } @Override public CompressedBitmapBuilder or(Bitmap other) { - if (isSameCompressedBitmap(other)) { - bitset.or(((CompressedBitmap) other).bitmap); - } else if (isSameCompressedBitmapBuilder(other)) { - CompressedBitmapBuilder b = (CompressedBitmapBuilder) other; - bitset.or(b.bitset.combine()); - } else { - throw new IllegalArgumentException(); - } + bitset.or(ewahBitmap(other)); return this; } @Override public CompressedBitmapBuilder andNot(Bitmap other) { - if (isSameCompressedBitmap(other)) { - bitset.andNot(((CompressedBitmap) other).bitmap); - } else if (isSameCompressedBitmapBuilder(other)) { - CompressedBitmapBuilder b = (CompressedBitmapBuilder) other; - bitset.andNot(b.bitset.combine()); - } else { - throw new IllegalArgumentException(); - } + bitset.andNot(ewahBitmap(other)); return this; } @Override public CompressedBitmapBuilder xor(Bitmap other) { - if (isSameCompressedBitmap(other)) { - bitset.xor(((CompressedBitmap) other).bitmap); - } else if (isSameCompressedBitmapBuilder(other)) { - CompressedBitmapBuilder b = (CompressedBitmapBuilder) other; - bitset.xor(b.bitset.combine()); - } else { - throw new IllegalArgumentException(); - } + bitset.xor(ewahBitmap(other)); return this; } /** @return the fully built immutable bitmap */ @Override public CompressedBitmap build() { - return new CompressedBitmap(bitset.combine()); + return new CompressedBitmap(bitset.combine(), bitmapIndex); } @Override @@ -305,14 +290,14 @@ public class BitmapIndexImpl implements BitmapIndex { @Override public boolean removeAllOrNone(PackBitmapIndex index) { - if (!packIndex.equals(index)) + if (!bitmapIndex.packIndex.equals(index)) return false; EWAHCompressedBitmap curr = bitset.combine() - .xor(ones(indexObjectCount)); + .xor(ones(bitmapIndex.indexObjectCount)); IntIterator ii = curr.intIterator(); - if (ii.hasNext() && ii.next() < indexObjectCount) + if (ii.hasNext() && ii.next() < bitmapIndex.indexObjectCount) return false; bitset = new ComboBitset(curr); return true; @@ -320,49 +305,59 @@ public class BitmapIndexImpl implements BitmapIndex { @Override public BitmapIndexImpl getBitmapIndex() { - return BitmapIndexImpl.this; + return bitmapIndex; + } + + private EWAHCompressedBitmap ewahBitmap(Bitmap other) { + if (other instanceof CompressedBitmap) { + CompressedBitmap b = (CompressedBitmap) other; + if (b.bitmapIndex != bitmapIndex) { + throw new IllegalArgumentException(); + } + return b.bitmap; + } + if (other instanceof CompressedBitmapBuilder) { + CompressedBitmapBuilder b = (CompressedBitmapBuilder) other; + if (b.bitmapIndex != bitmapIndex) { + throw new IllegalArgumentException(); + } + return b.bitset.combine(); + } + throw new IllegalArgumentException(); } } - final class CompressedBitmap implements Bitmap { + static final class CompressedBitmap implements Bitmap { final EWAHCompressedBitmap bitmap; + final BitmapIndexImpl bitmapIndex; - CompressedBitmap(EWAHCompressedBitmap bitmap) { + CompressedBitmap(EWAHCompressedBitmap bitmap, BitmapIndexImpl bitmapIndex) { this.bitmap = bitmap; + this.bitmapIndex = bitmapIndex; } @Override public CompressedBitmap or(Bitmap other) { - return new CompressedBitmap(bitmap.or(bitmapOf(other))); + return new CompressedBitmap(bitmap.or(ewahBitmap(other)), bitmapIndex); } @Override public CompressedBitmap andNot(Bitmap other) { - return new CompressedBitmap(bitmap.andNot(bitmapOf(other))); + return new CompressedBitmap(bitmap.andNot(ewahBitmap(other)), bitmapIndex); } @Override public CompressedBitmap xor(Bitmap other) { - return new CompressedBitmap(bitmap.xor(bitmapOf(other))); - } - - private EWAHCompressedBitmap bitmapOf(Bitmap other) { - if (isSameCompressedBitmap(other)) - return ((CompressedBitmap) other).bitmap; - if (isSameCompressedBitmapBuilder(other)) - return ((CompressedBitmapBuilder) other).build().bitmap; - CompressedBitmapBuilder builder = newBitmapBuilder(); - builder.or(other); - return builder.build().bitmap; + return new CompressedBitmap(bitmap.xor(ewahBitmap(other)), bitmapIndex); } private final IntIterator ofObjectType(int type) { - return packIndex.ofObjectType(bitmap, type).intIterator(); + return bitmapIndex.packIndex.ofObjectType(bitmap, type).intIterator(); } @Override public Iterator iterator() { - final IntIterator dynamic = bitmap.andNot(ones(indexObjectCount)) + final IntIterator dynamic = bitmap.andNot(ones(bitmapIndex.indexObjectCount)) .intIterator(); final IntIterator commits = ofObjectType(Constants.OBJ_COMMIT); final IntIterator trees = ofObjectType(Constants.OBJ_TREE); @@ -399,12 +394,12 @@ public class BitmapIndexImpl implements BitmapIndex { throw new NoSuchElementException(); int position = cached.next(); - if (position < indexObjectCount) { + if (position < bitmapIndex.indexObjectCount) { out.type = type; - out.objectId = packIndex.getObject(position); + out.objectId = bitmapIndex.packIndex.getObject(position); } else { - position -= indexObjectCount; - MutableEntry entry = mutableIndex.getObject(position); + position -= bitmapIndex.indexObjectCount; + MutableEntry entry = bitmapIndex.mutableIndex.getObject(position); out.type = entry.type; out.objectId = entry; } @@ -421,8 +416,22 @@ public class BitmapIndexImpl implements BitmapIndex { return bitmap; } - BitmapIndexImpl getPackBitmapIndex() { - return BitmapIndexImpl.this; + private EWAHCompressedBitmap ewahBitmap(Bitmap other) { + if (other instanceof CompressedBitmap) { + CompressedBitmap b = (CompressedBitmap) other; + if (b.bitmapIndex != bitmapIndex) { + throw new IllegalArgumentException(); + } + return b.bitmap; + } + if (other instanceof CompressedBitmapBuilder) { + CompressedBitmapBuilder b = (CompressedBitmapBuilder) other; + if (b.bitmapIndex != bitmapIndex) { + throw new IllegalArgumentException(); + } + return b.bitset.combine(); + } + throw new IllegalArgumentException(); } } @@ -490,22 +499,6 @@ public class BitmapIndexImpl implements BitmapIndex { } } - boolean isSameCompressedBitmap(Bitmap other) { - if (other instanceof CompressedBitmap) { - CompressedBitmap b = (CompressedBitmap) other; - return this == b.getPackBitmapIndex(); - } - return false; - } - - boolean isSameCompressedBitmapBuilder(Bitmap other) { - if (other instanceof CompressedBitmapBuilder) { - CompressedBitmapBuilder b = (CompressedBitmapBuilder) other; - return this == b.getBitmapIndex(); - } - return false; - } - static final EWAHCompressedBitmap ones(int sizeInBits) { EWAHCompressedBitmap mask = new EWAHCompressedBitmap(); mask.addStreamOfEmptyWords(