RevCommitCG: Read changed-path-filters directly from commit graph

RevCommit and RevCommitCG were designed like "pointers" to data that
load the content on demand, not on construction. This saves memory.

Make the loading of changed-path-filter follow the same pattern. The
ChangedPathFilters are only pointers to locations in the commit-graph
(not the actual data), so the memory saving is not that big, but this
is more consistent with the rest of the API.

As 6.7 is not released, we can still change the RevWalk API.

Change-Id: Id4186ea744b8a2418d0329facae69f785108d356

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriter.java
index 9d1c33959f1819e5e59c977d50a68a58b022e0c1..c5cfe95864a854ad3469c630d1116fb388ebef4f 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/commitgraph/CommitGraphWriter.java
@@ -48,6 +48,7 @@ import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.treewalk.EmptyTreeIterator;
 import org.eclipse.jgit.treewalk.TreeWalk;
 import org.eclipse.jgit.util.NB;
@@ -405,25 +406,27 @@ public class CommitGraphWriter {
                data.write(scratch);
                int dataHeaderSize = data.size();
 
-               for (RevCommit cmit : graphCommits) {
-                       ChangedPathFilter cpf = cmit.getChangedPathFilter();
-                       if (cpf != null) {
-                               stats.changedPathFiltersReused++;
-                       } else {
-                               stats.changedPathFiltersComputed++;
-                               Optional<HashSet<ByteBuffer>> paths = computeBloomFilterPaths(
-                                               graphCommits.getObjectReader(), cmit);
-                               if (paths.isEmpty()) {
-                                       cpf = ChangedPathFilter.FULL;
+               try (RevWalk rw = new RevWalk(graphCommits.getObjectReader())) {
+                       for (RevCommit cmit : graphCommits) {
+                               ChangedPathFilter cpf = cmit.getChangedPathFilter(rw);
+                               if (cpf != null) {
+                                       stats.changedPathFiltersReused++;
                                } else {
-                                       cpf = ChangedPathFilter.fromPaths(paths.get());
+                                       stats.changedPathFiltersComputed++;
+                                       Optional<HashSet<ByteBuffer>> paths = computeBloomFilterPaths(
+                                                       graphCommits.getObjectReader(), cmit);
+                                       if (paths.isEmpty()) {
+                                               cpf = ChangedPathFilter.FULL;
+                                       } else {
+                                               cpf = ChangedPathFilter.fromPaths(paths.get());
+                                       }
                                }
+                               cpf.writeTo(data);
+                               NB.encodeInt32(scratch, 0, data.size() - dataHeaderSize);
+                               index.write(scratch);
                        }
-                       cpf.writeTo(data);
-                       NB.encodeInt32(scratch, 0, data.size() - dataHeaderSize);
-                       index.write(scratch);
+                       return new BloomFilterChunks(index, data);
                }
-               return new BloomFilterChunks(index, data);
        }
 
        private void writeExtraEdges(CancellableDigestOutputStream out)

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java
index e0bdf3eaf9d72353c737409e86616b11d5b207b7..461993814705d55dfc8912b715e0c1fe480a7799 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommit.java
@@ -694,10 +694,11 @@ public class RevCommit extends RevObject {
         * commit graph file, or the commit graph file was generated without changed
         * path filters.
         *
+        * @param rw A revwalk to load the commit graph (if available)
         * @return the changed path filter
         * @since 6.7
         */
-       public ChangedPathFilter getChangedPathFilter() {
+       public ChangedPathFilter getChangedPathFilter(RevWalk rw) {
                return null;
        }
 

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommitCG.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommitCG.java
index 8c8100320ae8f69815f3dcee5b6017fc630b4778..c7a03992b7660293e5a676e1af49c42c92d06299 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommitCG.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevCommitCG.java
@@ -30,8 +30,6 @@ class RevCommitCG extends RevCommit {
 
        private final int graphPosition;
 
-       private final ChangedPathFilter changedPathFilter;
-
        private int generation = Constants.COMMIT_GENERATION_UNKNOWN;
 
        /**
@@ -41,14 +39,10 @@ class RevCommitCG extends RevCommit {
         *            object name for the commit.
         * @param graphPosition
         *            the position in the commit-graph of the object.
-        * @param changedPathFilter
-        *            the changed path filter if one exists
         */
-       protected RevCommitCG(AnyObjectId id, int graphPosition,
-                       ChangedPathFilter changedPathFilter) {
+       protected RevCommitCG(AnyObjectId id, int graphPosition) {
                super(id);
                this.graphPosition = graphPosition;
-               this.changedPathFilter = changedPathFilter;
        }
 
        @Override
@@ -110,7 +104,7 @@ class RevCommitCG extends RevCommit {
 
        /** {@inheritDoc} */
        @Override
-       public ChangedPathFilter getChangedPathFilter() {
-               return changedPathFilter;
+       public ChangedPathFilter getChangedPathFilter(RevWalk rw) {
+               return rw.commitGraph().getChangedPathFilter(graphPosition);
        }
 }

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java
index f4bf710ed2e5a1996aed067321f9e9c4fd0d383f..27a09f49561c3de0e81cce9a09524fcfe1dcfcd8 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/RevWalk.java
@@ -1713,8 +1713,7 @@ public class RevWalk implements Iterable<RevCommit>, AutoCloseable {
 
        private RevCommit createCommit(AnyObjectId id, int graphPos) {
                if (graphPos >= 0) {
-                       return new RevCommitCG(id, graphPos,
-                                       commitGraph().getChangedPathFilter(graphPos));
+                       return new RevCommitCG(id, graphPos);
                }
                return new RevCommit(id);
        }

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/TreeRevFilter.java b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/TreeRevFilter.java
index f8b11200a7cfb8802c6b676c7516b6ff67280f80..43571a686868e24d1cac355f5df6a7797a137243 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/TreeRevFilter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/revwalk/TreeRevFilter.java
@@ -133,7 +133,7 @@ public class TreeRevFilter extends RevFilter {
                        int chgs = 0, adds = 0;
                        boolean changedPathFilterUsed = false;
                        boolean mustCalculateChgs = true;
-                       ChangedPathFilter cpf = c.getChangedPathFilter();
+                       ChangedPathFilter cpf = c.getChangedPathFilter(walker);
                        if (cpf != null) {
                                Optional<Set<byte[]>> paths = pathFilter.getFilter()
                                                .getPathsBestEffort();