[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
|
Re: [jgit-dev] Implementing shallow clones
|
Matt Fischer <mattfischer84@xxxxxxxxx> wrote:
>
> 1) Lots of wrapper classes (DepthRevCommit, DepthWalk,
> DepthObjectWalk, etc.) I know this is how we do most of this stuff in
> Java, but some of these (especially the Walk classes) seem a little
> silly for the amount of code that's in them. Would it be better to
> just fold them into the base classes, or do you prefer to leave them
> as separate subclasses?
These shouldn't be in the base classes. But DepthRevCommit and
DepthObjectWalk could be folded into DepthWalk as static inner
classes if the number of top-level classes is bothering you.
> 2) StartGenerator makes use of a new DepthGenerator to tag the
> commits, but it has to be told to include this generator. At the
> moment, I just did this by adding a new setter on StartGenerator,
> which gets the job done, but feels kind of ugly.
Why not:
if (walker instanceof DepthWalk)
g = new DepthGenerator(g);
No need for a flag, its just done automatically if the walker was
allocated to perform depth computations.
> 3) Most significantly, I'm still not sure I'm doing the depth tagging
> stuff correctly. I'm inserting the DepthGenerator directly after the
> PendingGenerator, which is correct as long as PendingGenerator always
> produces topologically sorted commits.
But it doesn't. PendingGenerator's order is uh, mostly commit time
but really is not topo ordering.
> The right thing to do is to
> put it after the TopoSortGenerator,
Maybe not....
> but if I do that, things don't
> work. This is because the FIFORevQueue tries to suck in the entire
> commit chain right up front,
Where is the FIFORevQueue getting injected into the chain? Oh!
The TopoSortGenerator is consuming everything up front into a
private FIFORevQueue and isn't incremental.
OK, so we can't chain the DepthGenerator after the TopoSortGenerator.
But do we even really need topological ordering to compute depth?
If the depth field is initialized to -1 when the commit is allocated,
and set to 0 when its marked as a starting point, we have our proper
values for depth level 0, and we know if a given commit hasn't had
a depth assigned yet (its depth == -1). Yes?
If we wrap the PendingGenerator with a DepthGenerator like this we
can assign depth on the fly:
private final DateRevQueue pending;
private final Generator g;
private final RevFlag REJECTED;
RevCommit next() {
DepthCommit c = (DepthCommit)g.next();
if (c != null) {
int pDepth = c.depth + 1;
for (RevCommit myParent : c.parents) {
DepthCommit p = (DepthCommit)myParent;
if (p.depth < 0)
p.depth = pDepth;
else if (pDepth < p.depth) {
if (pDepth <= cutOffDepth && p.has(REJECTED)) {
// We didn't want this commit before, but we do now.
// Shove it back into the pending queue.
//
p.remove(REJECTED);
pending.add(p);
}
p.depth = pDepth;
}
}
}
return c;
}
I don't think you need to enforce topo order here at all.
Its possible for a commit to move to a lower depth after its been
produced, but that's OK, its still above the depth cut off either
way, so we really don't care that much.
The depth filter may cause the pending generator to cut early because
of this late depth update. This could cause us to exclude some
commits whose depth used to be below the cut-off, but are now above
it because we later found a shorter path to them. But I think we
can fix this by just re-inserting the moved parent into the pending
queue, but only if it had been previously discarded by the filter.
To do that you probably need to allocate a RevFlag and share it
between this generator and the filter. If the filter rejects a
commit because its depth is higher than the cutOffDepth it should
add the flag. Then here if the flag is set we shove it back into
the queue when it moved above the cut off. We'll pick it up again
during a future next() call and it will appear in the output.
If you still want topo order, it goes after this generator and the
full buffering of its FIFORevQueue will ensure all of the depths
are accurate when commits are output. But the filter will be mostly
accurate and thus be able to cut the revision walker off early.
--
Shawn.
