Reduce boxing in callback (#671)

* reduce extra boxing
close https://github.com/tikv/rust-rocksdb/issues/513Signed-off-by: Jayice <1185430411@qq.com>

* rename generic
close https://github.com/tikv/rust-rocksdb/issues/513Signed-off-by: Jayice <1185430411@qq.com>

* format code
close https://github.com/tikv/rust-rocksdb/issues/513Signed-off-by: Jayice <1185430411@qq.com>

src/rocksdb_options.rs

@@ -1387,20 +1387,17 @@ impl ColumnFamilyOptions {
     /// recent call to GetSnapshot() to filter.
     ///
     /// See also `CompactionFilter`.
-    pub fn set_compaction_filter<S>(
-        &mut self,
-        name: S,
-        filter: Box<dyn CompactionFilter>,
-    ) -> Result<(), String>
+    pub fn set_compaction_filter<S, C>(&mut self, name: S, filter: C) -> Result<(), String>
     where
         S: Into<Vec<u8>>,
+        C: CompactionFilter,
     {
         unsafe {
             let c_name = match CString::new(name) {
                 Ok(s) => s,
                 Err(e) => return Err(format!("failed to convert to cstring: {:?}", e)),
             };
-            let filter = new_compaction_filter(c_name, filter);
+            let filter = new_compaction_filter::<C>(c_name, filter);
             crocksdb_ffi::crocksdb_options_set_compaction_filter(self.inner, filter.inner);
             self.filter = Some(filter);
             Ok(())
@@ -1410,20 +1407,17 @@ impl ColumnFamilyOptions {
     /// Set compaction filter factory.
     ///
     /// See also `CompactionFilterFactory`.
-    pub fn set_compaction_filter_factory<S>(
-        &mut self,
-        name: S,
-        factory: Box<dyn CompactionFilterFactory>,
-    ) -> Result<(), String>
+    pub fn set_compaction_filter_factory<S, C>(&mut self, name: S, factory: C) -> Result<(), String>
     where
         S: Into<Vec<u8>>,
+        C: CompactionFilterFactory,
     {
         let c_name = match CString::new(name) {
             Ok(s) => s,
             Err(e) => return Err(format!("failed to convert to cstring: {:?}", e)),
         };
         unsafe {
-            let factory = new_compaction_filter_factory(c_name, factory)?;
+            let factory = new_compaction_filter_factory::<C>(c_name, factory)?;
             crocksdb_ffi::crocksdb_options_set_compaction_filter_factory(self.inner, factory.inner);
             std::mem::forget(factory); // Deconstructor will be called after `self` is dropped.
             Ok(())
@@ -1789,20 +1783,17 @@ impl ColumnFamilyOptions {
         unsafe { crocksdb_ffi::crocksdb_options_get_num_levels(self.inner) as usize }
     }
 
-    pub fn set_prefix_extractor<S>(
-        &mut self,
-        name: S,
-        transform: Box<dyn SliceTransform>,
-    ) -> Result<(), String>
+    pub fn set_prefix_extractor<S, ST>(&mut self, name: S, transform: ST) -> Result<(), String>
     where
         S: Into<Vec<u8>>,
+        ST: SliceTransform,
     {
         unsafe {
             let c_name = match CString::new(name) {
                 Ok(s) => s,
                 Err(e) => return Err(format!("failed to convert to cstring: {:?}", e)),
             };
-            let transform = new_slice_transform(c_name, transform)?;
+            let transform = new_slice_transform::<ST>(c_name, transform)?;
             crocksdb_ffi::crocksdb_options_set_prefix_extractor(self.inner, transform);
             Ok(())
         }
@@ -1814,20 +1805,21 @@ impl ColumnFamilyOptions {
         }
     }
 
-    pub fn set_memtable_insert_hint_prefix_extractor<S>(
+    pub fn set_memtable_insert_hint_prefix_extractor<S, ST>(
         &mut self,
         name: S,
-        transform: Box<dyn SliceTransform>,
+        transform: ST,
     ) -> Result<(), String>
     where
         S: Into<Vec<u8>>,
+        ST: SliceTransform,
     {
         unsafe {
             let c_name = match CString::new(name) {
                 Ok(s) => s,
                 Err(e) => return Err(format!("failed to convert to cstring: {:?}", e)),
             };
-            let transform = new_slice_transform(c_name, transform)?;
+            let transform = new_slice_transform::<ST>(c_name, transform)?;
             crocksdb_ffi::crocksdb_options_set_memtable_insert_with_hint_prefix_extractor(
                 self.inner, transform,
             );

src/slice_transform.rs

@@ -16,7 +16,7 @@ use libc::{c_char, c_void, size_t};
 use std::ffi::CString;
 use std::slice;
 
-// `SliceTranform` is a generic pluggable way of transforming one string
+// `SliceTransform` is a generic pluggable way of transforming one string
 // mainly used for prefix blooms.
 pub trait SliceTransform {
     // Extract a prefix from a specified key
@@ -38,29 +38,29 @@ pub trait SliceTransform {
 }
 
 #[repr(C)]
-pub struct SliceTransformProxy {
+pub struct SliceTransformProxy<S: SliceTransform> {
     name: CString,
-    transform: Box<dyn SliceTransform>,
+    transform: S,
 }
 
-extern "C" fn name(transform: *mut c_void) -> *const c_char {
-    unsafe { (*(transform as *mut SliceTransformProxy)).name.as_ptr() }
+extern "C" fn name<S: SliceTransform>(transform: *mut c_void) -> *const c_char {
+    unsafe { (*(transform as *mut SliceTransformProxy<S>)).name.as_ptr() }
 }
 
-extern "C" fn destructor(transform: *mut c_void) {
+extern "C" fn destructor<S: SliceTransform>(transform: *mut c_void) {
     unsafe {
-        Box::from_raw(transform as *mut SliceTransformProxy);
+        Box::from_raw(transform as *mut SliceTransformProxy<S>);
     }
 }
 
-extern "C" fn transform(
+extern "C" fn transform<S: SliceTransform>(
     transform: *mut c_void,
     key: *const u8,
     key_len: size_t,
     dest_len: *mut size_t,
 ) -> *const u8 {
     unsafe {
-        let transform = &mut *(transform as *mut SliceTransformProxy);
+        let transform = &mut *(transform as *mut SliceTransformProxy<S>);
         let key = slice::from_raw_parts(key, key_len);
         let prefix = transform.transform.transform(key);
         *dest_len = prefix.len() as size_t;
@@ -68,25 +68,33 @@ extern "C" fn transform(
     }
 }
 
-extern "C" fn in_domain(transform: *mut c_void, key: *const u8, key_len: size_t) -> u8 {
+extern "C" fn in_domain<S: SliceTransform>(
+    transform: *mut c_void,
+    key: *const u8,
+    key_len: size_t,
+) -> u8 {
     unsafe {
-        let transform = &mut *(transform as *mut SliceTransformProxy);
+        let transform = &mut *(transform as *mut SliceTransformProxy<S>);
         let key = slice::from_raw_parts(key, key_len);
         transform.transform.in_domain(key) as u8
     }
 }
 
-extern "C" fn in_range(transform: *mut c_void, key: *const u8, key_len: size_t) -> u8 {
+extern "C" fn in_range<S: SliceTransform>(
+    transform: *mut c_void,
+    key: *const u8,
+    key_len: size_t,
+) -> u8 {
     unsafe {
-        let transform = &mut *(transform as *mut SliceTransformProxy);
+        let transform = &mut *(transform as *mut SliceTransformProxy<S>);
         let key = slice::from_raw_parts(key, key_len);
         transform.transform.in_range(key) as u8
     }
 }
 
-pub unsafe fn new_slice_transform(
+pub unsafe fn new_slice_transform<S: SliceTransform>(
     c_name: CString,
-    f: Box<dyn SliceTransform>,
+    f: S,
 ) -> Result<*mut DBSliceTransform, String> {
     let proxy = Box::into_raw(Box::new(SliceTransformProxy {
         name: c_name,
@@ -94,11 +102,11 @@ pub unsafe fn new_slice_transform(
     }));
     let transform = crocksdb_ffi::crocksdb_slicetransform_create(
         proxy as *mut c_void,
-        destructor,
-        transform,
-        in_domain,
-        in_range,
-        name,
+        destructor::<S>,
+        transform::<S>,
+        in_domain::<S>,
+        in_range::<S>,
+        name::<S>,
     );
     Ok(transform)
 }

tests/cases/test_compaction_filter.rs

@@ -55,12 +55,12 @@ fn test_compaction_filter() {
     // reregister with ignore_snapshots set to true
     let mut cf_opts = ColumnFamilyOptions::new();
     cf_opts
-        .set_compaction_filter(
+        .set_compaction_filter::<&str, Filter>(
             "test",
-            Box::new(Filter {
+            Filter {
                 drop_called: drop_called.clone(),
                 filtered_kvs: filtered_kvs.clone(),
-            }),
+            },
         )
         .unwrap();
 

tests/cases/test_delete_range.rs

@@ -639,9 +639,9 @@ fn test_delete_range_prefix_bloom_case_1() {
     let mut cf_opts = ColumnFamilyOptions::new();
     // Prefix extractor(trim the timestamp at tail) for write cf.
     cf_opts
-        .set_prefix_extractor(
+        .set_prefix_extractor::<&str, FixedSuffixSliceTransform>(
             "FixedSuffixSliceTransform",
-            Box::new(FixedSuffixSliceTransform::new(3)),
+            FixedSuffixSliceTransform::new(3),
         )
         .unwrap_or_else(|err| panic!(format!("{:?}", err)));
     // Create prefix bloom filter for memtable.
@@ -714,9 +714,9 @@ fn test_delete_range_prefix_bloom_case_2() {
     let mut cf_opts = ColumnFamilyOptions::new();
     // Prefix extractor(trim the timestamp at tail) for write cf.
     cf_opts
-        .set_prefix_extractor(
+        .set_prefix_extractor::<&str, FixedSuffixSliceTransform>(
             "FixedSuffixSliceTransform",
-            Box::new(FixedSuffixSliceTransform::new(3)),
+            FixedSuffixSliceTransform::new(3),
         )
         .unwrap_or_else(|err| panic!(format!("{:?}", err)));
     // Create prefix bloom filter for memtable.
@@ -770,9 +770,9 @@ fn test_delete_range_prefix_bloom_case_2() {
     let mut cf_opts = ColumnFamilyOptions::new();
     // Prefix extractor(trim the timestamp at tail) for write cf.
     cf_opts
-        .set_prefix_extractor(
+        .set_prefix_extractor::<&str, FixedSuffixSliceTransform>(
             "FixedSuffixSliceTransform",
-            Box::new(FixedSuffixSliceTransform::new(3)),
+            FixedSuffixSliceTransform::new(3),
         )
         .unwrap_or_else(|err| panic!(format!("{:?}", err)));
     // Create prefix bloom filter for memtable.
@@ -807,9 +807,9 @@ fn test_delete_range_prefix_bloom_case_3() {
     let mut cf_opts = ColumnFamilyOptions::new();
     // Prefix extractor(trim the timestamp at tail) for write cf.
     cf_opts
-        .set_prefix_extractor(
+        .set_prefix_extractor::<&str, FixedSuffixSliceTransform>(
             "FixedSuffixSliceTransform",
-            Box::new(FixedSuffixSliceTransform::new(3)),
+            FixedSuffixSliceTransform::new(3),
         )
         .unwrap_or_else(|err| panic!(format!("{:?}", err)));
     // Create prefix bloom filter for memtable.
@@ -852,9 +852,9 @@ fn test_delete_range_prefix_bloom_case_3() {
     let mut cf_opts = ColumnFamilyOptions::new();
     // Prefix extractor(trim the timestamp at tail) for write cf.
     cf_opts
-        .set_prefix_extractor(
+        .set_prefix_extractor::<&str, FixedSuffixSliceTransform>(
             "FixedSuffixSliceTransform",
-            Box::new(FixedSuffixSliceTransform::new(3)),
+            FixedSuffixSliceTransform::new(3),
         )
         .unwrap_or_else(|err| panic!(format!("{:?}", err)));
     // Create prefix bloom filter for memtable.
@@ -903,9 +903,9 @@ fn test_delete_range_prefix_bloom_case_4() {
     let mut cf_opts = ColumnFamilyOptions::new();
     // Prefix extractor(trim the timestamp at tail) for write cf.
     cf_opts
-        .set_prefix_extractor(
+        .set_prefix_extractor::<&str, FixedSuffixSliceTransform>(
             "FixedSuffixSliceTransform",
-            Box::new(FixedSuffixSliceTransform::new(3)),
+            FixedSuffixSliceTransform::new(3),
         )
         .unwrap_or_else(|err| panic!(format!("{:?}", err)));
     // Create prefix bloom filter for memtable.
@@ -948,9 +948,9 @@ fn test_delete_range_prefix_bloom_case_4() {
     let mut cf_opts = ColumnFamilyOptions::new();
     // Prefix extractor(trim the timestamp at tail) for write cf.
     cf_opts
-        .set_prefix_extractor(
+        .set_prefix_extractor::<&str, FixedSuffixSliceTransform>(
             "FixedSuffixSliceTransform",
-            Box::new(FixedSuffixSliceTransform::new(3)),
+            FixedSuffixSliceTransform::new(3),
         )
         .unwrap_or_else(|err| panic!(format!("{:?}", err)));
     // Create prefix bloom filter for memtable.
@@ -1000,9 +1000,9 @@ fn test_delete_range_prefix_bloom_case_5() {
     let mut cf_opts = ColumnFamilyOptions::new();
     // Prefix extractor(trim the timestamp at tail) for write cf.
     cf_opts
-        .set_prefix_extractor(
+        .set_prefix_extractor::<&str, FixedSuffixSliceTransform>(
             "FixedSuffixSliceTransform",
-            Box::new(FixedSuffixSliceTransform::new(3)),
+            FixedSuffixSliceTransform::new(3),
         )
         .unwrap_or_else(|err| panic!(format!("{:?}", err)));
     // Create prefix bloom filter for memtable.
@@ -1043,9 +1043,9 @@ fn test_delete_range_prefix_bloom_case_5() {
     let mut cf_opts = ColumnFamilyOptions::new();
     // Prefix extractor(trim the timestamp at tail) for write cf.
     cf_opts
-        .set_prefix_extractor(
+        .set_prefix_extractor::<&str, FixedSuffixSliceTransform>(
             "FixedSuffixSliceTransform",
-            Box::new(FixedSuffixSliceTransform::new(3)),
+            FixedSuffixSliceTransform::new(3),
         )
         .unwrap_or_else(|err| panic!(format!("{:?}", err)));
     // Create prefix bloom filter for memtable.
@@ -1092,9 +1092,9 @@ fn test_delete_range_prefix_bloom_case_6() {
     let mut cf_opts = ColumnFamilyOptions::new();
     // Prefix extractor(trim the timestamp at tail) for write cf.
     cf_opts
-        .set_prefix_extractor(
+        .set_prefix_extractor::<&str, FixedSuffixSliceTransform>(
             "FixedSuffixSliceTransform",
-            Box::new(FixedSuffixSliceTransform::new(3)),
+            FixedSuffixSliceTransform::new(3),
         )
         .unwrap_or_else(|err| panic!(format!("{:?}", err)));
     // Create prefix bloom filter for memtable.
@@ -1137,9 +1137,9 @@ fn test_delete_range_prefix_bloom_case_6() {
     let mut cf_opts = ColumnFamilyOptions::new();
     // Prefix extractor(trim the timestamp at tail) for write cf.
     cf_opts
-        .set_prefix_extractor(
+        .set_prefix_extractor::<&str, FixedSuffixSliceTransform>(
             "FixedSuffixSliceTransform",
-            Box::new(FixedSuffixSliceTransform::new(3)),
+            FixedSuffixSliceTransform::new(3),
         )
         .unwrap_or_else(|err| panic!(format!("{:?}", err)));
     // Create prefix bloom filter for memtable.
@@ -1210,9 +1210,9 @@ fn test_delete_range_prefix_bloom_compact_case() {
     let mut cf_opts = ColumnFamilyOptions::new();
     // Prefix extractor(trim the timestamp at tail) for write cf.
     cf_opts
-        .set_prefix_extractor(
+        .set_prefix_extractor::<&str, FixedSuffixSliceTransform>(
             "FixedSuffixSliceTransform",
-            Box::new(FixedSuffixSliceTransform::new(3)),
+            FixedSuffixSliceTransform::new(3),
         )
         .unwrap_or_else(|err| panic!(format!("{:?}", err)));
     // Create prefix bloom filter for memtable.
@@ -1255,9 +1255,9 @@ fn test_delete_range_prefix_bloom_compact_case() {
     let mut cf_opts = ColumnFamilyOptions::new();
     // Prefix extractor(trim the timestamp at tail) for write cf.
     cf_opts
-        .set_prefix_extractor(
+        .set_prefix_extractor::<&str, FixedSuffixSliceTransform>(
             "FixedSuffixSliceTransform",
-            Box::new(FixedSuffixSliceTransform::new(3)),
+            FixedSuffixSliceTransform::new(3),
         )
         .unwrap_or_else(|err| panic!(format!("{:?}", err)));
     // Create prefix bloom filter for memtable.

tests/cases/test_iterator.rs

@@ -295,9 +295,9 @@ fn test_total_order_seek() {
     opts.create_if_missing(true);
     cf_opts.set_block_based_table_factory(&bbto);
     cf_opts
-        .set_prefix_extractor(
+        .set_prefix_extractor::<&str, FixedPrefixTransform>(
             "FixedPrefixTransform",
-            Box::new(FixedPrefixTransform { prefix_len: 2 }),
+            FixedPrefixTransform { prefix_len: 2 },
         )
         .unwrap();
     // also create prefix bloom for memtable
@@ -383,9 +383,9 @@ fn test_fixed_suffix_seek() {
     opts.create_if_missing(true);
     cf_opts.set_block_based_table_factory(&bbto);
     cf_opts
-        .set_prefix_extractor(
+        .set_prefix_extractor::<&str, FixedSuffixTransform>(
             "FixedSuffixTransform",
-            Box::new(FixedSuffixTransform { suffix_len: 2 }),
+            FixedSuffixTransform { suffix_len: 2 },
         )
         .unwrap();
 
@@ -428,7 +428,7 @@ fn test_iter_sequence_number() {
         }
     }
     let (tx, rx) = mpsc::sync_channel(8);
-    let filter = Box::new(TestCompactionFilter(tx));
+    let filter = TestCompactionFilter(tx);
 
     let path = tempdir_with_prefix("_rust_rocksdb_sequence_number");
     let mut opts = DBOptions::new();
@@ -436,7 +436,9 @@ fn test_iter_sequence_number() {
     let mut cf_opts = ColumnFamilyOptions::new();
     cf_opts.set_disable_auto_compactions(true);
     cf_opts.set_num_levels(7);
-    cf_opts.set_compaction_filter("test", filter).unwrap();
+    cf_opts
+        .set_compaction_filter::<&str, TestCompactionFilter>("test", filter)
+        .unwrap();
     let db = DB::open_cf(
         opts,
         path.path().to_str().unwrap(),

tests/cases/test_prefix_extractor.rs

@@ -60,9 +60,9 @@ fn test_prefix_extractor_compatibility() {
         opts.create_if_missing(false);
         cf_opts.set_block_based_table_factory(&bbto);
         cf_opts
-            .set_prefix_extractor(
+            .set_prefix_extractor::<&str, FixedPrefixTransform>(
                 "FixedPrefixTransform",
-                Box::new(FixedPrefixTransform { prefix_len: 2 }),
+                FixedPrefixTransform { prefix_len: 2 },
             )
             .unwrap();
         // also create prefix bloom for memtable

tests/cases/test_rocksdb_options.rs

@@ -124,9 +124,9 @@ fn test_memtable_insert_hint_prefix_extractor() {
     let mut cf_opts = ColumnFamilyOptions::new();
     opts.create_if_missing(true);
     cf_opts
-        .set_memtable_insert_hint_prefix_extractor(
+        .set_memtable_insert_hint_prefix_extractor::<&str, FixedPrefixTransform>(
             "FixedPrefixTransform",
-            Box::new(FixedPrefixTransform { prefix_len: 2 }),
+            FixedPrefixTransform { prefix_len: 2 },
         )
         .unwrap();
     let db = DB::open_cf(

tests/cases/test_slice_transform.rs

@@ -46,7 +46,10 @@ fn test_slice_transform() {
     cf_opts.set_memtable_prefix_bloom_size_ratio(0.25);
 
     cf_opts
-        .set_prefix_extractor("test", Box::new(FixedPostfixTransform { postfix_len: 2 }))
+        .set_prefix_extractor::<&str, FixedPostfixTransform>(
+            "test",
+            FixedPostfixTransform { postfix_len: 2 },
+        )
         .unwrap();
     opts.create_if_missing(true);