Patchwork bdiff: implement cffi version of blocks

login
register
mail settings
Submitter Maciej Fijalkowski
Date Aug. 10, 2016, 2:45 p.m.
Message ID <2f4cd91aaa9a2ad1e4de.1470840335@brick.arcode.com>
Download mbox | patch
Permalink /patch/16236/
State Superseded
Headers show

Comments

Maciej Fijalkowski - Aug. 10, 2016, 2:45 p.m.
# HG changeset patch
# User Maciej Fijalkowski <fijall@gmail.com>
# Date 1469708228 -7200
#      Thu Jul 28 14:17:08 2016 +0200
# Node ID 2f4cd91aaa9a2ad1e4de097a1ec7a59127d51e20
# Parent  54ce0308751b2772dbfbe94004a139d53f292a3f
bdiff: implement cffi version of blocks
Yuya Nishihara - Aug. 11, 2016, 11:22 a.m.
On Wed, 10 Aug 2016 16:45:35 +0200, Maciej Fijalkowski wrote:
> # HG changeset patch
> # User Maciej Fijalkowski <fijall@gmail.com>
> # Date 1469708228 -7200
> #      Thu Jul 28 14:17:08 2016 +0200
> # Node ID 2f4cd91aaa9a2ad1e4de097a1ec7a59127d51e20
> # Parent  54ce0308751b2772dbfbe94004a139d53f292a3f
> bdiff: implement cffi version of blocks

The code looks good to me, but any plan for mercurial/cffi/ ?

https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-July/086442.html
Pierre-Yves David - Aug. 11, 2016, 8:13 p.m.
On 08/11/2016 01:22 PM, Yuya Nishihara wrote:
> On Wed, 10 Aug 2016 16:45:35 +0200, Maciej Fijalkowski wrote:
>> # HG changeset patch
>> # User Maciej Fijalkowski <fijall@gmail.com>
>> # Date 1469708228 -7200
>> #      Thu Jul 28 14:17:08 2016 +0200
>> # Node ID 2f4cd91aaa9a2ad1e4de097a1ec7a59127d51e20
>> # Parent  54ce0308751b2772dbfbe94004a139d53f292a3f
>> bdiff: implement cffi version of blocks
>
> The code looks good to me, but any plan for mercurial/cffi/ ?
>
> https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-July/086442.html

If I remember correctly a discussion with maciej in february, cffi is 
reusing a good share of the pure code so the split is not that simple.

I'll let Maciej jump in and discuss this further.

Cheers,
Maciej Fijalkowski - Aug. 12, 2016, 9:43 p.m.
On Thu, Aug 11, 2016 at 10:13 PM, Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
>
>
> On 08/11/2016 01:22 PM, Yuya Nishihara wrote:
>>
>> On Wed, 10 Aug 2016 16:45:35 +0200, Maciej Fijalkowski wrote:
>>>
>>> # HG changeset patch
>>> # User Maciej Fijalkowski <fijall@gmail.com>
>>> # Date 1469708228 -7200
>>> #      Thu Jul 28 14:17:08 2016 +0200
>>> # Node ID 2f4cd91aaa9a2ad1e4de097a1ec7a59127d51e20
>>> # Parent  54ce0308751b2772dbfbe94004a139d53f292a3f
>>> bdiff: implement cffi version of blocks
>>
>>
>> The code looks good to me, but any plan for mercurial/cffi/ ?
>>
>>
>> https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-July/086442.html
>
>
> If I remember correctly a discussion with maciej in february, cffi is
> reusing a good share of the pure code so the split is not that simple.
>
> I'll let Maciej jump in and discuss this further.
>
> Cheers,
>
> --
> Pierre-Yves David

I *think* as far as cffi declarations go, this is mostly it. I'm using
a lot of pure code, just written with performance in mind.
Yuya Nishihara - Aug. 13, 2016, 10:01 a.m.
On Fri, 12 Aug 2016 23:43:52 +0200, Maciej Fijalkowski wrote:
> On Thu, Aug 11, 2016 at 10:13 PM, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org> wrote:
> > On 08/11/2016 01:22 PM, Yuya Nishihara wrote:
> >>
> >> On Wed, 10 Aug 2016 16:45:35 +0200, Maciej Fijalkowski wrote:
> >>>
> >>> # HG changeset patch
> >>> # User Maciej Fijalkowski <fijall@gmail.com>
> >>> # Date 1469708228 -7200
> >>> #      Thu Jul 28 14:17:08 2016 +0200
> >>> # Node ID 2f4cd91aaa9a2ad1e4de097a1ec7a59127d51e20
> >>> # Parent  54ce0308751b2772dbfbe94004a139d53f292a3f
> >>> bdiff: implement cffi version of blocks
> >>
> >>
> >> The code looks good to me, but any plan for mercurial/cffi/ ?
> >>
> >>
> >> https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-July/086442.html
> >
> >
> > If I remember correctly a discussion with maciej in february, cffi is
> > reusing a good share of the pure code so the split is not that simple.

