From aab75dba7e63c88ddce92a75b2afa24cc97aeb04 Mon Sep 17 00:00:00 2001 From: Ivan Frade Date: Mon, 20 Nov 2023 12:01:12 -0800 Subject: [PATCH] BitmapIndex: Add interface to track bitmaps found (or not) We want to know what objects had bitmaps in the walk of the request. We can check their position in the history and evaluate our bitmap selection algorithm. Introduce a listener interface to the BitmapIndex to report which getBitmap() calls returned a bitmap (or not) and a method to the bitmap index to set the listener. Change-Id: Iac8fcc1539ddd2dd450e8a1cf5a5b1089679c378 --- .../org/eclipse/jgit/lib/BitmapIndexTest.java | 85 +++++++++++++++++++ .../storage/file/BitmapIndexImpl.java | 16 +++- .../src/org/eclipse/jgit/lib/BitmapIndex.java | 52 ++++++++++++ 3 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/BitmapIndexTest.java diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/BitmapIndexTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/BitmapIndexTest.java new file mode 100644 index 000000000..ee4fa8bcc --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/BitmapIndexTest.java @@ -0,0 +1,85 @@ +/* + * Copyright (C) 2024, Google Inc. and others + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Distribution License v. 1.0 which is available at + * https://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: BSD-3-Clause + */ +package org.eclipse.jgit.lib; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +import org.eclipse.jgit.internal.storage.file.FileRepository; +import org.eclipse.jgit.internal.storage.file.GC; +import org.eclipse.jgit.junit.LocalDiskRepositoryTestCase; +import org.eclipse.jgit.junit.TestRepository; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; +import org.junit.Before; +import org.junit.Test; + +public class BitmapIndexTest extends LocalDiskRepositoryTestCase { + + private static final String MAIN = "refs/heads/main"; + + TestRepository repo; + + RevCommit tipWithBitmap; + + @Override + @Before + public void setUp() throws Exception { + super.setUp(); + FileRepository db = createWorkRepository(); + repo = new TestRepository<>(db); + + RevCommit base = repo.commit().create(); + RevCommit one = repo.commit().parent(base).create(); + tipWithBitmap = repo.commit().parent(one).create(); + repo.update(MAIN, tipWithBitmap); + + GC gc = new GC(repo.getRepository()); + gc.setAuto(false); + gc.gc().get(); + + assertNotNull(repo.getRevWalk().getObjectReader().getBitmapIndex()); + } + + + @Test + public void listener_getBitmap_counted() throws Exception { + try (RevWalk rw = repo.getRevWalk(); + ObjectReader or = rw.getObjectReader()) { + BitmapLookupCounter counter = new BitmapLookupCounter(); + BitmapIndex bitmapIndex = or.getBitmapIndex(); + bitmapIndex.addBitmapLookupListener(counter); + + bitmapIndex.getBitmap(tipWithBitmap); + bitmapIndex.getBitmap(tipWithBitmap); + bitmapIndex.getBitmap(ObjectId.zeroId()); + + assertEquals(2, counter.bitmapFound); + assertEquals(1, counter.bitmapNotFound); + } + } + + private static class BitmapLookupCounter + implements BitmapIndex.BitmapLookupListener { + int bitmapFound = 0; + + int bitmapNotFound = 0; + + @Override + public void onBitmapFound(AnyObjectId oid) { + bitmapFound += 1; + } + + @Override + public void onBitmapNotFound(AnyObjectId oid) { + bitmapNotFound += 1; + } + } +} \ No newline at end of file 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 8d8c6a045..5602158d2 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 @@ -38,6 +38,8 @@ public class BitmapIndexImpl implements BitmapIndex { final int indexObjectCount; + private BitmapLookupListener listener = BitmapLookupListener.NOOP; + /** * Creates a BitmapIndex that is back by Compressed bitmaps. * @@ -57,8 +59,11 @@ PackBitmapIndex getPackBitmapIndex() { @Override public CompressedBitmap getBitmap(AnyObjectId objectId) { EWAHCompressedBitmap compressed = packIndex.getBitmap(objectId); - if (compressed == null) + if (compressed == null) { + listener.onBitmapNotFound(objectId); return null; + } + listener.onBitmapFound(objectId); return new CompressedBitmap(compressed, this); } @@ -67,6 +72,15 @@ public CompressedBitmapBuilder newBitmapBuilder() { return new CompressedBitmapBuilder(this); } + @Override + public void addBitmapLookupListener(BitmapLookupListener listener) { + if (listener == null) { + throw new IllegalArgumentException( + "Use NOOP instance for no listener"); // @NON-NLS-1@ + } + this.listener = listener; + } + int findPosition(AnyObjectId objectId) { int position = packIndex.findPosition(objectId); if (position < 0) { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/BitmapIndex.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/BitmapIndex.java index 97b5847f9..acaa6335d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/BitmapIndex.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/BitmapIndex.java @@ -39,6 +39,58 @@ public interface BitmapIndex { */ BitmapBuilder newBitmapBuilder(); + /** + * Report the results of {@link #getBitmap(AnyObjectId)} + * + * @since 6.8 + */ + interface BitmapLookupListener { + + /** + * This object has a bitmap in the index + * + * @param oid + * object id + */ + void onBitmapFound(AnyObjectId oid); + + /** + * This object does not have a bitmap in the index + * + * @param oid + * object id + */ + void onBitmapNotFound(AnyObjectId oid); + + /** + * No-op instance + */ + BitmapLookupListener NOOP = new BitmapLookupListener() { + @Override + public void onBitmapFound(AnyObjectId oid) { + // Nothing to do + } + + @Override + public void onBitmapNotFound(AnyObjectId oid) { + // Nothing to do + } + }; + } + + /** + * Report to this listener whether {@link #getBitmap(AnyObjectId)} finds a + * commit. + * + * @param listener + * instance listening to lookup events in the index. Never null. + * Set to {@link BitmapLookupListener#NOOP} to disable. + * @since 6.8 + */ + default void addBitmapLookupListener(BitmapLookupListener listener) { + // Empty implementation for API compatibility + } + /** * A bitmap representation of ObjectIds that can be iterated to return the * underlying {@code ObjectId}s or operated on with other {@code Bitmap}s.