Closes #17

32 bit width as a special test Fixed test so that they cover 32 bits width Added proptest. Added unit test
quickwit-oss · Jul 29, 2019 · c20ebf3 · c20ebf3
1 parent d1eea5c
commit c20ebf3
Show file tree

Hide file tree

Showing 8 changed files with 181 additions and 29 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,9 @@
+# bitpacking 0.8.1
+
+- Bugfix for arrays requiring 32-bits. (#17)
+- Added proptest (thanks to @danburkert)
+- Switched to edition 2018
+
 # bitpacking 0.8.0
 
 Enabling 1x, 4x, and 8x bitpacking based on feature flags.

diff --git a/Cargo.toml b/Cargo.toml
@@ -1,18 +1,20 @@
 [package]
 name = "bitpacking"
-version = "0.8.0"
+version = "0.8.1"
 authors = ["Paul Masurel <paul.masurel@gmail.com>"]
 license = "MIT"
 readme = "README.md"
 keywords = ["integer", "compression", "bitpacking"]
 description = "Fast integer compression/decompression via SIMD bit-packing. Port of simdcomp to rust."
+edition = "2018"
 
 [dependencies]
 crunchy = "0.2.2"
 
 [dev-dependencies]
 rand = "0.7"
 criterion = "0.2"
+proptest = "0.9.4"
 
 [features]
 bitpacker1x = []

diff --git a/src/bitpacker1x.rs b/src/bitpacker1x.rs
@@ -7,7 +7,7 @@ mod scalar {
     use std::ptr::write_unaligned as store_unaligned;
 
     use super::BLOCK_LEN;
-    use Available;
+    use crate::Available;
 
     type DataType = u32;
 

diff --git a/src/bitpacker4x.rs b/src/bitpacker4x.rs
@@ -1,15 +1,15 @@
 use super::{BitPacker, UnsafeBitPacker};
 
 #[cfg(target_arch = "x86_64")]
-use Available;
+use crate::Available;
 
 const BLOCK_LEN: usize = 32 * 4;
 
 #[cfg(any(target_arch = "x86_64"))]
 mod sse3 {
 
     use super::BLOCK_LEN;
-    use Available;
+    use crate::Available;
 
     use std::arch::x86_64::__m128i as DataType;
     use std::arch::x86_64::_mm_and_si128 as op_and;
@@ -66,8 +66,8 @@ mod sse3 {
 mod scalar {
 
     use super::BLOCK_LEN;
+    use crate::Available;
     use std::ptr;
-    use Available;
 
     type DataType = [u32; 4];
 
@@ -291,8 +291,9 @@ impl BitPacker for BitPacker4x {
 mod tests {
     use super::BLOCK_LEN;
     use super::{scalar, sse3};
-    use tests::test_util_compatible;
-    use Available;
+    use crate::tests::test_util_compatible;
+    use crate::Available;
+    use crate::{BitPacker, BitPacker4x};
 
     #[test]
     fn test_compatible() {
@@ -302,4 +303,37 @@ mod tests {
             );
         }
     }
+
+    #[test]
+    fn test_delta_bit_width_32() {
+        let values = vec![i32::max_value() as u32 + 1; BitPacker4x::BLOCK_LEN];
+        let bit_packer = BitPacker4x::new();
+        let bit_width = bit_packer.num_bits_sorted(0, &values);
+        assert_eq!(bit_width, 32);
+
+        let mut block = vec![0u8; BitPacker4x::compressed_block_size(bit_width)];
+        bit_packer.compress_sorted(0, &values, &mut block, bit_width);
+
+        let mut decoded_values = vec![0x10101010; BitPacker4x::BLOCK_LEN];
+        bit_packer.decompress_sorted(0, &block, &mut decoded_values, bit_width);
+
+        assert_eq!(values, decoded_values);
+    }
+
+    #[test]
+    fn test_bit_width_32() {
+        let mut values = vec![i32::max_value() as u32 + 1; BitPacker4x::BLOCK_LEN];
+        values[0] = 0;
+        let bit_packer = BitPacker4x::new();
+        let bit_width = bit_packer.num_bits(&values);
+        assert_eq!(bit_width, 32);
+
+        let mut block = vec![0u8; BitPacker4x::compressed_block_size(bit_width)];
+        bit_packer.compress(&values, &mut block, bit_width);
+
+        let mut decoded_values = vec![0x10101010; BitPacker4x::BLOCK_LEN];
+        bit_packer.decompress(&block, &mut decoded_values, bit_width);
+
+        assert_eq!(values, decoded_values);
+    }
 }
diff --git a/src/bitpacker8x.rs b/src/bitpacker8x.rs
@@ -1,15 +1,15 @@
 use super::{BitPacker, UnsafeBitPacker};
 
 #[cfg(target_arch = "x86_64")]
-use Available;
+use crate::Available;
 
 const BLOCK_LEN: usize = 32 * 8;
 
 #[cfg(target_arch = "x86_64")]
 mod avx2 {
 
     use super::BLOCK_LEN;
-    use Available;
+    use crate::Available;
 
     use std::arch::x86_64::__m256i as DataType;
     use std::arch::x86_64::_mm256_and_si256 as op_and;
@@ -77,8 +77,8 @@ mod avx2 {
 mod scalar {
 
     use super::BLOCK_LEN;
+    use crate::Available;
     use std::ptr;
-    use Available;
 
     type DataType = [u32; 8];
 
@@ -325,8 +325,8 @@ impl BitPacker for BitPacker8x {
 mod tests {
     use super::BLOCK_LEN;
     use super::{avx2, scalar};
-    use tests::test_util_compatible;
-    use Available;
+    use crate::tests::test_util_compatible;
+    use crate::Available;
 
     #[test]
     fn test_compatible() {

diff --git a/src/lib.rs b/src/lib.rs
@@ -23,9 +23,6 @@ See the [`BitPacker` trait](./trait.BitPacker.html) for example usage.
 #![allow(unused_unsafe)]
 #![warn(missing_docs)]
 
-#[macro_use]
-extern crate crunchy;
-
 use std::marker::Sized;
 
 #[cfg(test)]
@@ -277,3 +274,49 @@ pub use bitpacker1x::BitPacker1x;
 pub use bitpacker4x::BitPacker4x;
 #[cfg(feature = "bitpacker8x")]
 pub use bitpacker8x::BitPacker8x;
+
+#[cfg(test)]
+mod functional_tests {
+    use crate::{BitPacker, BitPacker4x};
+    use proptest::prelude::*;
+
+    proptest! {
+        #[test]
+        #[ignore]
+        fn check_block(
+            values in prop::collection::vec(u32::arbitrary(), BitPacker4x::BLOCK_LEN),
+        ) {
+            let bit_packer = BitPacker4x::new();
+            let bit_width = bit_packer.num_bits(&*values);
+
+            let mut block = vec![0u8; BitPacker4x::compressed_block_size(bit_width)];
+            bit_packer.compress(&values, &mut block, bit_width);
+
+            let mut decoded_values = vec![0x10101010; BitPacker4x::BLOCK_LEN];
+            bit_packer.decompress(&block, &mut decoded_values, bit_width);
+
+            prop_assert_eq!(values, decoded_values);
+        }
+
+        #[ignore]
+        #[test]
+        fn check_sorted_block(
+            (values, init_value) in prop::collection::vec(u32::arbitrary(), BitPacker4x::BLOCK_LEN).prop_flat_map(|mut values| {
+                values.sort();
+                let min_value = values[0];
+                (Just(values), (0..=min_value))
+            }),
+        ) {
+            let bit_packer = BitPacker4x::new();
+            let bit_width = bit_packer.num_bits_sorted(init_value, &*values);
+
+            let mut block = vec![0u8; BitPacker4x::compressed_block_size(bit_width)];
+            bit_packer.compress_sorted(init_value, &values, &mut block, bit_width);
+
+            let mut decoded_values = vec![0x10101010; BitPacker4x::BLOCK_LEN];
+            bit_packer.decompress_sorted(init_value, &block, &mut decoded_values, bit_width);
+
+            prop_assert_eq!(values, decoded_values);
+        }
+    }
+}
diff --git a/src/macros.rs b/src/macros.rs
@@ -2,7 +2,10 @@ macro_rules! pack_unpack_with_bits {
 
     ($name:ident, $n:expr, $cpufeature:meta) => {
 
+
         mod $name {
+
+            use crunchy::unroll;
             use super::BLOCK_LEN;
             use super::{Sink, Transformer};
             use super::{DataType,
@@ -17,7 +20,6 @@ macro_rules! pack_unpack_with_bits {
             const NUM_BITS: usize = $n;
             const NUM_BYTES_PER_BLOCK: usize = NUM_BITS * BLOCK_LEN / 8;
 
-
             #[$cpufeature]
             pub(crate) unsafe fn pack<TDeltaComputer: Transformer>(input_arr: &[u32], output_arr: &mut [u8], mut delta_computer: TDeltaComputer) -> usize {
                 assert_eq!(input_arr.len(), BLOCK_LEN, "Input block too small {}, (expected {})", input_arr.len(), BLOCK_LEN);
@@ -56,13 +58,8 @@ macro_rules! pack_unpack_with_bits {
                     }
                 }
                 let in_register: DataType = delta_computer.transform(load_unaligned(input_ptr.add(31)));
-                out_register =
-                    if NUM_BITS != 32 {
-                        let shifted = left_shift_32(in_register, 32 - NUM_BITS as i32);
-                        op_or(out_register, shifted)
-                    } else {
-                        in_register
-                    };
+                let shifted = left_shift_32(in_register, 32 - NUM_BITS as i32);
+                out_register = op_or(out_register, shifted);
                 store_unaligned(output_ptr, out_register);
 
                 NUM_BYTES_PER_BLOCK
@@ -121,10 +118,80 @@ macro_rules! pack_unpack_with_bits {
     }
 }
 
+macro_rules! pack_unpack_with_bits_32 {
+    ($cpufeature:meta) => {
+        mod pack_unpack_with_bits_32 {
+            use super::BLOCK_LEN;
+            use super::{load_unaligned, store_unaligned, DataType};
+            use super::{Sink, Transformer};
+            use crunchy::unroll;
+
+            const NUM_BITS: usize = 32;
+            const NUM_BYTES_PER_BLOCK: usize = NUM_BITS * BLOCK_LEN / 8;
+
+            #[$cpufeature]
+            pub(crate) unsafe fn pack<TDeltaComputer: Transformer>(
+                input_arr: &[u32],
+                output_arr: &mut [u8],
+                mut delta_computer: TDeltaComputer,
+            ) -> usize {
+                assert_eq!(
+                    input_arr.len(),
+                    BLOCK_LEN,
+                    "Input block too small {}, (expected {})",
+                    input_arr.len(),
+                    BLOCK_LEN
+                );
+                assert!(
+                    output_arr.len() >= NUM_BYTES_PER_BLOCK,
+                    "Output array too small (numbits {}). {} <= {}",
+                    NUM_BITS,
+                    output_arr.len(),
+                    NUM_BYTES_PER_BLOCK
+                );
+
+                let input_ptr: *const DataType = input_arr.as_ptr() as *const DataType;
+                let output_ptr = output_arr.as_mut_ptr() as *mut DataType;
+                unroll! {
+                    for i in 0..32 {
+                        let input_offset_ptr = input_ptr.offset(i as isize);
+                        let output_offset_ptr = output_ptr.offset(i as isize);
+                        let input_register = load_unaligned(input_offset_ptr);
+                        let output_register = delta_computer.transform(input_register);
+                        store_unaligned(output_offset_ptr, output_register);
+                    }
+                }
+                NUM_BYTES_PER_BLOCK
+            }
+
+            #[$cpufeature]
+            pub(crate) unsafe fn unpack<Output: Sink>(
+                compressed: &[u8],
+                mut output: Output,
+            ) -> usize {
+                assert!(
+                    compressed.len() >= NUM_BYTES_PER_BLOCK,
+                    "Compressed array seems too small. ({} < {}) ",
+                    compressed.len(),
+                    NUM_BYTES_PER_BLOCK
+                );
+                let input_ptr = compressed.as_ptr() as *const DataType;
+                for i in 0..32 {
+                    let input_offset_ptr = input_ptr.offset(i as isize);
+                    let in_register: DataType = load_unaligned(input_offset_ptr);
+                    output.process(in_register);
+                }
+                NUM_BYTES_PER_BLOCK
+            }
+        }
+    };
+}
+
 macro_rules! declare_bitpacker {
     ($cpufeature:meta) => {
         use super::super::UnsafeBitPacker;
-        use most_significant_bit;
+        use crate::most_significant_bit;
+        use crunchy::unroll;
 
         pack_unpack_with_bits!(pack_unpack_with_bits_1, 1, $cpufeature);
         pack_unpack_with_bits!(pack_unpack_with_bits_2, 2, $cpufeature);
@@ -157,7 +224,7 @@ macro_rules! declare_bitpacker {
         pack_unpack_with_bits!(pack_unpack_with_bits_29, 29, $cpufeature);
         pack_unpack_with_bits!(pack_unpack_with_bits_30, 30, $cpufeature);
         pack_unpack_with_bits!(pack_unpack_with_bits_31, 31, $cpufeature);
-        pack_unpack_with_bits!(pack_unpack_with_bits_32, 32, $cpufeature);
+        pack_unpack_with_bits_32!($cpufeature);
 
         unsafe fn compress_generic<DeltaComputer: Transformer>(
             decompressed: &[u32],
@@ -432,9 +499,9 @@ macro_rules! declare_bitpacker {
         #[cfg(test)]
         mod tests {
             use super::UnsafeBitPackerImpl;
-            use tests::test_suite_compress_decompress;
-            use Available;
-            use UnsafeBitPacker;
+            use crate::tests::test_suite_compress_decompress;
+            use crate::Available;
+            use crate::UnsafeBitPacker;
 
             #[test]
             fn test_num_bits() {

diff --git a/src/tests.rs b/src/tests.rs
@@ -132,7 +132,7 @@ fn test_util_compress_decompress_delta<TBitPacker: UnsafeBitPacker>(
 pub(crate) fn test_suite_compress_decompress<TBitPacker: UnsafeBitPacker>(delta: bool) {
     let num_blocks = (1 << 15) / TBitPacker::BLOCK_LEN;
     let n = num_blocks * TBitPacker::BLOCK_LEN;
-    for num_bits in 0u8..32u8 {
+    for num_bits in 0u8..33u8 {
         let original = generate_array(n, num_bits);
         for i in 0..num_blocks {
             let block = &original[i * TBitPacker::BLOCK_LEN..(i + 1) * TBitPacker::BLOCK_LEN];