-
Notifications
You must be signed in to change notification settings - Fork 92
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
macro: Not working inside impl blocks #16
Comments
Unfortunately this won't work. |
Would it make sense to have a pair of macros, one for the cache-name/lazy_static, and another one only the fn item, taking the name of the cache as input, which can sit inside impl blocks? If course, the cache key type needs to again explicit here. I think that's a fairly small cost to pay. |
Yes, that's definitely an option! The existing macros are in need of a re-think anyway |
As a workaround, the following does the job for now: #[macro_use] extern crate cached;
use std::time::{Instant, Duration};
use std::thread::sleep;
use cached::SizedCache;
struct Foo;
impl Foo {
fn slow_fn(&self, n: u32) -> String {
cached! {
SLOW_FN: SizedCache<(u32), String> = SizedCache::with_size(50);
fn inner(n: u32) -> String = {
if n == 0 { return "done".to_string(); }
sleep(Duration::new(1, 0));
inner(n-1)
}
}
inner(n)
}
}
pub fn main() {
println!("Initial run...");
let now = Instant::now();
let foo = Foo;
let _ = foo.slow_fn(10);
println!("Elapsed: {}\n", now.elapsed().as_secs());
println!("Cached run...");
let now = Instant::now();
let _ = foo.slow_fn(10);
println!("Elapsed: {}\n", now.elapsed().as_secs());
// Inspecting the cache is not possible this way
} |
Nice one, I still have problems with using self references (or any var outside of cached block - "can't capture dynamic environment in a fn item") in the cached function, I understand the problem behind that, you can change the state and cache will be giving wrong values, but in my case I am ok with that, I just need a cache. Sad cannot use it, need to build something on my own. Any suggestions? |
Hmm, you could give I think the cached macros won't do the job here. But you can implement it yourself! Example: extern crate cached;
use std::time::{Instant, Duration};
use std::thread::sleep;
use cached::SizedCache;
pub struct Foo {
bar: u32,
}
impl Foo {
pub fn new(bar: u32) -> Foo {
Foo {
bar,
}
}
pub fn slow_fn(&self, n: u32) -> String {
// The cache is static. Please don't get scared from this monster ;)
static SLOW_FN_CACHE: cached::once_cell::sync::Lazy<::std::sync::Mutex<SizedCache<(u32), String>>> =
cached::once_cell::sync::Lazy {
__cell: cached::once_cell::sync::OnceCell::INIT,
__init: || std::sync::Mutex::new(SizedCache::with_size(50)),
};
// We need to clone the key so we can later on store it in the cache.
let key = n.clone();
// This block locks the cache and checks if we have a cache-hit
{
let mut cache = SLOW_FN_CACHE.lock().unwrap();
let res = cached::Cached::cache_get(&mut *cache, &key);
if let Some(res) = res {
// Return early in that case
return res.clone();
}
}
// If we have missed the cache, compute the value.
// This is where your code goes!
let val = {
// You can access `self` here!
if n == self.bar {
return "done".to_string();
}
sleep(Duration::new(1, 0));
// Even call the cached function recursively!
self.slow_fn(n - 1)
};
// Write the value to the cache so we get a hit next time
let mut cache = SLOW_FN_CACHE.lock().unwrap();
cached::Cached::cache_set(&mut *cache, key, val.clone());
val
}
}
pub fn main() {
println!("Initial run...");
let now = Instant::now();
let foo = Foo::new(42);
let _ = foo.slow_fn(52);
println!("Elapsed: {}\n", now.elapsed().as_secs());
println!("Cached run...");
let now = Instant::now();
let _ = foo.slow_fn(52);
println!("Elapsed: {}\n", now.elapsed().as_secs());
// Inspecting the cache is not possible this way
} This is essentially what the I don't know if this is a good solution, but at least it would work, I guess ... |
Thanks, will try this approach |
@Rahix , @kirhgoff - Yeah, the cached_key! {
LENGTH: SizedCache<String, usize> = SizedCache::with_size(50);
Key = { format!("{}{}", a, b) };
fn length(a: &str, b: &str, c: &str, d: &str) -> usize = {
let size = a.len() + b.len();
println!("ignoring: {:?}", &[c, d]);
sleep(Duration::new(size as u64, 0));
size
}
}
pub fn main() {
println!("Initial run...");
length("1", "123", "not", "cached");
println!("again ...");
length("1", "123", "still not", "cached");
} So instead of capturing the environment, you could pass what you need as a reference and exclude it from the cache-key. |
This would be such a good feature to have. |
Hey, I know it's been a hell of a long time, but is there any news on this or any way that I can help implement this? I've been digging through the code getting a preliminary understanding, so I can make a PR in some time if there's not another simpler way to do this already. @jaemk |
Your example code really impressed me, thank you! |
A year ago, we have removed So now we can implement this feature. The way I'm doing it is backward compatible. Just for If you agree, show me the 👍🏼 and if not, write a comment about your use case and why it's not suitable. Or if you have a better suggestion or better naming. For example, this code: struct Test;
impl Test {
#[cached(size = 50, in_impl = true)]
pub fn slow_fn(n: u32) -> String {
if n == 0 {
return "done".to_string();
}
sleep(Duration::new(1, 0));
Self::slow_fn(n - 1)
}
} Generates this: struct Test;
impl Test {
///Cached static for the [`slow_fn`] function.
pub fn slow_fn_get_cache_ident() -> &'static ::cached::once_cell::sync::Lazy<
std::sync::Mutex<cached::SizedCache<(u32), String>>,
> {
static SLOW_FN: ::cached::once_cell::sync::Lazy<
std::sync::Mutex<cached::SizedCache<(u32), String>>,
> = ::cached::once_cell::sync::Lazy::new(|| std::sync::Mutex::new(
cached::SizedCache::with_size(50usize),
));
&SLOW_FN
}
///Origin of the cached function [`slow_fn`].
///This is a cached function that uses the [`SLOW_FN`] cached static.
pub fn slow_fn_no_cache(n: u32) -> String {
if n == 0 {
return "done".to_string();
}
sleep(Duration::new(1, 0));
Self::slow_fn(n - 1)
}
///This is a cached function that uses the [`SLOW_FN`] cached static.
pub fn slow_fn(n: u32) -> String {
use cached::Cached;
use cached::CloneCached;
let key = (n.clone());
{
let mut cache = Self::slow_fn_get_cache_ident().lock().unwrap();
if let Some(result) = cache.cache_get(&key) {
return result.to_owned();
}
}
let result = Self::slow_fn_no_cache(n);
let mut cache = Self::slow_fn_get_cache_ident().lock().unwrap();
cache.cache_set(key, result.clone());
result
}
///Primes the cached function [`slow_fn`].
#[allow(dead_code)]
///This is a cached function that uses the [`SLOW_FN`] cached static.
pub fn slow_fn_prime_cache(n: u32) -> String {
use cached::Cached;
let key = (n.clone());
let mut cache = Self::slow_fn_get_cache_ident().lock().unwrap();
let result = Self::slow_fn_no_cache(n);
cache.cache_set(key, result.clone());
result
}
} |
I'd be happy to get access to this! I suppose this could also be extended to support self arguments? |
Thanks @omid ! The only thing missing is providing a way to access the cache since you can't grab the static directly when it's defined in the cache function |
@jaemk isn't calling |
Yes, you're right, looks good! I'll take a closer look and merge later today |
The
cached!
macro doesn't work inside struct'simpl
block. (Haven't tried other impl blocks.)Repro:
Error:
The text was updated successfully, but these errors were encountered: