[PATCH] rev: C implementation of delta chain resolution

Jun Wu quark at fb.com
Thu Jun 29 23:45:42 EDT 2017


Looks good to me. I have some minor comments, which don't affect correctness
and only count for 2% performance difference. So nothing serious. I'd
suggest queue this now, and we can improve later.

Excerpts from 's message of 2017-06-25 12:42:53 -0700:
> +static inline int index_baserev(indexObject *self, int rev)

Might worth a comment that "rev" range check must be done by the caller. So
new users of this function will be less likely to cause SIGSEGV.

> +    if (!PyArg_ParseTuple(args, "iOi", &rev, &stoparg, &generaldelta)) {

"generaldelta" could be part of indexObject (passed via initfunc) to avoid
"self._generaldelta" in Python code which is a hash lookup. That also makes
it possible to have a same signature with self._deltachain.

> [...]
> +    /* This should never happen. */
> +    if (baserev == -2) {

Might use "<= -2" to rule out other strange cases.

> +        # Try C implementation.
> +        try:
> +            return self.index.deltachain(rev, stoprev, self._generaldelta)
> +        except AttributeError:
> +            pass
> +

I think it's better to replace the function instead of calling "try" each
time. Like in revlog.__init__:

  try:
      self._deltachain = self.index.deltachain
  except AttributeError:
      pass

With the above changes (avoid passing generaldelta and replace the method
directly), perfrevlogrevision --cache deltachain gets 2% improvement.
Smaller than I excepted, so might be not that worthy to do the change.


More information about the Mercurial-devel mailing list