Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce boxing in callback structs #513

Closed
yiwu-arbug opened this issue Jun 21, 2020 · 1 comment · Fixed by #663 or #671
Closed

Reduce boxing in callback structs #513

yiwu-arbug opened this issue Jun 21, 2020 · 1 comment · Fixed by #663 or #671
Labels
difficulty/medium Medium task. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. sig/engine SIG: Engine

Comments

@yiwu-arbug
Copy link
Collaborator

yiwu-arbug commented Jun 21, 2020

RocksDB expose several APIs to allow client code to inject custom logic, such as compaction filter and table properties collector. rust-rocksdb usually implement such APIs in the follow way.

Assume rocksdb expose such a struct for callback

// C++
class Callback {
    virtual void SomeCallback() = 0;
}

rust-rocksdb usually provide the API in the following way:

// Rust
pub trait Callback {
    fn callback();
}

pub fn new_callback<C: Callback>(callback: C) -> *mut DBCallback {
    let boxed: Box<dyn Callback> = Box::new(callback) as _;
    unsafe {
        crocksdb_ffi::crocksdb_callback_create(
            Box::into_raw(Box::new(boxed)),
            callback_fn,
        )
    }
}

extern "C" fn callback_fn(ctx: *mut c_void) {
    let callback = &*(ctx as *mut Box<dyn Callback>);
    callback.callback();
}

Note here ctx is a Box<Box<dyn Callback>>. The inner box is to hold the trait object. The outer box is needed since C code cannot hold the raw pointer of Box<dyn Callback> natively since the pointer is of 128-bits. However, the extra boxing can be reduced by using generic.

pub fn new_callback<C: Callback>(callback: C) -> *mut DBCallback {
    let boxed: Box<C> = Box::new(callback);
    unsafe {
        crocksdb_ffi::crocksdb_callback_create(
            Box::into_raw(boxed),
            callback_fn::<C>,
        )
    }
}

extern "C" fn callback_fn<C: Callback>(ctx: *mut c_void) {
    let callback = &*(ctx as *mut C);
    callback.callback();
}

The following APIs need to be updated:

  • CompactionFilter
  • TablePropertiesCollector
  • SliceTransform
  • EventListener
  • Logger
  • TableFilter
@yiwu-arbug yiwu-arbug added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. sig/engine SIG: Engine difficulty/easy Easy task. difficulty/medium Medium task. and removed difficulty/easy Easy task. labels Jun 21, 2020
tabokie pushed a commit that referenced this issue Sep 16, 2021
Close #513.

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>
@tabokie
Copy link
Member

tabokie commented Nov 4, 2021

Reopen this, #663 missed the following types:

  • CompationFilter, CompactionFilterFactory
  • SliceTransform

@tabokie tabokie reopened this Nov 4, 2021
JayiceZ added a commit to JayiceZ/rust-rocksdb that referenced this issue Dec 11, 2021
close tikv#513

Signed-off-by: Jayice <1185430411@qq.com>
JayiceZ added a commit to JayiceZ/rust-rocksdb that referenced this issue Dec 11, 2021
close tikv#513

Signed-off-by: Jayice <1185430411@qq.com>
JayiceZ added a commit to JayiceZ/rust-rocksdb that referenced this issue Dec 11, 2021
close tikv#513

Signed-off-by: Jayice <1185430411@qq.com>
tabokie pushed a commit that referenced this issue Dec 11, 2021
* reduce extra boxing
close #513

Signed-off-by: Jayice <1185430411@qq.com>

* rename generic
close #513

Signed-off-by: Jayice <1185430411@qq.com>

* format code
close #513

Signed-off-by: Jayice <1185430411@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty/medium Medium task. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. sig/engine SIG: Engine
Projects
None yet
2 participants