Browse Source
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: Ib745645f187e1b1483f8587e99501dfcb7b867a5stable-4.2
Jonathan Nieder
9 years ago
2 changed files with 24 additions and 4 deletions
Loading…
Reference in new issue