LRU statement cache

[gwenn]

No description provided.

[jgallagher]

I'm inclined to think this should return a Result<CachedStatement<'cache>>. CachedStatement would wrap up Statement (and implement Deref/DerefMut for it too, I think), and would return it to the cache in drop. This would remove the need to remember to call release. We could actually remove release altogether, I think, and add a release or discard method to CachedStatement that would (a) take self (to consume it) and (b) set a flag (or something similar) to avoid returning the statement to the cache in drop.

What do you think?

[gwenn]

Yes, introducing a CachedStatement is the right way.

[gwenn]

I am sorry to bother you but I cannot make the CachedStatement work: FIXME cannot borrow immutable borrowed content*self.cacheas mutable.
Should I introduce a RefCell?

[jgallagher]

Good news / bad news. Good news is you can get around the issues in drop by putting the statement inside an Option (which we'll guarantee is always Some() and is therefore safe to call .unwrap() on), so you can .take() the value out during destruction:

pub struct CachedStatement<'conn> {
    stmt: Option<Statement<'conn>>,
    cache: RefCell<StatementCache<'conn>>,
    pub cacheable : bool,
}

impl<'conn> Drop for CachedStatement<'conn> {
    #[allow(unused_must_use)]
    fn drop(&mut self) {
        if self.cacheable {
            self.cache.borrow_mut().release(self.stmt.take().unwrap(), false);
        } else {
            self.stmt.take().unwrap().finalize();
        }
    }
}

Bad news is it looks like lru-cache doesn't work on stable or beta rust. It depends on linked-hash-map which uses an unstable feature. I don't think we can merge this in and keep rusqlite usable on stable rust. :(

[gwenn]

Thanks.
This one works on stable but the Value must be clonable and it cannot be cleared...

[gwenn]

I will need your help: how to create a RefCell from a reference ?

src/cache.rs:78         stmt.map(|stmt| CachedStatement::new(stmt, RefCell::new(self)))

[jgallagher]

Mm, that's never going to work - if it did, you'd end up with multiple &mut StatementCaches in each statement, which should be impossible.

What if you put the internal cache itself inside a RefCell? Then get and release could take &self (they could borrow_mut() the underlying storage), and CachedStatement could hold a plain &'a StatementCache?

[gwenn]

The implementation is not perfect but it seems to be working...
I will try to make some benches to show the difference between cache and no cache.

[gwenn]

test bench_cache ... bench: 124 ns/iter (+/- 76)
test bench_no_cache ... bench: 5,538 ns/iter (+/- 4,178)

[jgallagher]

Merged as part of #113.