From d2d6fd538beab18e1ca729c53ab3c261e672c119 Mon Sep 17 00:00:00 2001 From: Ilan Schnell Date: Tue, 28 Nov 2023 22:15:16 -0600 Subject: [PATCH] simplify implementation of copy_n() (remove special cases) --- CHANGE_LOG | 1 + bitarray/_bitarray.c | 90 ++++++++-------------- bitarray/copy_n.txt | 100 ------------------------ bitarray/test_bitarray.py | 2 +- examples/copy_n.py | 158 +++++++++++++++++++++----------------- 5 files changed, 123 insertions(+), 228 deletions(-) delete mode 100644 bitarray/copy_n.txt diff --git a/CHANGE_LOG b/CHANGE_LOG index 1ca99447..d06c070e 100644 --- a/CHANGE_LOG +++ b/CHANGE_LOG @@ -1,5 +1,6 @@ 2023-XX-XX 2.8.4: ------------------- + * simplify implementation of `copy_n()` (remove special cases) * improve documentation and testing diff --git a/bitarray/_bitarray.c b/bitarray/_bitarray.c index 8bd79610..dd603b92 100644 --- a/bitarray/_bitarray.c +++ b/bitarray/_bitarray.c @@ -226,7 +226,7 @@ shift_r8(bitarrayobject *self, Py_ssize_t a, Py_ssize_t b, int n, int bebr) return; /* as the big-endian representation has reversed bit order in each - byte, we reverse each byte, and (re-) reverse again below */ + byte, we reverse each byte, and (re-) reverse again at the end */ if (bebr && IS_BE(self)) bytereverse(self, a, b); @@ -268,16 +268,17 @@ shift_r8(bitarrayobject *self, Py_ssize_t a, Py_ssize_t b, int n, int bebr) } /* copy n bits from other (starting at b) onto self (starting at a), - please find details about how this function works in copy_n.txt */ + see also: examples/copy_n.py */ static void copy_n(bitarrayobject *self, Py_ssize_t a, bitarrayobject *other, Py_ssize_t b, Py_ssize_t n) { - Py_ssize_t p1 = a / 8; - Py_ssize_t p2 = (a + n - 1) / 8; - Py_ssize_t p3 = b / 8; - int sa = a % 8; - int sb = -(b % 8); + Py_ssize_t p1 = a / 8; /* first byte to copied to */ + Py_ssize_t p2 = (a + n - 1) / 8; /* last byte to be copied to */ + Py_ssize_t p3 = b / 8; /* first byte to be copied from */ + int sa = a % 8, sb = -(b % 8), be = IS_BE(self); + char t3 = 0; /* silence uninitialized warning on some compilers */ + Py_ssize_t i; assert(0 <= n && n <= self->nbits && n <= other->nbits); assert(0 <= a && a <= self->nbits - n); @@ -286,59 +287,32 @@ copy_n(bitarrayobject *self, Py_ssize_t a, if (n == 0 || (self == other && a == b)) return; - if (sa == 0 && sb == 0) { /***** aligned case *****/ - char *cp2 = self->ob_item + p2; - char m2 = ones_table[IS_BE(self)][(a + n) % 8]; - char t2 = *cp2; - - assert(p1 + BYTES(n) == p2 + 1 && p1 <= p2); - - memmove(self->ob_item + p1, other->ob_item + p3, (size_t) BYTES(n)); + if (sa + sb < 0) { + t3 = other->ob_item[p3++]; /* store byte in case other == self */ + sb += 8; + } + assert(a - sa == 8 * p1 && b + sb == 8 * p3); + assert(p1 <= p2 && 8 * p2 < a + n && a + n <= 8 * (p2 + 1)); + if (n > sb) { + Py_ssize_t m = BYTES(n - sb); + char *cp1 = self->ob_item + p1, m1 = ones_table[be][sa]; + char *cp2 = self->ob_item + p2, m2 = ones_table[be][(a + n) % 8]; + char t1 = *cp1, t2 = *cp2; + + assert(p1 + m == p2 || p1 + m == p2 + 1); + assert(p1 + m <= Py_SIZE(self) && p3 + m <= Py_SIZE(other)); + memmove(self->ob_item + p1, other->ob_item + p3, (size_t) m); if (self->endian != other->endian) - bytereverse(self, p1, p2 + 1); - - if (m2) /* restore bits overwritten by highest copied byte */ - *cp2 = (*cp2 & m2) | (t2 & ~m2); - } - else if (n < 8) { /***** small n case *****/ - Py_ssize_t i; - - if (a <= b) { /* loop forward (delete) */ - for (i = 0; i < n; i++) - setbit(self, i + a, getbit(other, i + b)); - } - else { /* loop backwards (insert) */ - for (i = n - 1; i >= 0; i--) - setbit(self, i + a, getbit(other, i + b)); - } - } - else { /***** general case *****/ - char *cp1 = self->ob_item + p1; - char *cp2 = self->ob_item + p2; - char m1 = ones_table[IS_BE(self)][sa]; - char m2 = ones_table[IS_BE(self)][(a + n) % 8]; - char t1 = *cp1, t2 = *cp2, t3 = other->ob_item[p3]; - Py_ssize_t i; - - assert(n >= 8 && cp1 <= cp2); - assert(a - sa == 8 * p1); /* useful equations */ - assert(b + sb == 8 * p3); - assert(a + n > 8 * p2); - - if (sa + sb < 0) - sb += 8; - copy_n(self, a - sa, other, b + sb, n - sb); /* aligned copy */ - shift_r8(self, p1, p2 + 1, sa + sb, 1); /* right shift */ - - if (m1) /* restore bits at p1 */ - *cp1 = (*cp1 & ~m1) | (t1 & m1); - - if (m2 && sa + sb) /* if shifted, restore bits at p2 */ - *cp2 = (*cp2 & m2) | (t2 & ~m2); + bytereverse(self, p1, p1 + m); - for (i = 0; i < sb; i++) /* copy first bits missed by copy_n() */ - setbit(self, i + a, t3 & BITMASK(other, i + b)); + shift_r8(self, p1, p2 + 1, sa + sb, 1); /* right shift by sa + sb */ + if (m1) + *cp1 = (*cp1 & ~m1) | (t1 & m1); /* restore bits at p1 */ + if (m2) + *cp2 = (*cp2 & m2) | (t2 & ~m2); /* restore bits at p2 */ } + for (i = 0; i < sb && i < n; i++) /* copy first sb bits */ + setbit(self, i + a, t3 & BITMASK(other, i + b)); } /* starting at start, delete n bits from self */ @@ -705,7 +679,7 @@ extend_bytes01(bitarrayobject *self, PyObject *bytes) const Py_ssize_t original_nbits = self->nbits; unsigned char c; char *data; - int vi = 0; /* to avoid uninitialized warning for some compilers */ + int vi = 0; /* silence uninitialized warning on some compilers */ assert(PyBytes_Check(bytes)); data = PyBytes_AS_STRING(bytes); diff --git a/bitarray/copy_n.txt b/bitarray/copy_n.txt deleted file mode 100644 index 703e2106..00000000 --- a/bitarray/copy_n.txt +++ /dev/null @@ -1,100 +0,0 @@ -copy_n() in _bitarray.c -======================= - -The following variable names are used to in this document, as well as in -the source code: - - self bitarray object bits are copies onto - a start bit (in self) bits are copied onto - other bitarray object bits are copied from - b start bit (in other) bits are copied from - n number of bits being copied - p1 start byte (in self) memory is copied to, i.e. a / 8 - p2 last byte (in self) memory is copied to, i.e. (a + n - 1) / 8 - p3 first byte (in other) memory is copied from, i.e. b / 8 - -There are 3 cases handled by this function: (i) aligned case (ii) small n case -(iii) general case. For all cases, it is important to handle self == other. -We will briefly discuss the first two cases, and then go into more detail -about the general case. - - -Aligned case ------------- - -In the aligned case, i.e. when both start positions (a and b) are a -multiple of 8, we use memmove() on all bytes to be copied. This includes -any partial byte (p2), whose bits get restored afterwards. - - -Small n case ------------- - -For n smaller than 8 bits, we use a simple sequence of getbit() and setbit() -calls which is relatively slow. However, as the general case has some -overhead, it is still faster to copy a small number of bits one by one. -As other might be self, we need to either loop forward or backwards in order -to not copy from bits already changed. - - -General case ------------- - -Here we assume that n is at least 8 bits. We choose an aligned region to -be copied using copy_n() itself, and use shift_r8() and a few fixes to -create the correct copy. The is done in the following steps: - -1.) Calculate positions p1, p2, p3 and store the corresponding bytes from - memory into t1, t2, t3. We need these bytes later to restore bits which - got overwritten or shifted away. Note that we have to store t3 (the byte - with b in other), as other can be self. - -2.) Calculate the total right shift of bytes p1..p2 (once copied into self). - This shift depends on a % 8 and b % 8, and has to be below 8. - -3.) Using copy_n(), copy the byte region from other at p3 or p3 + 1 into - self at p1. Because in the latter case we miss the beginning bits in - other, we need t3 and copy those bits later. - -4.) Right shift self in byte range p1..p2. This region includes the - bit-range(a, a + n), but is generally larger. This is why we need t1 - and t2 to restore the bytes at p1 and p2 in self later. - -5.) Restore bits in self at, see step 4: - - p1 using t1 (those got overwritten and shifted) - - p2 using t2 (those got shifted) - -6.) Copy the first few bits from other to self (using t3, see step 3). - - -Here is an example with the following parameters (created using -examples/copy_n.py): - -a = 21 -b = 6 -n = 31 -p1 = 2 -p2 = 6 -p3 = 0 - -other -bitarray('00101110 11111001 01011101 11001011 10110000 01011110 011') -b..b+n ^^ ^^^^^^^^ ^^^^^^^^ ^^^^^^^^ ^^^^^ - ======== ======== ======== ===== - 33 -self -bitarray('01011101 11100101 01110101 01011001 01110100 10001010 01111011') -a..a+n ^^^ ^^^^^^^^ ^^^^^^^^ ^^^^^^^^ ^^^^ - 11111 - 2222 -copy_n - ======== ======== ======== ===== -bitarray('01011101 11100101 11111001 01011101 11001011 10110010 01111011') -rshift 7 - >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> -bitarray('01011101 11100101 00000001 11110010 10111011 10010111 01100100') -a..a+n = ======== ======== ======== ==== - 11111 - 2222 - 33 -bitarray('01011101 11100101 01110101 11110010 10111011 10010111 01101011') diff --git a/bitarray/test_bitarray.py b/bitarray/test_bitarray.py index 35606c01..f118fddc 100644 --- a/bitarray/test_bitarray.py +++ b/bitarray/test_bitarray.py @@ -731,7 +731,7 @@ def test_copy_n_explicit(self): self.assertEqual(x, y) def test_copy_n_example(self): - # example givin in bitarray/copy_n.txt + # example given in examples/copy_n.py y = bitarray( '00101110 11111001 01011101 11001011 10110000 01011110 011') x = bitarray( diff --git a/examples/copy_n.py b/examples/copy_n.py index c2405ab2..1859d38a 100644 --- a/examples/copy_n.py +++ b/examples/copy_n.py @@ -2,7 +2,38 @@ The purpose of this script is to illustrate how copy_n() in _bitarray.c works. This is essentially a Python implementation of copy_n() with output of the different stages of the bitarray we copy into. -For more details, see also: bitarray/copy_n.txt + +Sample output: +a = 21 +b = 6 +n = 31 +p1 = 2 +p2 = 6 +p3 = 0 +sa = 5 +sb = -6 + -> sb = 2 +other +bitarray('00101110 11111001 01011101 11001011 10110000 01011110 011') +b..b+n ^^ ^^^^^^^^ ^^^^^^^^ ^^^^^^^^ ^^^^^ + ======== ======== ======== ======== + 33 +self +bitarray('01011101 11100101 01110101 01011001 01110100 10001010 01111011') +a..a+n ^^^ ^^^^^^^^ ^^^^^^^^ ^^^^^^^^ ^^^^ + 11111 + 2222 +memmove 4 + ======== ======== ======== ======== +bitarray('01011101 11100101 11111001 01011101 11001011 10110000 01111011') +rshift 7 + >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> +bitarray('01011101 11100101 00000001 11110010 10111011 10010111 01100000') + = ======== ======== ======== ======== + 11111 + 2222 + 33 +bitarray('01011101 11100101 01110101 11110010 10111011 10010111 01101011') """ from random import getrandbits, randrange, randint from io import StringIO @@ -52,6 +83,7 @@ def copy_n(self, a, other, b, n): p3 = b // 8 sa = a % 8 sb = -(b % 8) + t3 = 0 assert 0 <= n <= min(len(self), len(other)) assert 0 <= a <= len(self) - n @@ -59,101 +91,87 @@ def copy_n(self, a, other, b, n): if n == 0 or (self is other and a == b): return - if sa == 0 and sb == 0: # aligned case - m = bits2bytes(n) - - assert p1 + m == p2 + 1 - m2 = ones_table[is_be(self)][(a + n) % 8] - t2 = memoryview(self)[p2] - - memoryview(self)[p1:p1 + m] = memoryview(other)[p3:p3 + m] - if self.endian() != other.endian(): - self.bytereverse(p1, p2 + 1) - - if m2: # restore bits overwritten by highest copied byte - memoryview(self)[p2] = (memoryview(self)[p2] & m2) | (t2 & ~m2) - - elif n < 8: # small n case - if a <= b: # loop forward (delete) - for i in range(n): - self[i + a] = other[i + b] - else: # loop backwards (insert) - for i in range(n - 1, -1, -1): - self[i + a] = other[i + b] + if verbose: + print('a =', a) + print('b =', b) + print('n =', n) + print('p1 =', p1) + print('p2 =', p2) + print('p3 =', p3) + print('sa =', sa) + print('sb =', sb) + + if sa + sb < 0: + # In order to keep total right shift (sa + sb) positive, we + # increase the first byte to be copied from (p3) by one byte, + # such that memmove() will move all bytes one extra to the left. + t3 = memoryview(other)[p3] + p3 += 1 + sb += 8 + if verbose: + print(' -> sb =', sb) + + assert a - sa == 8 * p1 and b + sb == 8 * p3 + assert p1 <= p2 and 8 * p2 < a + n <= 8 * (p2 + 1) + + if verbose: + print('other') + pprint(other) + mark_range_n(b, n, '^', 'b..b+n') + if n > sb: + mark_range_n(8 * p3, 8 * bits2bytes(n - sb), '=') + mark_range_n(b, sb, '3') + print('self') + pprint(self) + mark_range_n(a, n, '^', 'a..a+n') + if n > sb: + mark_range(8 * p1, a, '1') + mark_range(a + n, 8 * p2 + 8, '2') - else: # general case + if n > sb: + m = bits2bytes(n - sb) m1 = ones_table[is_be(self)][sa] m2 = ones_table[is_be(self)][(a + n) % 8] - - assert n >= 8 and p1 <= p2 - assert a - sa == 8 * p1 - assert b + sb == 8 * p3 - assert a + n > 8 * p2 - - if verbose: - print('a =', a) - print('b =', b) - print('n =', n) - print('p1 =', p1) - print('p2 =', p2) - print('p3 =', p3) - print('sa =', sa) - print('sb =', sb) - t1 = memoryview(self)[p1] t2 = memoryview(self)[p2] - t3 = memoryview(other)[p3] - - if sa + sb < 0: - sb += 8 - if verbose: - print(' -> sb =', sb) - - if verbose: - print('other') - pprint(other) - mark_range_n(b, n, '^', 'b..b+n') - mark_range_n(b + sb, n - sb, '=') - mark_range_n(b, sb, '3') - print('self') - pprint(self) - mark_range_n(a, n, '^', 'a..a+n') - mark_range(8 * p1, a, '1') - mark_range(a + n, 8 * p2 + 8, '2') + assert p1 + m == p2 or p1 + m == p2 + 1 + assert p1 + m <= self.nbytes and p3 + m <= other.nbytes - print('copy_n') - mark_range_n(a - sa, n - sb, '=') + # aligned copy -- copy first sb bits (if any) later + memoryview(self)[p1:p1 + m] = memoryview(other)[p3:p3 + m] + if self.endian() != other.endian(): + self.bytereverse(p1, p1 + m) - copy_n(self, a - sa, other, b + sb, n - sb) # aligned copy if verbose: + print('memmove', m) + mark_range_n(8 * p1, 8 * m, '=') pprint(self) - print('rshift', sa + sb) mark_range(8 * p1, 8 * (p2 + 1), '>') shift_r8(self, p1, p2 + 1, sa + sb) # right shift if verbose: pprint(self) - mark_range_n(8 * p1 + sa + sb, n - sb, '=', 'a..a+n') + mark_range(8 * p1 + sa + sb, 8 * (p2 + 1), '=') if m1: # restore bits at p1 if verbose: mark_range(8 * p1, a, '1') memoryview(self)[p1] = (memoryview(self)[p1] & ~m1) | (t1 & m1) - if m2 and sa + sb: # if shifted, restore bits at p2 + if m2: # restore bits at p2 if verbose: mark_range(a + n, 8 * p2 + 8, '2') memoryview(self)[p2] = (memoryview(self)[p2] & m2) | (t2 & ~m2) - if verbose: - mark_range_n(a, sb, '3') - for i in range(sb): # copy first bits missed by copy_n() - self[i + a] = bool(t3 & bitmask_table[is_be(other)][(i + b) % 8]) + if verbose: + mark_range_n(a, sb, '3') + for i in range(min(sb, n)): # copy first sb bits + self[i + a] = bool(t3 & bitmask_table[is_be(other)][(i + b) % 8]) - if verbose: - pprint(self) + if verbose: + pprint(self) def test_copy_n(): @@ -196,3 +214,5 @@ def random_endian(): copy_n(self, 21, other, 6, 31) assert self == bitarray( '01011101 11100101 01110101 11110010 10111011 10010111 01101011') + #copy_n(self, 2, other, 12, 1) + #copy_n(self, 9, other, 17, 23)