From c8f5a3f99de13a9eb0ee9dd86b97b5d8ad10435c Mon Sep 17 00:00:00 2001 From: Ivan Frade Date: Tue, 25 Jul 2023 03:25:33 -0700 Subject: [PATCH] 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 --- .../commitgraph/CommitGraphWriter.java | 33 ++++++++++--------- .../org/eclipse/jgit/revwalk/RevCommit.java | 3 +- .../org/eclipse/jgit/revwalk/RevCommitCG.java | 12 ++----- .../src/org/eclipse/jgit/revwalk/RevWalk.java | 3 +- .../eclipse/jgit/revwalk/TreeRevFilter.java | 2 +- 5 files changed, 25 insertions(+), 28 deletions(-) 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 9d1c33959..c5cfe9586 100644 --- 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.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 @@ private BloomFilterChunks computeBloomFilterChunks(Stats stats) data.write(scratch); int dataHeaderSize = data.size(); - for (RevCommit cmit : graphCommits) { - ChangedPathFilter cpf = cmit.getChangedPathFilter(); - if (cpf != null) { - stats.changedPathFiltersReused++; - } else { - stats.changedPathFiltersComputed++; - Optional> 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> 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 e0bdf3eaf..461993814 100644 --- 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 @@ int getGeneration() { * 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 8c8100320..c7a03992b 100644 --- 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 @@ int getGeneration() { /** {@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 f4bf710ed..27a09f495 100644 --- 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 @@ protected RevCommit createCommit(AnyObjectId id) { 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 f8b11200a..43571a686 100644 --- 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 boolean include(RevWalk walker, RevCommit c) int chgs = 0, adds = 0; boolean changedPathFilterUsed = false; boolean mustCalculateChgs = true; - ChangedPathFilter cpf = c.getChangedPathFilter(); + ChangedPathFilter cpf = c.getChangedPathFilter(walker); if (cpf != null) { Optional> paths = pathFilter.getFilter() .getPathsBestEffort();