diff --git a/src/c_api/ext_fns/handle.rs b/src/c_api/ext_fns/handle.rs index 79888cb..d58c69d 100644 --- a/src/c_api/ext_fns/handle.rs +++ b/src/c_api/ext_fns/handle.rs @@ -133,6 +133,14 @@ pub extern "C" fn possum_single_read_at( Err(err) => return err.into(), Ok(ok) => ok, }; + // eprintln!( + // "reading single {} bytes at {} from {}, read {}: {}", + // read_buf.len(), + // offset, + // value.length(), + // r_nbyte, + // rust_key.escape_ascii(), + // ); buf.size = r_nbyte; NoError } diff --git a/src/handle.rs b/src/handle.rs index 5255eca..fd66859 100644 --- a/src/handle.rs +++ b/src/handle.rs @@ -257,7 +257,8 @@ impl Handle { /// Begins a read transaction. pub fn read(&self) -> rusqlite::Result> { let reader = Reader { - owned_tx: self.start_deferred_transaction()?, + owned_tx: self + .start_writable_transaction_with_behaviour(TransactionBehavior::Immediate)?, reads: Default::default(), }; Ok(reader) diff --git a/src/lib.rs b/src/lib.rs index 0b737bd..3733f09 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -480,10 +480,15 @@ impl Deref for Value { impl Value { fn from_row(row: &rusqlite::Row) -> rusqlite::Result { - let file_id: Option = row.get(0)?; - let file_offset: Option = row.get(1)?; - let length = row.get(2)?; - let last_used = row.get(3)?; + Self::from_column_values(row.get(0)?, row.get(1)?, row.get(2)?, row.get(3)?) + } + + fn from_column_values( + file_id: Option, + file_offset: Option, + length: ValueLength, + last_used: Timestamp, + ) -> rusqlite::Result { let location = if length == 0 { assert_eq!(file_id, None); assert_eq!(file_offset, None); diff --git a/src/tests.rs b/src/tests.rs index 61754d7..a71856a 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -3,6 +3,7 @@ use std::thread::*; use std::time::*; use anyhow::Result; +use rusqlite::TransactionState; use self::test; use super::*; @@ -150,3 +151,24 @@ fn test_torrent_storage_benchmark() -> anyhow::Result<()> { use testing::torrent_storage::*; BENCHMARK_OPTS.build()?.run() } + +/// Show that update moves a transaction to write, even if nothing is changed. This was an +/// investigation on how to optimize touch_for_read if last_used doesn't change. +#[test] +fn test_sqlite_update_same_value_txn_state() -> Result<()> { + let mut conn = rusqlite::Connection::open_in_memory()?; + conn.execute_batch( + r" + create table a(b); + --insert into a values (1); + ", + )?; + let tx = conn.transaction()?; + assert_eq!(tx.transaction_state(None)?, TransactionState::None); + let changed = tx.execute("update a set b=1", [])?; + // No rows were changed. + assert_eq!(changed, 0); + // But now we're a write transaction anyway. + assert_eq!(tx.transaction_state(None)?, TransactionState::Write); + Ok(()) +} diff --git a/src/tx.rs b/src/tx.rs index e9a86e9..98b27c7 100644 --- a/src/tx.rs +++ b/src/tx.rs @@ -185,15 +185,37 @@ impl ReadTransaction for T where T: ReadOnlyTransactionAccessor {} impl<'h, H> Transaction<'h, H> { pub fn touch_for_read(&mut self, key: &[u8]) -> rusqlite::Result { - self.tx - .prepare_cached(&format!( - "update keys \ - set last_used=cast(unixepoch('subsec')*1e3 as integer) \ - where key=? \ - returning {}", + // Avoid modifying the manifest. We had to take a write lock already to ensure our data + // isn't modified on us, but it still seems to be an improvement. (-67% on read times in + // fact). + let (file_id, file_offset, value_length, mut last_used, now) = self + .tx + .prepare_cached_readonly(&format!( + "select {}, cast(unixepoch('subsec')*1e3 as integer) \ + from keys where key=?", value_columns_sql() ))? - .query_row([key], Value::from_row) + .query_row([key], |row| row.try_into())?; + let update_last_used = last_used != now; + // eprintln!("updating last used: {}", update_last_used); + if update_last_used { + let (new_last_used,) = self + .tx + .prepare_cached( + r" + update keys + set last_used=cast(unixepoch('subsec')*1e3 as integer) + where key=? + returning last_used + ", + )? + .query_row([key], |row| row.try_into())?; + // This can in fact change between calls. Since we're updating now anyway, we don't + // really care. + //assert_eq!(new_last_used, now); + last_used = new_last_used; + } + Value::from_column_values(file_id, file_offset, value_length, last_used) } }