tree:<depth>: do not revisit tree during packing
If a tree is visited during pack and filtered out with tree:<depth>, we may need to include it if it is visited again at a lower depth. Until now we revisit it no matter what the depth is. Now, avoid visiting it if it has been visited at a lower or equal depth. Change-Id: I68cc1d08f1999a8336684a05fe16e7ae51898866 Signed-off-by: Matthew DeVore <matvore@gmail.com>
This commit is contained in:
parent
175e66548b
commit
0a15cb3a2b
|
@ -1731,6 +1731,128 @@ public void testV2FetchFilterTreeDepth_iterateOverTreeAtTwoLevels()
|
||||||
.has(preparator.foo.toObjectId()));
|
.has(preparator.foo.toObjectId()));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Creates a commit with the following files:
|
||||||
|
* <pre>
|
||||||
|
* a/x/b/foo
|
||||||
|
* b/u/c/baz
|
||||||
|
* y/x/b/foo
|
||||||
|
* z/v/c/baz
|
||||||
|
* </pre>
|
||||||
|
* which has two pairs of identical trees:
|
||||||
|
* <ul>
|
||||||
|
* <li>one at /a and /y
|
||||||
|
* <li>one at /b/u and /z/v
|
||||||
|
* </ul>
|
||||||
|
* Note that this class defines unique 8 trees (rootTree and subtree1-7)
|
||||||
|
* which means PackStatistics should report having visited 8 trees.
|
||||||
|
*/
|
||||||
|
class RepeatedSubtreeAtSameLevelPreparator {
|
||||||
|
RevBlob foo = remote.blob("foo");
|
||||||
|
|
||||||
|
/** foo */
|
||||||
|
RevTree subtree1 = remote.tree(remote.file("foo", foo));
|
||||||
|
|
||||||
|
/** b/foo */
|
||||||
|
RevTree subtree2 = (new TreeBuilder() {
|
||||||
|
@Override
|
||||||
|
void addElements(DirCacheBuilder dcBuilder) throws Exception {
|
||||||
|
dcBuilder.addTree(new byte[] {'b'}, DirCacheEntry.STAGE_0,
|
||||||
|
remote.getRevWalk().getObjectReader(), subtree1);
|
||||||
|
}
|
||||||
|
}).build();
|
||||||
|
|
||||||
|
/** x/b/foo */
|
||||||
|
RevTree subtree3 = (new TreeBuilder() {
|
||||||
|
@Override
|
||||||
|
void addElements(DirCacheBuilder dcBuilder) throws Exception {
|
||||||
|
dcBuilder.addTree(new byte[] {'x'}, DirCacheEntry.STAGE_0,
|
||||||
|
remote.getRevWalk().getObjectReader(), subtree2);
|
||||||
|
}
|
||||||
|
}).build();
|
||||||
|
|
||||||
|
RevBlob baz = remote.blob("baz");
|
||||||
|
|
||||||
|
/** baz */
|
||||||
|
RevTree subtree4 = remote.tree(remote.file("baz", baz));
|
||||||
|
|
||||||
|
/** c/baz */
|
||||||
|
RevTree subtree5 = (new TreeBuilder() {
|
||||||
|
@Override
|
||||||
|
void addElements(DirCacheBuilder dcBuilder) throws Exception {
|
||||||
|
dcBuilder.addTree(new byte[] {'c'}, DirCacheEntry.STAGE_0,
|
||||||
|
remote.getRevWalk().getObjectReader(), subtree4);
|
||||||
|
}
|
||||||
|
}).build();
|
||||||
|
|
||||||
|
/** u/c/baz */
|
||||||
|
RevTree subtree6 = (new TreeBuilder() {
|
||||||
|
@Override
|
||||||
|
void addElements(DirCacheBuilder dcBuilder) throws Exception {
|
||||||
|
dcBuilder.addTree(new byte[] {'u'}, DirCacheEntry.STAGE_0,
|
||||||
|
remote.getRevWalk().getObjectReader(), subtree5);
|
||||||
|
}
|
||||||
|
}).build();
|
||||||
|
|
||||||
|
/** v/c/baz */
|
||||||
|
RevTree subtree7 = (new TreeBuilder() {
|
||||||
|
@Override
|
||||||
|
void addElements(DirCacheBuilder dcBuilder) throws Exception {
|
||||||
|
dcBuilder.addTree(new byte[] {'v'}, DirCacheEntry.STAGE_0,
|
||||||
|
remote.getRevWalk().getObjectReader(), subtree5);
|
||||||
|
}
|
||||||
|
}).build();
|
||||||
|
|
||||||
|
RevTree rootTree = (new TreeBuilder() {
|
||||||
|
@Override
|
||||||
|
void addElements(DirCacheBuilder dcBuilder) throws Exception {
|
||||||
|
dcBuilder.addTree(new byte[] {'a'}, DirCacheEntry.STAGE_0,
|
||||||
|
remote.getRevWalk().getObjectReader(), subtree3);
|
||||||
|
dcBuilder.addTree(new byte[] {'b'}, DirCacheEntry.STAGE_0,
|
||||||
|
remote.getRevWalk().getObjectReader(), subtree6);
|
||||||
|
dcBuilder.addTree(new byte[] {'y'}, DirCacheEntry.STAGE_0,
|
||||||
|
remote.getRevWalk().getObjectReader(), subtree3);
|
||||||
|
dcBuilder.addTree(new byte[] {'z'}, DirCacheEntry.STAGE_0,
|
||||||
|
remote.getRevWalk().getObjectReader(), subtree7);
|
||||||
|
}
|
||||||
|
}).build();
|
||||||
|
RevCommit commit = remote.commit(rootTree);
|
||||||
|
|
||||||
|
RepeatedSubtreeAtSameLevelPreparator() throws Exception {}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testV2FetchFilterTreeDepth_repeatTreeAtSameLevelIncludeFile()
|
||||||
|
throws Exception {
|
||||||
|
RepeatedSubtreeAtSameLevelPreparator preparator =
|
||||||
|
new RepeatedSubtreeAtSameLevelPreparator();
|
||||||
|
remote.update("master", preparator.commit);
|
||||||
|
|
||||||
|
uploadV2WithTreeDepthFilter(5, preparator.commit.toObjectId());
|
||||||
|
|
||||||
|
assertTrue(client.getObjectDatabase()
|
||||||
|
.has(preparator.foo.toObjectId()));
|
||||||
|
assertTrue(client.getObjectDatabase()
|
||||||
|
.has(preparator.baz.toObjectId()));
|
||||||
|
assertEquals(8, stats.getTreesTraversed());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testV2FetchFilterTreeDepth_repeatTreeAtSameLevelExcludeFile()
|
||||||
|
throws Exception {
|
||||||
|
RepeatedSubtreeAtSameLevelPreparator preparator =
|
||||||
|
new RepeatedSubtreeAtSameLevelPreparator();
|
||||||
|
remote.update("master", preparator.commit);
|
||||||
|
|
||||||
|
uploadV2WithTreeDepthFilter(4, preparator.commit.toObjectId());
|
||||||
|
|
||||||
|
assertFalse(client.getObjectDatabase()
|
||||||
|
.has(preparator.foo.toObjectId()));
|
||||||
|
assertFalse(client.getObjectDatabase()
|
||||||
|
.has(preparator.baz.toObjectId()));
|
||||||
|
assertEquals(8, stats.getTreesTraversed());
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testWantFilteredObject() throws Exception {
|
public void testWantFilteredObject() throws Exception {
|
||||||
RepeatedSubtreePreparator preparator = new RepeatedSubtreePreparator();
|
RepeatedSubtreePreparator preparator = new RepeatedSubtreePreparator();
|
||||||
|
|
|
@ -63,6 +63,7 @@
|
||||||
import java.util.Collection;
|
import java.util.Collection;
|
||||||
import java.util.Collections;
|
import java.util.Collections;
|
||||||
import java.util.Comparator;
|
import java.util.Comparator;
|
||||||
|
import java.util.HashMap;
|
||||||
import java.util.HashSet;
|
import java.util.HashSet;
|
||||||
import java.util.Iterator;
|
import java.util.Iterator;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
|
@ -873,19 +874,35 @@ private ObjectWalk getObjectWalk() {
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* A visitation policy which causes objects to be visited repeatedly by
|
* A visitation policy which uses the depth at which the object is seen to
|
||||||
* making {@code shouldVisit} always return {@code true}.
|
* decide if re-traversal is necessary. In particular, if the object has
|
||||||
|
* already been visited at this depth or shallower, it is not necessary to
|
||||||
|
* re-visit at this depth.
|
||||||
*/
|
*/
|
||||||
private static final ObjectWalk.VisitationPolicy ALWAYS_VISIT_POLICY =
|
private class DepthAwareVisitationPolicy
|
||||||
new ObjectWalk.VisitationPolicy() {
|
implements ObjectWalk.VisitationPolicy {
|
||||||
@Override
|
private final Map<ObjectId, Long> lowestDepthVisited = new HashMap<>();
|
||||||
public boolean shouldVisit(RevObject o) {
|
|
||||||
return true;
|
private final ObjectWalk walk;
|
||||||
|
|
||||||
|
DepthAwareVisitationPolicy(ObjectWalk walk) {
|
||||||
|
this.walk = requireNonNull(walk);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void visited(RevObject o) {}
|
public boolean shouldVisit(RevObject o) {
|
||||||
};
|
Long lastDepth = lowestDepthVisited.get(o);
|
||||||
|
if (lastDepth == null) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
return walk.getTreeDepth() < lastDepth;
|
||||||
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public void visited(RevObject o) {
|
||||||
|
lowestDepthVisited.put(o, (long) walk.getTreeDepth());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Prepare the list of objects to be written to the pack stream.
|
* Prepare the list of objects to be written to the pack stream.
|
||||||
|
@ -929,7 +946,7 @@ public void preparePack(ProgressMonitor countingMonitor,
|
||||||
throw new IllegalArgumentException(
|
throw new IllegalArgumentException(
|
||||||
JGitText.get().shallowPacksRequireDepthWalk);
|
JGitText.get().shallowPacksRequireDepthWalk);
|
||||||
if (filterSpec.getTreeDepthLimit() >= 0) {
|
if (filterSpec.getTreeDepthLimit() >= 0) {
|
||||||
walk.setVisitationPolicy(ALWAYS_VISIT_POLICY);
|
walk.setVisitationPolicy(new DepthAwareVisitationPolicy(walk));
|
||||||
}
|
}
|
||||||
findObjectsToPack(countingMonitor, walk, interestingObjects,
|
findObjectsToPack(countingMonitor, walk, interestingObjects,
|
||||||
uninterestingObjects, noBitmaps);
|
uninterestingObjects, noBitmaps);
|
||||||
|
|
Loading…
Reference in New Issue