[PATCH 5 of 7] reachableroots: use internal "revstates" array to test if rev is a root

[Augie Fackler]

On Tue, Aug 18, 2015 at 12:32:55PM -0700, Pierre-Yves David wrote:
>
>
> On 08/18/2015 12:26 PM, Pierre-Yves David wrote:
> >
> >
> >On 08/18/2015 07:42 AM, Yuya Nishihara wrote:
> >># HG changeset patch
> >># User Yuya Nishihara <yuya at tcha.org>
> >># Date 1439534609 -32400
> >>#      Fri Aug 14 15:43:29 2015 +0900
> >># Node ID 93e5cd30d2ff9b63f11616054cb647d6ac998053
> >># Parent  2ca0b48b6de1c79bc205e7a660a5531c125cad9e
> >>reachableroots: use internal "revstates" array to test if rev is a root
> >>
> >>The main goal of this patch series is to reduce the use of PyXxx()
> >>function
> >>that is likely to require ugly error handling and inc/decref. Plus,
> >>this is
> >>faster than using PySet_Contains().
> >>
> >>   revset #0: 0::tip
> >>   0) 0.004168
> >>   1) 0.003678  88%
> >>
> >>This patch ignores out-of-range roots as they are in the pure
> >>implementation.
> >>Because reachable sets are calculated from heads, and out-of-range
> >>heads raise
> >>IndexError, we can just take out-of-range roots as unreachable.
> >>Otherwise,
> >>the test of "hg log -Gr '. + wdir()'" would fail.
> >>
> >>"heads" argument is changed to a list. Should we have to rename the C
> >>function
> >>as its signature is changed?
> >
> >Do we actually need (take advantage of) theses argument being a list?
> >Could it take an arbitrary iterable? This would allow use to pass
> >arbitrary smartset.
>
> So reading more of the current implementation, it seems we are not doing
> anything specific with the list (beside iterating over it) and nothing
> special with the set beside membership (removed in this series).
>
> So, I think we should enforce the argument to be smartsets and use smartset
> available API in the C code.
>
> that would probably justify a signature change, but we never released this
> code, so I'm unsure. If the old/new code raise proper exception when passed
> the new/argument and fallback to the pure implementation, I do not think we
> need to change the name

It will not fall back to the old version - it will just explode
because of type mismatches. That's by design - a previous incarnation
of the patch series silently swallowed errors and it meant that it was
too easy to have a trivial programming error cause the C extension to
go unused silently.

>
> --
> Pierre-Yves David
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel