From 8476d83438651a1c42bb4fd673800285b9f94462 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Sat, 2 Dec 2006 23:44:00 -0500 Subject: [PATCH] Reduce the number of temporary byte[20] arrays allocated. While searching for and loading an object from a pack file we were allocating two 20 byte arrays: one to store the temporary object id during index searching and another to store the variable length header data (as well as an OBJ_REF) during object fetching. But we can easily combine these two together to reduce the amount of garbage we need to load onto the heap. When searching multiple pack files for an object we can easily share the same byte[20] array across all of them by passing it down as a parameter until one of the packs returns with the desired object. This reduces the amount of garbage we generate when there is more than one pack file open. But it also complicates the API a little and isn't very "Java like" so we should document this odd behavior. Signed-off-by: Shawn O. Pearce --- .../jgit/lib/DeltaRefPackedObjectLoader.java | 3 +- .../src/org/spearce/jgit/lib/PackFile.java | 75 ++++++++++++++++++---- .../src/org/spearce/jgit/lib/Repository.java | 50 +++++++++------ .../tst/org/spearce/jgit/lib/T0004_PackReader.java | 2 +- 4 files changed, 93 insertions(+), 37 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/DeltaRefPackedObjectLoader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/DeltaRefPackedObjectLoader.java index 72b73e32..4e4ca7c1 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/DeltaRefPackedObjectLoader.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/DeltaRefPackedObjectLoader.java @@ -15,7 +15,8 @@ class DeltaRefPackedObjectLoader extends DeltaPackedObjectLoader { } protected ObjectLoader getBaseLoader() throws IOException { - final ObjectLoader or = pack.resolveBase(deltaBase); + final ObjectLoader or = pack.get(deltaBase, + new byte[Constants.OBJECT_ID_LENGTH]); if (or == null) throw new MissingObjectException(deltaBase, "delta base"); return or; diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java index b3962e50..facec5ac 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackFile.java @@ -72,23 +72,70 @@ public class PackFile { } } - ObjectLoader resolveBase(final ObjectId id) throws IOException { - return get(id); - } - ObjectLoader resolveBase(final long ofs) throws IOException { - return reader(ofs); + return reader(ofs, new byte[Constants.OBJECT_ID_LENGTH]); } - public boolean hasObject(final ObjectId id) throws IOException { - return findOffset(id) != -1; + /** + * Determine if an object is contained within the pack file. + *

+ * For performance reasons only the index file is searched; the main + * pack content is ignored entirely. + *

