ARK-Builders / ark-android

Reusable components for the ARK project
https://www.ark-builders.dev
MIT License
0 stars 0 forks source link

JNI for `fs-storage` #90

Open kirillt opened 6 months ago

kirillt commented 6 months ago

After this issue is completed:

We'll need to:

twitu commented 4 months ago

I'm currently implementing the java bindings for fs-storage using the jni crate. Here are a few points and open question that will decide the design -

  1. Currently, the BaseStorage trait defines the below functions. All these functions will be exposed through java bindings.
pub trait BaseStorage<K, V>: AsRef<BTreeMap<K, V>> {
    /// Create or update an entry in the internal mapping.
    fn set(&mut self, id: K, value: V);

    /// Remove an entry from the internal mapping.
    fn remove(&mut self, id: &K) -> Result<()>;

    /// Check if the storage is up-to-date,
    /// i.e. that the internal mapping is consistent
    /// with the data in the filesystem.
    fn is_storage_updated(&self) -> Result<bool>;

    /// Scan and load the key-value mapping
    /// from pre-configured location in the filesystem.
    fn read_fs(&mut self) -> Result<BTreeMap<K, V>>;

    /// Persist the internal key-value mapping
    /// to pre-configured location in the filesystem.
    fn write_fs(&mut self) -> Result<()>;

    /// Remove all persisted data
    /// by pre-configured location in the file-system.
    fn erase(&self) -> Result<()>;
}
  1. FileStorage<K, V> is the concrete class that implements BaseStorage. However, it is generic over the type of key and value and java bindings cannot be generic. This means the java bindings must specialize the FileStorage based on some passed parameter. On the java side this means that the FileStorage class will need to be specialized based on the parameter i.e. a new class for FileStorageStringScore and FileStorageStringTags or a single FileStorage classes that internally typecasts values to the appropriate type.
impl<K, V> BaseStorage<K, V> for FileStorage<K, V>
where

For e.g.

pub extern "system" fn Java_FileStorage_create(
    env: &mut JNIEnv,
    _class: JClass,
    label: JString,
    path: JString,
    key_type: JString,
    value_type: JString,
) -> jlong {
    let label: String = env.get_string(&label).unwrap().into();
    let path: String = env.get_string(&path).unwrap().into();
    let key_type: String = env.get_string(&label).unwrap().into();
    let value_type: String = env.get_string(&path).unwrap().into();

    // specialize FileStorage based on function arguments
    if key_type == "string" && value_type == "score" {
        let file_storage: FileStorage<String, Score> = FileStorage::new(label, Path::new(&path));
        Box::into_raw(Box::new(file_storage)) as jlong
    }
}

The complexity can be reduced by removing the generic parameter K for Key because from the use cases I've seen keys are always strings. What are your thoughts on this?

kirillt commented 4 months ago

@twitu fixing the type parameter for keys to always be String should be fine. We might need more flexibility around value type, because cache storages like previews have huge values. However, I think we need to design previews layer differently, passing the images across the JNI border sounds like a bad idea.

Probably we can just start with binding Storage<String, String> only, for the prototype of our framework.