From 25de884720f5441bae3cff1491a9a57c3140a417 Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Sun, 2 Aug 2015 12:07:49 +0200 Subject: [PATCH 01/18] LRU statement cache --- Cargo.toml | 5 ++++ src/cache.rs | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 9 +++++++ 3 files changed, 89 insertions(+) create mode 100644 src/cache.rs diff --git a/Cargo.toml b/Cargo.toml index 5d05ed5..1cd05f0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,7 @@ name = "rusqlite" [features] load_extension = ["libsqlite3-sys/load_extension"] +cache = ["lru-cache"] [dependencies] time = "~0.1.0" @@ -26,3 +27,7 @@ tempdir = "~0.3.4" [dependencies.libsqlite3-sys] path = "libsqlite3-sys" version = "0.2.0" + +[dependencies.lru-cache] +version = "~0.0.4" +optional = true \ No newline at end of file diff --git a/src/cache.rs b/src/cache.rs new file mode 100644 index 0000000..2b3165b --- /dev/null +++ b/src/cache.rs @@ -0,0 +1,75 @@ +extern crate lru_cache; + +use {SqliteResult, SqliteConnection, SqliteStatement}; +use self::lru_cache::LruCache; + +/// Prepared statements are cached for faster execution. +/// FIXME limitation: the same SQL can be cached only once... +#[derive(Debug)] +pub struct StatementCache<'conn> { + pub conn: &'conn SqliteConnection, + cache: LruCache>, +} + +impl<'conn> StatementCache<'conn> { + pub fn new(conn: &'conn SqliteConnection, capacity: usize) -> StatementCache<'conn> { + StatementCache{ conn: conn, cache: LruCache::new(capacity) } + } + + /// Search the cache for a prepared-statement object that implements `sql`. + // If no such prepared-statement can be found, allocate and prepare a new one. + pub fn get(&mut self, sql: &str) -> SqliteResult> { + let stmt = self.cache.remove(sql); + match stmt { + Some(stmt) => Ok(stmt), + _ => self.conn.prepare(sql) + } + } + + /// If `discard` is true, then the statement is deleted immediately. + /// Otherwise it is added to the LRU list and may be returned + /// by a subsequent call to `get()`. + pub fn release(&mut self, stmt: SqliteStatement<'conn>, discard: bool) -> SqliteResult<()> { + if discard { + return stmt.finalize(); + } + // FIXME stmt.reset_if_needed(); + // clear bindings ??? + self.cache.insert(stmt.sql(), stmt).map_or(Ok(()), |stmt| stmt.finalize()) + } + + /// Flush the prepared statement cache + pub fn flush(&mut self) { + self.cache.clear(); + } + + /// Return (current, max) sizes. + pub fn size(&self) -> (usize, usize) { + (self.cache.len(), self.cache.capacity()) + } + + /// Set the maximum number of cached statements. + pub fn set_size(&mut self, capacity: usize) { + self.cache.set_capacity(capacity); + } +} + +#[cfg(test)] +mod test { + use SqliteConnection; + use super::StatementCache; + + #[test] + fn test_cache() { + let db = SqliteConnection::open_in_memory().unwrap(); + let mut cache = StatementCache::new(&db, 10); + let sql = "PRAGMA schema_version"; + let mut stmt = cache.get(sql).unwrap(); + //println!("NEW {:?}", stmt); + cache.release(stmt, false).unwrap(); + stmt = cache.get(sql).unwrap(); + //println!("CACHED {:?}", stmt); + cache.release(stmt, true).unwrap(); + cache.flush(); + } +} diff --git a/src/lib.rs b/src/lib.rs index f98780b..7949b3f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -79,6 +79,7 @@ pub use transaction::{SqliteTransactionBehavior, pub mod types; mod transaction; #[cfg(feature = "load_extension")] mod load_extension_guard; +#[cfg(feature = "cache")] pub mod cache; /// A typedef of the result returned by many methods. pub type SqliteResult = Result; @@ -710,6 +711,14 @@ impl<'conn> SqliteStatement<'conn> { } } + fn sql(&self) -> String { // TODO Maybe SQL should by kept as an SqliteStatement field ? + unsafe { + let c_slice = CStr::from_ptr(ffi::sqlite3_sql(self.stmt)).to_bytes(); + let utf8_str = str::from_utf8(c_slice); + utf8_str.unwrap().to_string() + } + } + fn finalize_(&mut self) -> SqliteResult<()> { let r = unsafe { ffi::sqlite3_finalize(self.stmt) }; self.stmt = ptr::null_mut(); From a470499dcb4b8399072a0e1eece514c705fb29cb Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Sat, 8 Aug 2015 16:33:08 +0200 Subject: [PATCH 02/18] Improve documentation --- src/cache.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/cache.rs b/src/cache.rs index 2b3165b..8724766 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -1,9 +1,11 @@ +//! Prepared statements cache for faster execution. extern crate lru_cache; use {SqliteResult, SqliteConnection, SqliteStatement}; use self::lru_cache::LruCache; -/// Prepared statements are cached for faster execution. +/// Prepared statements cache. +/// /// FIXME limitation: the same SQL can be cached only once... #[derive(Debug)] pub struct StatementCache<'conn> { @@ -12,6 +14,7 @@ pub struct StatementCache<'conn> { } impl<'conn> StatementCache<'conn> { + /// Create a statement cache. pub fn new(conn: &'conn SqliteConnection, capacity: usize) -> StatementCache<'conn> { StatementCache{ conn: conn, cache: LruCache::new(capacity) } } From 4d2d8b43e65c6a44e988a5373782cd53626cd740 Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Wed, 11 Nov 2015 14:45:25 +0100 Subject: [PATCH 03/18] Clean use statement. --- src/cache.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cache.rs b/src/cache.rs index 8724766..ae03c67 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -2,7 +2,7 @@ extern crate lru_cache; use {SqliteResult, SqliteConnection, SqliteStatement}; -use self::lru_cache::LruCache; +use lru_cache::LruCache; /// Prepared statements cache. /// From fa03bcd5647d017a21abd7cc5d4cf3b085c1dcff Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Sat, 5 Dec 2015 12:28:12 +0100 Subject: [PATCH 04/18] Fix use declaration. --- src/cache.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cache.rs b/src/cache.rs index ae03c67..8724766 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -2,7 +2,7 @@ extern crate lru_cache; use {SqliteResult, SqliteConnection, SqliteStatement}; -use lru_cache::LruCache; +use self::lru_cache::LruCache; /// Prepared statements cache. /// From 7ab79d6de60f3aacf22c21b35a245fa9f74a6b0f Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Sun, 6 Dec 2015 19:57:20 +0100 Subject: [PATCH 05/18] Add Failure documentation. --- src/cache.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/cache.rs b/src/cache.rs index 8724766..1b8141e 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -21,6 +21,10 @@ impl<'conn> StatementCache<'conn> { /// Search the cache for a prepared-statement object that implements `sql`. // If no such prepared-statement can be found, allocate and prepare a new one. + /// + /// # Failure + /// + /// Will return `Err` if no cached statement can be found and the underlying SQLite prepare call fails. pub fn get(&mut self, sql: &str) -> SqliteResult> { let stmt = self.cache.remove(sql); match stmt { @@ -32,11 +36,16 @@ impl<'conn> StatementCache<'conn> { /// If `discard` is true, then the statement is deleted immediately. /// Otherwise it is added to the LRU list and may be returned /// by a subsequent call to `get()`. + /// + /// # Failure + /// + /// Will return `Err` if `stmt` (or the already cached statement implementing the same SQL) statement is `discard`ed + /// and the underlying SQLite finalize call fails. pub fn release(&mut self, stmt: SqliteStatement<'conn>, discard: bool) -> SqliteResult<()> { if discard { return stmt.finalize(); } - // FIXME stmt.reset_if_needed(); + stmt.reset_if_needed(); // clear bindings ??? self.cache.insert(stmt.sql(), stmt).map_or(Ok(()), |stmt| stmt.finalize()) } From 880a78ae83293d68b7a245fcf02ca4083c7a3022 Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Tue, 15 Dec 2015 21:49:59 +0100 Subject: [PATCH 06/18] Partial fix following John suggestions. --- src/cache.rs | 34 +++++++++++++++++++++++++++------- src/lib.rs | 10 +++++++++- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index d0eeede..4f7087c 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -49,22 +49,27 @@ impl<'conn> StatementCache<'conn> { return stmt.finalize(); } stmt.reset_if_needed(); - // clear bindings ??? + stmt.clear_bindings(); self.cache.insert(stmt.sql(), stmt).map_or(Ok(()), |stmt| stmt.finalize()) } /// Flush the prepared statement cache - pub fn flush(&mut self) { + pub fn clear(&mut self) { self.cache.clear(); } - /// Return (current, max) sizes. - pub fn size(&self) -> (usize, usize) { - (self.cache.len(), self.cache.capacity()) + /// Return current cache size. + pub fn len(&self) -> usize { + self.cache.len() + } + + /// Return maximum cache size. + pub fn capacity(&self) -> usize { + self.cache.capacity() } /// Set the maximum number of cached statements. - pub fn set_size(&mut self, capacity: usize) { + pub fn set_capacity(&mut self, capacity: usize) { self.cache.set_capacity(capacity); } } @@ -78,13 +83,28 @@ mod test { fn test_cache() { let db = Connection::open_in_memory().unwrap(); let mut cache = StatementCache::new(&db, 10); + assert_eq!(0, cache.len()); + assert_eq!(10, cache.capacity()); + let sql = "PRAGMA schema_version"; let mut stmt = cache.get(sql).unwrap(); + assert_eq!(0, cache.len()); + assert_eq!(0, stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + // println!("NEW {:?}", stmt); cache.release(stmt, false).unwrap(); + assert_eq!(1, cache.len()); + stmt = cache.get(sql).unwrap(); + assert_eq!(0, cache.len()); + assert_eq!(0, stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + // println!("CACHED {:?}", stmt); cache.release(stmt, true).unwrap(); - cache.flush(); + assert_eq!(0, cache.len()); + + cache.clear(); + assert_eq!(0, cache.len()); + assert_eq!(10, cache.capacity()); } } diff --git a/src/lib.rs b/src/lib.rs index 81ff23c..2ea017d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -929,7 +929,15 @@ impl<'conn> Statement<'conn> { } } - fn sql(&self) -> String { // TODO Maybe SQL should by kept as an SqliteStatement field ? + #[cfg(feature = "cache")] + fn clear_bindings(&mut self) { + unsafe { + ffi::sqlite3_clear_bindings(self.stmt); + }; + } + + #[cfg(feature = "cache")] + fn sql(&self) -> String { unsafe { let c_slice = CStr::from_ptr(ffi::sqlite3_sql(self.stmt)).to_bytes(); let utf8_str = str::from_utf8(c_slice); From 9257987b375bb07a9673bf8fba664d54ae0f3ab8 Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Wed, 16 Dec 2015 19:42:03 +0100 Subject: [PATCH 07/18] Try to introduce a CachedStatement struct. --- src/cache.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/cache.rs b/src/cache.rs index 4f7087c..38a5e38 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -9,10 +9,28 @@ use self::lru_cache::LruCache; /// FIXME limitation: the same SQL can be cached only once... #[derive(Debug)] pub struct StatementCache<'conn> { - pub conn: &'conn Connection, + conn: &'conn Connection, cache: LruCache>, } +pub struct CachedStatement<'conn> { + stmt: Statement<'conn>, + cache: &'conn StatementCache<'conn>, + pub cacheable : bool, +} + +impl<'conn> Drop for CachedStatement<'conn> { + #[allow(unused_must_use)] + fn drop(&mut self) { + if self.cacheable { + // FIXME cannot borrow immutable borrowed content `*self.cache` as mutable + //self.cache.release(self.stmt, false); + } else { + self.stmt.finalize_(); + } + } +} + impl<'conn> StatementCache<'conn> { /// Create a statement cache. pub fn new(conn: &'conn Connection, capacity: usize) -> StatementCache<'conn> { From ff02213b531d5346b1ee9c1a252f266a1849db4f Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Wed, 16 Dec 2015 20:10:31 +0100 Subject: [PATCH 08/18] Introduce a RefCell in CachedStatement. --- src/cache.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 38a5e38..aa78e5a 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -1,6 +1,7 @@ //! Prepared statements cache for faster execution. extern crate lru_cache; +use std::cell::RefCell; use {Result, Connection, Statement}; use self::lru_cache::LruCache; @@ -15,7 +16,7 @@ pub struct StatementCache<'conn> { pub struct CachedStatement<'conn> { stmt: Statement<'conn>, - cache: &'conn StatementCache<'conn>, + cache: RefCell>, pub cacheable : bool, } @@ -23,8 +24,8 @@ impl<'conn> Drop for CachedStatement<'conn> { #[allow(unused_must_use)] fn drop(&mut self) { if self.cacheable { - // FIXME cannot borrow immutable borrowed content `*self.cache` as mutable - //self.cache.release(self.stmt, false); + // FIXME: cannot move out of type `cache::CachedStatement<'conn>`, which defines the `Drop` trait [E0509] + //self.cache.borrow_mut().release(self.stmt, false); } else { self.stmt.finalize_(); } From 109c26fea4100122d6819842a6069ca6993ebe99 Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Thu, 17 Dec 2015 20:02:49 +0100 Subject: [PATCH 09/18] Replace LruCache by VecDeque. --- Cargo.toml | 6 +----- src/cache.rs | 54 ++++++++++++++++++++++------------------------------ src/lib.rs | 5 ++--- 3 files changed, 26 insertions(+), 39 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d077e66..ea70aea 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,7 +16,7 @@ name = "rusqlite" load_extension = ["libsqlite3-sys/load_extension"] backup = [] blob = [] -cache = ["lru-cache"] +cache = [] functions = [] trace = [] @@ -34,10 +34,6 @@ regex = "~0.1.41" path = "libsqlite3-sys" version = "0.3.0" -[dependencies.lru-cache] -version = "~0.0.4" -optional = true - [[test]] name = "config_log" harness = false diff --git a/src/cache.rs b/src/cache.rs index aa78e5a..e51ef2c 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -1,33 +1,29 @@ //! Prepared statements cache for faster execution. -extern crate lru_cache; use std::cell::RefCell; +use std::collections::VecDeque; use {Result, Connection, Statement}; -use self::lru_cache::LruCache; /// Prepared statements cache. -/// -/// FIXME limitation: the same SQL can be cached only once... #[derive(Debug)] pub struct StatementCache<'conn> { conn: &'conn Connection, - cache: LruCache>, + cache: VecDeque>, // back = LRU } pub struct CachedStatement<'conn> { - stmt: Statement<'conn>, + stmt: Option>, cache: RefCell>, - pub cacheable : bool, + pub cacheable: bool, } impl<'conn> Drop for CachedStatement<'conn> { #[allow(unused_must_use)] fn drop(&mut self) { if self.cacheable { - // FIXME: cannot move out of type `cache::CachedStatement<'conn>`, which defines the `Drop` trait [E0509] - //self.cache.borrow_mut().release(self.stmt, false); + self.cache.borrow_mut().release(self.stmt.take().unwrap()); } else { - self.stmt.finalize_(); + self.stmt.take().unwrap().finalize(); } } } @@ -37,7 +33,7 @@ impl<'conn> StatementCache<'conn> { pub fn new(conn: &'conn Connection, capacity: usize) -> StatementCache<'conn> { StatementCache { conn: conn, - cache: LruCache::new(capacity), + cache: VecDeque::with_capacity(capacity), } } @@ -48,9 +44,8 @@ impl<'conn> StatementCache<'conn> { /// /// Will return `Err` if no cached statement can be found and the underlying SQLite prepare call fails. pub fn get(&mut self, sql: &str) -> Result> { - let stmt = self.cache.remove(sql); - match stmt { - Some(stmt) => Ok(stmt), + match self.cache.iter().rposition(|entry| entry.eq(sql)) { + Some(index) => Ok(self.cache.swap_remove_front(index).unwrap()), // FIXME Not LRU compliant _ => self.conn.prepare(sql), } } @@ -63,13 +58,13 @@ impl<'conn> StatementCache<'conn> { /// /// Will return `Err` if `stmt` (or the already cached statement implementing the same SQL) statement is `discard`ed /// and the underlying SQLite finalize call fails. - pub fn release(&mut self, mut stmt: Statement<'conn>, discard: bool) -> Result<()> { - if discard { - return stmt.finalize(); + fn release(&mut self, mut stmt: Statement<'conn>) { + if self.cache.capacity() == self.cache.len() { // is full + self.cache.pop_back(); // LRU dropped } stmt.reset_if_needed(); stmt.clear_bindings(); - self.cache.insert(stmt.sql(), stmt).map_or(Ok(()), |stmt| stmt.finalize()) + self.cache.push_front(stmt) } /// Flush the prepared statement cache @@ -86,11 +81,6 @@ impl<'conn> StatementCache<'conn> { pub fn capacity(&self) -> usize { self.cache.capacity() } - - /// Set the maximum number of cached statements. - pub fn set_capacity(&mut self, capacity: usize) { - self.cache.set_capacity(capacity); - } } #[cfg(test)] @@ -101,29 +91,31 @@ mod test { #[test] fn test_cache() { let db = Connection::open_in_memory().unwrap(); - let mut cache = StatementCache::new(&db, 10); + let mut cache = StatementCache::new(&db, 15); assert_eq!(0, cache.len()); - assert_eq!(10, cache.capacity()); + assert_eq!(15, cache.capacity()); let sql = "PRAGMA schema_version"; let mut stmt = cache.get(sql).unwrap(); assert_eq!(0, cache.len()); - assert_eq!(0, stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + assert_eq!(0, + stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); // println!("NEW {:?}", stmt); - cache.release(stmt, false).unwrap(); + cache.release(stmt); assert_eq!(1, cache.len()); stmt = cache.get(sql).unwrap(); assert_eq!(0, cache.len()); - assert_eq!(0, stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + assert_eq!(0, + stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); // println!("CACHED {:?}", stmt); - cache.release(stmt, true).unwrap(); - assert_eq!(0, cache.len()); + cache.release(stmt); + assert_eq!(1, cache.len()); cache.clear(); assert_eq!(0, cache.len()); - assert_eq!(10, cache.capacity()); + assert_eq!(15, cache.capacity()); } } diff --git a/src/lib.rs b/src/lib.rs index 2ea017d..2382815 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -937,11 +937,10 @@ impl<'conn> Statement<'conn> { } #[cfg(feature = "cache")] - fn sql(&self) -> String { + fn eq(&self, sql: &str) -> bool { unsafe { let c_slice = CStr::from_ptr(ffi::sqlite3_sql(self.stmt)).to_bytes(); - let utf8_str = str::from_utf8(c_slice); - utf8_str.unwrap().to_string() + str::from_utf8(c_slice).unwrap().eq(sql) } } From 85fb89b28079ba5bc52462f8ecdb10b47ecbd50f Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Thu, 17 Dec 2015 20:33:34 +0100 Subject: [PATCH 10/18] Fail to create a new CachedStatement. --- src/cache.rs | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index e51ef2c..bee947a 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -2,6 +2,7 @@ use std::cell::RefCell; use std::collections::VecDeque; +use std::ops::{Deref, DerefMut}; use {Result, Connection, Statement}; /// Prepared statements cache. @@ -17,6 +18,20 @@ pub struct CachedStatement<'conn> { pub cacheable: bool, } +impl<'conn> Deref for CachedStatement<'conn> { + type Target = Statement<'conn>; + + fn deref(&self) -> &Statement<'conn> { + self.stmt.as_ref().unwrap() + } +} + +impl<'conn> DerefMut for CachedStatement<'conn> { + fn deref_mut(&mut self) -> &mut Statement<'conn> { + self.stmt.as_mut().unwrap() + } +} + impl<'conn> Drop for CachedStatement<'conn> { #[allow(unused_must_use)] fn drop(&mut self) { @@ -28,6 +43,18 @@ impl<'conn> Drop for CachedStatement<'conn> { } } +impl<'conn> CachedStatement<'conn> { + fn new(stmt: Statement<'conn>, + cache: RefCell>) + -> CachedStatement<'conn> { + CachedStatement { + stmt: Some(stmt), + cache: cache, + cacheable: true, + } + } +} + impl<'conn> StatementCache<'conn> { /// Create a statement cache. pub fn new(conn: &'conn Connection, capacity: usize) -> StatementCache<'conn> { @@ -43,11 +70,12 @@ impl<'conn> StatementCache<'conn> { /// # Failure /// /// Will return `Err` if no cached statement can be found and the underlying SQLite prepare call fails. - pub fn get(&mut self, sql: &str) -> Result> { - match self.cache.iter().rposition(|entry| entry.eq(sql)) { + pub fn get(&mut self, sql: &str) -> Result> { + let stmt = match self.cache.iter().rposition(|entry| entry.eq(sql)) { Some(index) => Ok(self.cache.swap_remove_front(index).unwrap()), // FIXME Not LRU compliant _ => self.conn.prepare(sql), - } + }; + stmt.map(|stmt| CachedStatement::new(stmt, RefCell::new(self))) } /// If `discard` is true, then the statement is deleted immediately. @@ -59,7 +87,8 @@ impl<'conn> StatementCache<'conn> { /// Will return `Err` if `stmt` (or the already cached statement implementing the same SQL) statement is `discard`ed /// and the underlying SQLite finalize call fails. fn release(&mut self, mut stmt: Statement<'conn>) { - if self.cache.capacity() == self.cache.len() { // is full + if self.cache.capacity() == self.cache.len() { + // is full self.cache.pop_back(); // LRU dropped } stmt.reset_if_needed(); From 30c8910d1920e822e0790325852faee4d0faec53 Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Fri, 18 Dec 2015 20:18:46 +0100 Subject: [PATCH 11/18] Still some lifetime problem... --- src/cache.rs | 62 ++++++++++++++++++++++++---------------------------- 1 file changed, 29 insertions(+), 33 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index bee947a..f2e050e 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -9,12 +9,12 @@ use {Result, Connection, Statement}; #[derive(Debug)] pub struct StatementCache<'conn> { conn: &'conn Connection, - cache: VecDeque>, // back = LRU + cache: RefCell>>, // back = LRU } pub struct CachedStatement<'conn> { stmt: Option>, - cache: RefCell>, + cache: &'conn StatementCache<'conn>, pub cacheable: bool, } @@ -36,7 +36,7 @@ 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()); + self.cache.release(self.stmt.take().unwrap()); } else { self.stmt.take().unwrap().finalize(); } @@ -44,9 +44,7 @@ impl<'conn> Drop for CachedStatement<'conn> { } impl<'conn> CachedStatement<'conn> { - fn new(stmt: Statement<'conn>, - cache: RefCell>) - -> CachedStatement<'conn> { + fn new(stmt: Statement<'conn>, cache: &'conn StatementCache<'conn>) -> CachedStatement<'conn> { CachedStatement { stmt: Some(stmt), cache: cache, @@ -60,7 +58,7 @@ impl<'conn> StatementCache<'conn> { pub fn new(conn: &'conn Connection, capacity: usize) -> StatementCache<'conn> { StatementCache { conn: conn, - cache: VecDeque::with_capacity(capacity), + cache: RefCell::new(VecDeque::with_capacity(capacity)), } } @@ -70,12 +68,12 @@ impl<'conn> StatementCache<'conn> { /// # Failure /// /// Will return `Err` if no cached statement can be found and the underlying SQLite prepare call fails. - pub fn get(&mut self, sql: &str) -> Result> { - let stmt = match self.cache.iter().rposition(|entry| entry.eq(sql)) { - Some(index) => Ok(self.cache.swap_remove_front(index).unwrap()), // FIXME Not LRU compliant + pub fn get(&'conn self, sql: &str) -> Result> { + let stmt = match self.cache.borrow().iter().rposition(|entry| entry.eq(sql)) { + Some(index) => Ok(self.cache.borrow_mut().swap_remove_front(index).unwrap()), // FIXME Not LRU compliant _ => self.conn.prepare(sql), }; - stmt.map(|stmt| CachedStatement::new(stmt, RefCell::new(self))) + stmt.map(|stmt| CachedStatement::new(stmt, self)) } /// If `discard` is true, then the statement is deleted immediately. @@ -86,29 +84,29 @@ impl<'conn> StatementCache<'conn> { /// /// Will return `Err` if `stmt` (or the already cached statement implementing the same SQL) statement is `discard`ed /// and the underlying SQLite finalize call fails. - fn release(&mut self, mut stmt: Statement<'conn>) { - if self.cache.capacity() == self.cache.len() { + fn release(&self, mut stmt: Statement<'conn>) { + if self.cache.borrow().capacity() == self.cache.borrow().len() { // is full - self.cache.pop_back(); // LRU dropped + self.cache.borrow_mut().pop_back(); // LRU dropped } stmt.reset_if_needed(); stmt.clear_bindings(); - self.cache.push_front(stmt) + self.cache.borrow_mut().push_front(stmt) } /// Flush the prepared statement cache - pub fn clear(&mut self) { - self.cache.clear(); + pub fn clear(&self) { + self.cache.borrow_mut().clear(); } /// Return current cache size. pub fn len(&self) -> usize { - self.cache.len() + self.cache.borrow().len() } /// Return maximum cache size. pub fn capacity(&self) -> usize { - self.cache.capacity() + self.cache.borrow().capacity() } } @@ -125,22 +123,20 @@ mod test { assert_eq!(15, cache.capacity()); let sql = "PRAGMA schema_version"; - let mut stmt = cache.get(sql).unwrap(); - assert_eq!(0, cache.len()); - assert_eq!(0, - stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); - - // println!("NEW {:?}", stmt); - cache.release(stmt); + { + let mut stmt = cache.get(sql).unwrap(); + assert_eq!(0, cache.len()); + assert_eq!(0, + stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + } assert_eq!(1, cache.len()); - stmt = cache.get(sql).unwrap(); - assert_eq!(0, cache.len()); - assert_eq!(0, - stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); - - // println!("CACHED {:?}", stmt); - cache.release(stmt); + { + let mut stmt = cache.get(sql).unwrap(); + assert_eq!(0, cache.len()); + assert_eq!(0, + stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + } assert_eq!(1, cache.len()); cache.clear(); From 8d84e2b0764215734ca39c5b9146bf85c10212e8 Mon Sep 17 00:00:00 2001 From: Gwenael Treguier Date: Fri, 18 Dec 2015 20:21:41 +0100 Subject: [PATCH 12/18] Activate cache feature in travis. --- .travis.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 2bd2336..197fc9b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,8 +12,9 @@ script: - cargo test --features load_extension - cargo test --features trace - cargo test --features functions - - cargo test --features "backup functions load_extension trace" - - cargo doc --no-deps --features "backup functions load_extension trace" + - cargo test --features cache + - cargo test --features "backup functions load_extension trace cache" + - cargo doc --no-deps --features "backup functions load_extension trace cache" after_success: | [ $TRAVIS_BRANCH = master ] && From 7bdf80ccdb6c22907753441d6ef9420987c04e83 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 19 Dec 2015 16:49:11 +0100 Subject: [PATCH 13/18] Introduce two distinct lifetime parameters. --- src/cache.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index f2e050e..e2b1c67 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -12,27 +12,27 @@ pub struct StatementCache<'conn> { cache: RefCell>>, // back = LRU } -pub struct CachedStatement<'conn> { - stmt: Option>, - cache: &'conn StatementCache<'conn>, +pub struct CachedStatement<'c: 's, 's> { + stmt: Option>, + cache: &'s StatementCache<'c>, pub cacheable: bool, } -impl<'conn> Deref for CachedStatement<'conn> { - type Target = Statement<'conn>; +impl<'c, 's> Deref for CachedStatement<'c, 's> { + type Target = Statement<'c>; - fn deref(&self) -> &Statement<'conn> { + fn deref(&self) -> &Statement<'c> { self.stmt.as_ref().unwrap() } } -impl<'conn> DerefMut for CachedStatement<'conn> { - fn deref_mut(&mut self) -> &mut Statement<'conn> { +impl<'c, 's> DerefMut for CachedStatement<'c, 's> { + fn deref_mut(&mut self) -> &mut Statement<'c> { self.stmt.as_mut().unwrap() } } -impl<'conn> Drop for CachedStatement<'conn> { +impl<'c, 's> Drop for CachedStatement<'c, 's> { #[allow(unused_must_use)] fn drop(&mut self) { if self.cacheable { @@ -43,8 +43,8 @@ impl<'conn> Drop for CachedStatement<'conn> { } } -impl<'conn> CachedStatement<'conn> { - fn new(stmt: Statement<'conn>, cache: &'conn StatementCache<'conn>) -> CachedStatement<'conn> { +impl<'c, 's> CachedStatement<'c, 's> { + fn new(stmt: Statement<'c>, cache: &'s StatementCache<'c>) -> CachedStatement<'c, 's> { CachedStatement { stmt: Some(stmt), cache: cache, @@ -68,7 +68,7 @@ impl<'conn> StatementCache<'conn> { /// # Failure /// /// Will return `Err` if no cached statement can be found and the underlying SQLite prepare call fails. - pub fn get(&'conn self, sql: &str) -> Result> { + pub fn get<'s>(&'s self, sql: &str) -> Result> { let stmt = match self.cache.borrow().iter().rposition(|entry| entry.eq(sql)) { Some(index) => Ok(self.cache.borrow_mut().swap_remove_front(index).unwrap()), // FIXME Not LRU compliant _ => self.conn.prepare(sql), From cb1951c21a25adc660e3c9a02ee2e7a17c9e8fb5 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 19 Dec 2015 16:56:41 +0100 Subject: [PATCH 14/18] Fix borrowing --- src/cache.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index e2b1c67..f7599ca 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -69,8 +69,9 @@ impl<'conn> StatementCache<'conn> { /// /// Will return `Err` if no cached statement can be found and the underlying SQLite prepare call fails. pub fn get<'s>(&'s self, sql: &str) -> Result> { - let stmt = match self.cache.borrow().iter().rposition(|entry| entry.eq(sql)) { - Some(index) => Ok(self.cache.borrow_mut().swap_remove_front(index).unwrap()), // FIXME Not LRU compliant + let mut cache = self.cache.borrow_mut(); + let stmt = match cache.iter().rposition(|entry| entry.eq(sql)) { + Some(index) => Ok(cache.swap_remove_front(index).unwrap()), // FIXME Not LRU compliant _ => self.conn.prepare(sql), }; stmt.map(|stmt| CachedStatement::new(stmt, self)) @@ -85,13 +86,14 @@ impl<'conn> StatementCache<'conn> { /// Will return `Err` if `stmt` (or the already cached statement implementing the same SQL) statement is `discard`ed /// and the underlying SQLite finalize call fails. fn release(&self, mut stmt: Statement<'conn>) { - if self.cache.borrow().capacity() == self.cache.borrow().len() { + let mut cache = self.cache.borrow_mut(); + if cache.capacity() == cache.len() { // is full - self.cache.borrow_mut().pop_back(); // LRU dropped + cache.pop_back(); // LRU dropped } stmt.reset_if_needed(); stmt.clear_bindings(); - self.cache.borrow_mut().push_front(stmt) + cache.push_front(stmt) } /// Flush the prepared statement cache @@ -118,7 +120,7 @@ mod test { #[test] fn test_cache() { let db = Connection::open_in_memory().unwrap(); - let mut cache = StatementCache::new(&db, 15); + let cache = StatementCache::new(&db, 15); assert_eq!(0, cache.len()); assert_eq!(15, cache.capacity()); From 5876be3d480694f0faf7e1e4d94c3c79a9e9384c Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 19 Dec 2015 17:01:06 +0100 Subject: [PATCH 15/18] Add test with cacheable set to false --- src/cache.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/cache.rs b/src/cache.rs index f7599ca..1173901 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -145,4 +145,20 @@ mod test { assert_eq!(0, cache.len()); assert_eq!(15, cache.capacity()); } + + #[test] + fn test_cacheable() { + let db = Connection::open_in_memory().unwrap(); + let cache = StatementCache::new(&db, 15); + + let sql = "PRAGMA schema_version"; + { + let mut stmt = cache.get(sql).unwrap(); + assert_eq!(0, cache.len()); + assert_eq!(0, + stmt.query(&[]).unwrap().get_expected_row().unwrap().get::(0)); + stmt.cacheable = false; + } + assert_eq!(0, cache.len()); + } } From 68b4943a39d36d207ad86ada8fd4f6c4c05ad04e Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 19 Dec 2015 17:14:06 +0100 Subject: [PATCH 16/18] Add some doc --- src/cache.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/cache.rs b/src/cache.rs index 1173901..a0cce77 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -5,13 +5,17 @@ use std::collections::VecDeque; use std::ops::{Deref, DerefMut}; use {Result, Connection, Statement}; -/// Prepared statements cache. +/// Prepared statements LRU cache. #[derive(Debug)] pub struct StatementCache<'conn> { conn: &'conn Connection, cache: RefCell>>, // back = LRU } +/// Cacheable statement. +/// +/// Statement will return automatically to the cache by default. +/// If you want the statement to be discarded, you can set the `cacheable` flag to `false`. pub struct CachedStatement<'c: 's, 's> { stmt: Option>, cache: &'s StatementCache<'c>, From 1ec2dee5332f6acd40820141daa709f77ff0fe6e Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 19 Dec 2015 17:22:54 +0100 Subject: [PATCH 17/18] Ensure features documentation is generated. --- .travis.yml | 2 +- publish-ghp-docs.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index e769717..50e7585 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,4 +13,4 @@ script: - cargo test --features trace - cargo test --features functions - cargo test --features cache - - cargo test --features "backup functions load_extension trace cache" + - cargo test --features "backup cache functions load_extension trace" diff --git a/publish-ghp-docs.sh b/publish-ghp-docs.sh index f9eeb17..de31a13 100755 --- a/publish-ghp-docs.sh +++ b/publish-ghp-docs.sh @@ -8,7 +8,7 @@ fi cd $(git rev-parse --show-toplevel) rm -rf target/doc/ -multirust run nightly cargo doc --no-deps --features "load_extension trace" +multirust run nightly cargo doc --no-deps --features "backup cache functions load_extension trace" echo '' > target/doc/index.html ghp-import target/doc git push origin gh-pages:gh-pages From 9b4fdc29ee7550ebe2a498f36f6a197853274261 Mon Sep 17 00:00:00 2001 From: gwenn Date: Sun, 20 Dec 2015 09:46:05 +0100 Subject: [PATCH 18/18] Add benchmarks for statement cache. --- benches/lib.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 benches/lib.rs diff --git a/benches/lib.rs b/benches/lib.rs new file mode 100644 index 0000000..92fddef --- /dev/null +++ b/benches/lib.rs @@ -0,0 +1,23 @@ +#![feature(test)] +extern crate test; + +extern crate rusqlite; + +use rusqlite::Connection; +use rusqlite::cache::StatementCache; +use test::Bencher; + +#[bench] +fn bench_no_cache(b: &mut Bencher) { + let db = Connection::open_in_memory().unwrap(); + let sql = "SELECT 1, 'test', 3.14 UNION SELECT 2, 'exp', 2.71"; + b.iter(|| db.prepare(sql).unwrap()); +} + +#[bench] +fn bench_cache(b: &mut Bencher) { + let db = Connection::open_in_memory().unwrap(); + let cache = StatementCache::new(&db, 15); + let sql = "SELECT 1, 'test', 3.14 UNION SELECT 2, 'exp', 2.71"; + b.iter(|| cache.get(sql).unwrap()); +}