al8n / stretto

Stretto is a Rust implementation for Dgraph's ristretto (https://github.com/dgraph-io/ristretto). A high performance memory-bound Rust cache.
Apache License 2.0
412 stars 28 forks source link

Examples hint the unhandy usage of static variables, leaving ownership to others #48

Closed peter-scholtens closed 1 year ago

peter-scholtens commented 1 year ago

When trying to modify both examples as a novice exercise, I stumbled onto the fact that static string slices are used. This immediately goes wrong when I simply move the creation of the cache to a separate function, e.g. in the async case:

fn new_async_cache() -> AsyncCache<&str, &str> {
    let c : AsyncCache<&str, &str> = AsyncCache::new(12960, 1e6 as i64, tokio::spawn).unwrap();
    c
}

The compiler will now rightfully complains:

fn new_async_cache() -> AsyncCache<&str, &str> {
                                   ^     ^ expected named lifetime parameter
                                   |
                                   expected named lifetime parameter

Using references to string slices leaves the responsibility ownership to others: I would expect the cache takes up ownership, why create it otherwise? Do you accept a patch where I modify the structures to String types for both key and value?

al8n commented 1 year ago

Hi, in Rust, &str has no ownership, you should use something like AsyncCache<&'static str, &'static str> or AsyncCache<String, String>.

peter-scholtens commented 1 year ago

Indeed. If the original owner goes out of scope, the cache points to an empty spot, which the compiler indeed detects. Your suggestion to use static indeed solves the problem as far as the compiler concerns, but effective points to strings in the memory of the executable: I expect that is not the intention of making this software cache? That's why I said in the title that the examples hint to use static variables. It would be nicer to demonstrate use of heap stored variables, but don't feel obliged if you think the example becomes too complicated.

al8n commented 1 year ago

Indeed. If the original owner goes out of scope, the cache points to an empty spot, which the compiler indeed detects. Your suggestion to use static indeed solves the problem as far as the compiler concerns, but effective points to strings in the memory of the executable: I expect that is not the intention of making this software cache? That's why I said in the title that the examples hint to use static variables. It would be nicer to demonstrate use of heap stored variables, but don't feel obliged if you think the example becomes too complicated.

Yeah, you are right, it is not a good example. I am not aware of that because it was so long since I wrote the example. Thanks for pointing it out!

al8n commented 1 year ago

Do you accept a patch where I modify the structures to String types for both key and value?

I really appreciate it if you can help me modify it.