On 12/15/2016 02:09 PM, Augie Fackler wrote:
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.
The previous changesets (already public) is also breaking pure. Can one
of you submit the necessary test update?
--- 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.
--
Pierre-Yves David
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