I think the problem will be solved if we can do "from ..pure.bdiff import *".
And we wouldn't want many setup_*.py and *_cffi_*.so at the root directory.
Yuya Nishihara - Aug. 24, 2016, 12:54 p.m.
On Wed, 10 Aug 2016 16:45:35 +0200, Maciej Fijalkowski wrote:
> # HG changeset patch
> # User Maciej Fijalkowski <fijall@gmail.com>
> # Date 1469708228 -7200
> #      Thu Jul 28 14:17:08 2016 +0200
> # Node ID 2f4cd91aaa9a2ad1e4de097a1ec7a59127d51e20
> # Parent  54ce0308751b2772dbfbe94004a139d53f292a3f
> bdiff: implement cffi version of blocks

I'll take this patch to move things forward, thanks.

Patch

diff --git a/mercurial/pure/bdiff.py b/mercurial/pure/bdiff.py
--- a/mercurial/pure/bdiff.py
+++ b/mercurial/pure/bdiff.py
@@ -12,6 +12,10 @@ 
 import re
 import struct
 
+from . import policy
+policynocffi = policy.policynocffi
+modulepolicy = policy.policy
+
 def splitnewlines(text):
     '''like str.splitlines, but only split on newlines.'''
     lines = [l + '\n' for l in text.split('\n')]
@@ -96,3 +100,37 @@ 
         text = re.sub('[ \t\r]+', ' ', text)
         text = text.replace(' \n', '\n')
     return text
+
+if modulepolicy not in policynocffi:
+    try:
+        from _bdiff_cffi import ffi, lib
+    except ImportError:
+        if modulepolicy == 'cffi': # strict cffi import
+            raise
+    else:
+        def blocks(sa, sb):
+            a = ffi.new("struct bdiff_line**")
+            b = ffi.new("struct bdiff_line**")
+            ac = ffi.new("char[]", sa)
+            bc = ffi.new("char[]", sb)
+            try:
+                an = lib.bdiff_splitlines(ac, len(sa), a)
+                bn = lib.bdiff_splitlines(bc, len(sb), b)
+                if not a[0] or not b[0]:
+                    raise MemoryError
+                l = ffi.new("struct bdiff_hunk*")
+                count = lib.bdiff_diff(a[0], an, b[0], bn, l)
+                if count < 0:
+                    raise MemoryError
+                rl = [None] * count
+                h = l.next
+                i = 0
+                while h:
+                    rl[i] = (h.a1, h.a2, h.b1, h.b2)
+                    h = h.next
+                    i += 1
+            finally:
+                lib.free(a[0])
+                lib.free(b[0])
+                lib.bdiff_freehunks(l.next)
+            return rl
diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -319,7 +319,9 @@ 
             self.distribution.ext_modules = []
         elif self.distribution.cffi:
             import setup_mpatch_cffi
-            exts = [setup_mpatch_cffi.ffi.distutils_extension()]
+            import setup_bdiff_cffi
+            exts = [setup_mpatch_cffi.ffi.distutils_extension(),
+                    setup_bdiff_cffi.ffi.distutils_extension()]
             # cffi modules go here
             if sys.platform == 'darwin':
                 import setup_osutil_cffi
diff --git a/setup_bdiff_cffi.py b/setup_bdiff_cffi.py
new file mode 100644
--- /dev/null
+++ b/setup_bdiff_cffi.py
@@ -0,0 +1,31 @@ 
+from __future__ import absolute_import
+
+import cffi
+import os
+
+ffi = cffi.FFI()
+ffi.set_source("_bdiff_cffi",
+    open(os.path.join(os.path.join(os.path.dirname(__file__), 'mercurial'),
+        'bdiff.c')).read(), include_dirs=['mercurial'])
+ffi.cdef("""
+struct bdiff_line {
+    int hash, n, e;
+    ssize_t len;
+    const char *l;
+};
+
+struct bdiff_hunk;
+struct bdiff_hunk {
+    int a1, a2, b1, b2;
+    struct bdiff_hunk *next;
+};
+
+int bdiff_splitlines(const char *a, ssize_t len, struct bdiff_line **lr);
+int bdiff_diff(struct bdiff_line *a, int an, struct bdiff_line *b, int bn,
+    struct bdiff_hunk *base);
+void bdiff_freehunks(struct bdiff_hunk *l);
+void free(void*);
+""")
+
+if __name__ == '__main__':
+    ffi.compile()