[PATCH RFC] cg1packer: use fastpath when changesets are not skipped or reordered

[Mike Hommey]

On Tue, Apr 14, 2015 at 04:36:33PM -0400, Pierre-Yves David wrote:
> 
> 
> On 04/12/2015 07:59 PM, Mike Hommey wrote:
> >On Mon, Apr 13, 2015 at 08:40:26AM +0900, Mike Hommey wrote:
> >>On Mon, Apr 13, 2015 at 08:15:29AM +0900, Mike Hommey wrote:
> >>>The current heuristic is that we can use the fast path when the repo is
> >>>not filtered and the requested heads match the topological heads. This
> >>>leaves out a lot of cases where the fast path would work just fine.
> >>>
> >>>The most obvious one is when there is a race between a push and a http
> >>>pull, where the puller gets heads before the push, and does the getbundle
> >>>after, in which case the fast path is not taken because the pulled heads
> >>>are not topological heads anymore. A sibling case is when the puller
> >>>requests non-topological heads (such as branch heads that were subsequently
> >>>merged), or any changeset that appears in the history of another.
> >>
> >>Another legitimate case, which is not covered by this patch:
> >>- consider a changelog like the following:
> >>
> >>@  2
> >>|
> >>| o  1
> >>|/
> >>o  0
> >>
> >>- local clone has 0:1.
> >>- pulling 2 uses the slow path because `heads` in getsubsetraw only
> >>   contains the nodeid for rev 2, while repo.heads contains 1 and 2.
> >
> >In fact, the reason why this case is not covered by this patch also
> >makes this patch regress the case where the local clone has 0 and we
> >pull 1 and 2, which does use the fast path before the patch but not
> >after.
> 
> I've confused. Why do we not use the fast path after that.

Because the revision numbers don't start at 0, and that makes the
heuristic in the patch fail.

> the second question is: What is the performance impact of computing this
> "fast-passable" thing?

Negligible. I haven't seen a notable difference between before and
after.

> Could we do some lazy approach that use the fast path as long as possible
> and just fallback to the slow old (in the middle of the bundle generation)
> whenever something is skipped?

That was kind of what I was suggesting as a future improvement.

> Why is this patch RFC only (what do you think needs to be
> improved/discussed/clarify before moving forward).

Because I wasn't sure the approach was right, and sure enough, 2
messages later I figured it introduced a regression. And maybe we'd be
better off with a more aggressive approach, trying to minimize the slow
path even more.

Mike

PS: the funny thing is that the slow path is based on data that may not
be accurate (as the mozilla-central history demonstrates), but
fortunately, /shouldn't/ indicate new files that aren't new. That's
funny because if it was happening, then the slow path would need to be
even slower.