Rev 4790: Stop holding the gil while extracting data. in http://bazaar.launchpad.net/~jameinel/bzr/2.1.0b3-gc-nogil

[John Arbash Meinel]

At http://bazaar.launchpad.net/~jameinel/bzr/2.1.0b3-gc-nogil

------------------------------------------------------------
revno: 4790
revision-id: john at arbash-meinel.com-20091106180503-q3hcg1nxmut5b2cb
parent: john at arbash-meinel.com-20091106032111-0gs6xeq14qxv0erm
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: 2.1.0b3-gc-nogil
timestamp: Fri 2009-11-06 12:05:03 -0600
message:
  Stop holding the gil while extracting data.
  
  Previous experience has shown that we don't really spend a lot
  of time here, but there isn't any reason to hold the lock while
  we do it anyway.
  
  Performance testing has shown that for bzr operations 'with nogil'
  doesn't really do much, which was generally expected. Any variation
  observed is within the noise margins.
  
  This is mostly about 'the right thing to do', and potential
  future work where we may actually use threads.
-------------- next part --------------
=== modified file 'bzrlib/_groupcompress_pyx.pyx'
--- a/bzrlib/_groupcompress_pyx.pyx	2009-11-06 03:21:11 +0000
+++ b/bzrlib/_groupcompress_pyx.pyx	2009-11-06 18:05:03 +0000
@@ -31,10 +31,10 @@
 
 cdef extern from *:
     ctypedef unsigned long size_t
-    void * malloc(size_t)
-    void * realloc(void *, size_t)
-    void free(void *)
-    void memcpy(void *, void *, size_t)
+    void * malloc(size_t) nogil
+    void * realloc(void *, size_t) nogil
+    void free(void *) nogil
+    void memcpy(void *, void *, size_t) nogil
 
 
 cdef extern from "delta.h":
@@ -329,6 +329,7 @@
     cdef unsigned char *dst_buf, *out, cmd
     cdef Py_ssize_t size
     cdef unsigned int cp_off, cp_size
+    cdef int failed
 
     data = <unsigned char *>delta
     top = data + delta_size
@@ -338,37 +339,49 @@
     result = PyString_FromStringAndSize(NULL, size)
     dst_buf = <unsigned char*>PyString_AS_STRING(result)
 
-    out = dst_buf
-    while (data < top):
-        cmd = data[0]
-        data = data + 1
-        if (cmd & 0x80):
-            # Copy instruction
-            data = _decode_copy_instruction(data, cmd, &cp_off, &cp_size)
-            if (cp_off + cp_size < cp_size or
-                cp_off + cp_size > source_size or
-                cp_size > size):
-                raise RuntimeError('Something wrong with:'
-                    ' cp_off = %s, cp_size = %s'
-                    ' source_size = %s, size = %s'
-                    % (cp_off, cp_size, source_size, size))
-            memcpy(out, source + cp_off, cp_size)
-            out = out + cp_size
-            size = size - cp_size
-        else:
-            # Insert instruction
-            if cmd == 0:
-                # cmd == 0 is reserved for future encoding
-                # extensions. In the mean time we must fail when
-                # encountering them (might be data corruption).
-                raise RuntimeError('Got delta opcode: 0, not supported')
-            if (cmd > size):
-                raise RuntimeError('Insert instruction longer than remaining'
-                    ' bytes: %d > %d' % (cmd, size))
-            memcpy(out, data, cmd)
-            out = out + cmd
-            data = data + cmd
-            size = size - cmd
+    failed = 0
+    with nogil:
+        out = dst_buf
+        while (data < top):
+            cmd = data[0]
+            data = data + 1
+            if (cmd & 0x80):
+                # Copy instruction
+                data = _decode_copy_instruction(data, cmd, &cp_off, &cp_size)
+                if (cp_off + cp_size < cp_size or
+                    cp_off + cp_size > source_size or
+                    cp_size > size):
+                    failed = 1
+                    break
+                memcpy(out, source + cp_off, cp_size)
+                out = out + cp_size
+                size = size - cp_size
+            else:
+                # Insert instruction
+                if cmd == 0:
+                    # cmd == 0 is reserved for future encoding
+                    # extensions. In the mean time we must fail when
+                    # encountering them (might be data corruption).
+                    failed = 2
+                    break
+                if cmd > size:
+                    failed = 3
+                    break
+                memcpy(out, data, cmd)
+                out = out + cmd
+                data = data + cmd
+                size = size - cmd
+    if failed:
+        if failed == 1:
+            raise ValueError('Something wrong with:'
+                ' cp_off = %s, cp_size = %s'
+                ' source_size = %s, size = %s'
+                % (cp_off, cp_size, source_size, size))
+        elif failed == 2:
+            raise ValueError('Got delta opcode: 0, not supported')
+        elif failed == 3:
+            raise ValueError('Insert instruction longer than remaining'
+                ' bytes: %d > %d' % (cmd, size))
 
     # sanity check
     if (data != top or size != 0):