Re: [jgit-dev] Implementing shallow clones

[Matt Fischer <mattfischer84@xxxxxxxxx>]

>> 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.
>
I thought about that, but I'll need to do it for both DepthWalk and
DepthObjectWalk.  I guess two classes isn't all that bad, but it felt
like it might be getting a little ugly to have a big list in there.
I'll go for that, though.

> 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.
>
Actually we do.  In addition to chopping off the walk at the
appropriate times, I also need to send the list of boundary commits
back, so that the client can mark them as shallow.  This requires me
to know the depth on these commits exactly, or else I might mistakenly
send back a commit that looks like it's at the boundary but really
isn't.

> 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.
>
Interesting idea.  It seems a bit odd that the DepthGenerator knows
about the cutoff level from the DepthFilter, though.  Would it be so
bad to just make the DepthGenerator do all the filtering itself, and
dispose of the DepthFilter entirely?

I wasn't completely familiar with the concept of non-incremental
generators until now, but now that I understand them, I'm wondering if
the best way to do this would be to just make DepthGenerator descend
from TopoSortGenerator, and tag them as they come out of it that way.
If I do the filtering as part of the generator itself, then I won't
have to worry about the PendingGenerator applying the filter too
early, and everything should work out.  At that point, I could even
make the generator have a == mode and a <= mode, so that I can use one
for the boundary generation, and the other for the pack generation.
Does this sound like it would work?

This still has the disadvantage of walking the entire tree instead of
cutting off once the depth has been reached, but that doesn't seem
like it's the end of the world.  Do you feel like that's going to be a
big performance drain, or would it be ok?

--Matt