Deprecate BitmapBuilder.add and introduce simpler addObject method
The BitmapIndex.BitmapBuilder.add API is subtle: /** * Adds the id and the existing bitmap for the id, if one * exists, to the bitmap. * * @return true if the value was not contained or able to be * loaded. */ boolean add(AnyObjectId objectId, int type); Reading the name of the method does not make it obvious what it will do. Does it add the named object to the bitmap, or all objects reachable from it? It depends on whether the BitmapIndex owns an existing bitmap for that object. I did not notice this subtlety when skimming the javadoc, either. This resulted in enough confusion to subtly break the bitmap building code (see change I30844134bfde0cbabdfaab884c84b9809dd8bdb8 for details). So discourage use of the add() API by deprecating it. To replace it, provide a addObject() method that adds a single object. This way, callers can decide whether to use addObject() or or() based on the context. For example, if (bitmap.add(c, OBJ_COMMIT)) { for (RevCommit p : c.getParents()) { rememberToAlsoHandle(p); } } can be replaced with if (bitmap.contains(c)) { // already included } else if (index.getBitmap(c) != null) { bitmap.or(index.getBitmap(c)); } else { bitmap.addObject(c, OBJ_COMMIT); for (RevCommit p : c.getParents()) { rememberToAlsoHandle(p); } } which is more verbose but makes it clearer that the behavior depends on the content of index.getBitmaps(). Change-Id: Ib745645f187e1b1483f8587e99501dfcb7b867a5
This commit is contained in:
parent
424aa22b56
commit
f523f21e59
|
@ -119,10 +119,10 @@ int findPosition(AnyObjectId objectId) {
|
||||||
return position;
|
return position;
|
||||||
}
|
}
|
||||||
|
|
||||||
int addObject(AnyObjectId objectId, int type) {
|
int findOrInsert(AnyObjectId objectId, int type) {
|
||||||
int position = findPosition(objectId);
|
int position = findPosition(objectId);
|
||||||
if (position < 0) {
|
if (position < 0) {
|
||||||
position = mutableIndex.addObject(objectId, type);
|
position = mutableIndex.findOrInsert(objectId, type);
|
||||||
position += indexObjectCount;
|
position += indexObjectCount;
|
||||||
}
|
}
|
||||||
return position;
|
return position;
|
||||||
|
@ -215,7 +215,7 @@ private final class CompressedBitmapBuilder implements BitmapBuilder {
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public boolean add(AnyObjectId objectId, int type) {
|
public boolean add(AnyObjectId objectId, int type) {
|
||||||
int position = addObject(objectId, type);
|
int position = findOrInsert(objectId, type);
|
||||||
if (bitset.contains(position))
|
if (bitset.contains(position))
|
||||||
return false;
|
return false;
|
||||||
|
|
||||||
|
@ -235,6 +235,12 @@ public boolean contains(AnyObjectId objectId) {
|
||||||
return 0 <= position && bitset.contains(position);
|
return 0 <= position && bitset.contains(position);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public BitmapBuilder addObject(AnyObjectId objectId, int type) {
|
||||||
|
bitset.set(findOrInsert(objectId, type));
|
||||||
|
return this;
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void remove(AnyObjectId objectId) {
|
public void remove(AnyObjectId objectId) {
|
||||||
int position = findPosition(objectId);
|
int position = findPosition(objectId);
|
||||||
|
@ -446,7 +452,7 @@ MutableEntry getObject(int position) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
int addObject(AnyObjectId objectId, int type) {
|
int findOrInsert(AnyObjectId objectId, int type) {
|
||||||
MutableEntry entry = new MutableEntry(
|
MutableEntry entry = new MutableEntry(
|
||||||
objectId, type, revList.size());
|
objectId, type, revList.size());
|
||||||
revList.add(entry);
|
revList.add(entry);
|
||||||
|
|
|
@ -125,7 +125,9 @@ public interface BitmapBuilder extends Bitmap {
|
||||||
* @param type
|
* @param type
|
||||||
* the Git object type. See {@link Constants}.
|
* the Git object type. See {@link Constants}.
|
||||||
* @return true if the value was not contained or able to be loaded.
|
* @return true if the value was not contained or able to be loaded.
|
||||||
|
* @deprecated use {@link #or} or {@link #addObject} instead.
|
||||||
*/
|
*/
|
||||||
|
@Deprecated
|
||||||
boolean add(AnyObjectId objectId, int type);
|
boolean add(AnyObjectId objectId, int type);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -137,6 +139,18 @@ public interface BitmapBuilder extends Bitmap {
|
||||||
*/
|
*/
|
||||||
boolean contains(AnyObjectId objectId);
|
boolean contains(AnyObjectId objectId);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Adds the id to the bitmap.
|
||||||
|
*
|
||||||
|
* @param objectId
|
||||||
|
* the object ID
|
||||||
|
* @param type
|
||||||
|
* the Git object type. See {@link Constants}.
|
||||||
|
* @return the current builder.
|
||||||
|
* @since 4.2
|
||||||
|
*/
|
||||||
|
BitmapBuilder addObject(AnyObjectId objectId, int type);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Remove the id from the bitmap.
|
* Remove the id from the bitmap.
|
||||||
*
|
*
|
||||||
|
|
Loading…
Reference in New Issue