> On Dec 15, 2016, at 8:02 AM, Yuya Nishihara <y...@tcha.org> wrote: > > On Thu, 15 Dec 2016 09:01:11 +0100, Pierre-Yves David wrote: >> On 11/17/2016 06:16 PM, Mads Kiilerich wrote: >>> # HG changeset patch >>> # User Mads Kiilerich <mad...@unity3d.com> >>> # Date 1479322051 -3600 >>> # Wed Nov 16 19:47:31 2016 +0100 >>> # Node ID 851a14d5b80639efe61bd01ad215479c99106b40 >>> # Parent 7f39bccc1c96bffc83f3c6e774da57ecd38982a7 >>> bdiff: early pruning of common suffix before doing expensive computations >>> >>> Similar to the previous change, but trimming trailing lines. >> >> The change itself seems fine and the perf win is very tasty. However, >> this changesets breaks tests with --pure. >> >> I'm tempted to drop it unless we can provide a test update to fold into >> this. > > […]
I’d be fine to just #if pure the variations between pure and C, given the immense win this provides. > >>> --- a/mercurial/bdiff_module.c >>> +++ b/mercurial/bdiff_module.c >>> @@ -66,7 +66,7 @@ static PyObject *bdiff(PyObject *self, P >>> struct bdiff_line *al, *bl; >>> struct bdiff_hunk l, *h; >>> int an, bn, count; >>> - Py_ssize_t len = 0, la, lb, li = 0, lcommon = 0, lmax; >>> + Py_ssize_t len = 0, la, lb, li = 0, lcommon = 0, lcommonend = 0, lmax; >>> PyThreadState *_save; >>> >>> l.next = NULL; >>> @@ -88,9 +88,16 @@ static PyObject *bdiff(PyObject *self, P >>> if (*ia == '\n') >>> lcommon = li + 1; >>> /* we can almost add: if (li == lmax) lcommon = li; */ >>> + lmax -= lcommon; >>> + for (li = 0, ia = sa + la - 1, ib = sb + lb - 1; >>> + li <= lmax && *ia == *ib; >>> + ++li, --ia, --ib) >>> + if (*ia == '\n') >>> + lcommonend = li; > > And there appears to be an off-by-one error. 'li' is a zero-based index and > 'lmax' is a length. > > +1 for dropping this for now. Given the off-by-one, yeah, drop it. I’m a little surprised the bdiff automated stress tester didn’t trip over that. Sigh. _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