+ * + * @param id + * the object to look for. Must not be null. + * @param tmp + * a temporary buffer loaned to this pack for use during + * the search. This buffer must be at least + * {@link Constants#OBJECT_ID_LENGTH} bytes in size. The + * buffer will be overwritten during the search, but is + * unused upon return. + * @return true if the object is in this pack; false otherwise. + * @throws IOException + * there was an error reading data from the pack's index + * file. + */ + public boolean hasObject(final ObjectId id, final byte[] tmp) + throws IOException { + return findOffset(id, tmp) != -1; } - public PackedObjectLoader get(final ObjectId id) throws IOException { - final long offset = findOffset(id); + /** + * Get an object from this pack. + *

+ * For performance reasons the caller is responsible for supplying a + * temporary buffer of at least {@link Constants#OBJECT_ID_LENGTH} bytes + * for use during searching. If an object loader is returned this + * temporary buffer becomes the property of the object loader and must + * not be overwritten by the caller. If no object loader is returned + * then the temporary buffer remains the property of the caller and may + * be given to a different pack file to continue searching for the + * needed object. + *

+ * + * @param id + * the object to obtain from the pack. Must not be null. + * @param tmp + * a temporary buffer loaned to this pack for use during + * the search, and given to the returned loader if the + * object is found. This buffer must be at least + * {@link Constants#OBJECT_ID_LENGTH} bytes in size. The + * buffer will be overwritten during the search. The + * buffer will be given to the loader if a loader is + * returned. If null is returned the caller may reuse the + * buffer. + * @return the object loader for the requested object if it is contained + * in this pack; null if the object was not found. + * @throws IOException + * the pack file or the index could not be read. + */ + public PackedObjectLoader get(final ObjectId id, final byte[] tmp) + throws IOException { + final long offset = findOffset(id, tmp); if (offset == -1) return null; - final PackedObjectLoader objReader = reader(offset); + final PackedObjectLoader objReader = reader(offset, tmp); objReader.setId(id); return objReader; } @@ -138,8 +185,8 @@ public class PackFile { return idxHeader; } - private PackedObjectLoader reader(final long objOffset) throws IOException { - final byte[] ib = new byte[Constants.OBJECT_ID_LENGTH]; + private PackedObjectLoader reader(final long objOffset, final byte[] ib) + throws IOException { long pos = objOffset; int p = 0; @@ -193,9 +240,9 @@ public class PackFile { return new WholePackedObjectLoader(this, pos, type, (int) size); } - private long findOffset(final ObjectId objId) throws IOException { + private long findOffset(final ObjectId objId, final byte[] tmpid) + throws IOException { final int levelOne = objId.getFirstByte(); - final byte[] tmpid = new byte[Constants.OBJECT_ID_LENGTH]; long high = idxHeader[levelOne]; long low = levelOne == 0 ? 0 : idxHeader[levelOne - 1]; diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java index d84da318..3e91f90f 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java @@ -115,32 +115,40 @@ public class Repository { } public boolean hasObject(final ObjectId objectId) { - for (int k = packs.length - 1; k >= 0; k--) { - try { - if (packs[k].hasObject(objectId)) - return true; - } catch (IOException ioe) { - // This shouldn't happen unless the pack was corrupted - // after we opened it. We'll ignore the error as though - // the object does not exist in this pack. - // - } + int k = packs.length; + if (k > 0) { + final byte[] tmp = new byte[Constants.OBJECT_ID_LENGTH]; + do { + try { + if (packs[--k].hasObject(objectId, tmp)) + return true; + } catch (IOException ioe) { + // This shouldn't happen unless the pack was corrupted + // after we opened it. We'll ignore the error as though + // the object does not exist in this pack. + // + } + } while (k > 0); } return toFile(objectId).isFile(); } public ObjectLoader openObject(final ObjectId id) throws IOException { - for (int k = packs.length - 1; k >= 0; k--) { - try { - final ObjectLoader o = packs[k].get(id); - if (o != null) - return o; - } catch (IOException ioe) { - // This shouldn't happen unless the pack was corrupted after we - // opened it. We'll ignore the error as though the object does - // not exist in this pack. - // - } + int k = packs.length; + if (k > 0) { + final byte[] tmp = new byte[Constants.OBJECT_ID_LENGTH]; + do { + try { + final ObjectLoader ol = packs[--k].get(id, tmp); + if (ol != null) + return ol; + } catch (IOException ioe) { + // This shouldn't happen unless the pack was corrupted + // after we opened it. We'll ignore the error as though + // the object does not exist in this pack. + // + } + } while (k > 0); } try { return new UnpackedObjectLoader(this, id); diff --git a/org.spearce.jgit/tst/org/spearce/jgit/lib/T0004_PackReader.java b/org.spearce.jgit/tst/org/spearce/jgit/lib/T0004_PackReader.java index 2d51e5d7..473e13f7 100644 --- a/org.spearce.jgit/tst/org/spearce/jgit/lib/T0004_PackReader.java +++ b/org.spearce.jgit/tst/org/spearce/jgit/lib/T0004_PackReader.java @@ -30,7 +30,7 @@ public class T0004_PackReader extends RepositoryTestCase { id = new ObjectId("902d5476fa249b7abc9d84c611577a81381f0327"); pr = new PackFile(db, TEST_PACK); - or = pr.get(id); + or = pr.get(id, new byte[Constants.OBJECT_ID_LENGTH]); assertNotNull(or); assertEquals(id, or.getId()); assertEquals(Constants.TYPE_TREE, or.getType()); -- 2.11.4.GIT