From 614a0a45a694de92824ee0c72228717e2f99baa8 Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Mon, 11 Aug 2025 18:23:41 -0400 Subject: [PATCH 1/2] Relax and fix memory ordering --- cli/app.rs | 8 ++++---- core/storage/pager.rs | 35 ++++++++++++++++++----------------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/cli/app.rs b/cli/app.rs index 7b980ffed..7ce212f0a 100644 --- a/cli/app.rs +++ b/cli/app.rs @@ -160,7 +160,7 @@ impl Limbo { let interrupt_count: Arc = Arc::clone(&interrupt_count); ctrlc::set_handler(move || { // Increment the interrupt count on Ctrl-C - interrupt_count.fetch_add(1, Ordering::SeqCst); + interrupt_count.fetch_add(1, Ordering::Release); }) .expect("Error setting Ctrl-C handler"); } @@ -546,7 +546,7 @@ impl Limbo { fn reset_line(&mut self, _line: &str) -> rustyline::Result<()> { // Entry is auto added to history // self.rl.add_history_entry(line.to_owned())?; - self.interrupt_count.store(0, Ordering::SeqCst); + self.interrupt_count.store(0, Ordering::Release); Ok(()) } @@ -741,7 +741,7 @@ impl Limbo { OutputMode::List => { let mut headers_printed = false; loop { - if self.interrupt_count.load(Ordering::SeqCst) > 0 { + if self.interrupt_count.load(Ordering::Acquire) > 0 { println!("Query interrupted."); return Ok(()); } @@ -815,7 +815,7 @@ impl Limbo { } } OutputMode::Pretty => { - if self.interrupt_count.load(Ordering::SeqCst) > 0 { + if self.interrupt_count.load(Ordering::Acquire) > 0 { println!("Query interrupted."); return Ok(()); } diff --git a/core/storage/pager.rs b/core/storage/pager.rs index 8398bee58..e72caeb61 100644 --- a/core/storage/pager.rs +++ b/core/storage/pager.rs @@ -156,54 +156,55 @@ impl Page { } pub fn is_locked(&self) -> bool { - self.get().flags.load(Ordering::SeqCst) & PAGE_LOCKED != 0 + self.get().flags.load(Ordering::Acquire) & PAGE_LOCKED != 0 } - pub fn set_locked(&self) { - self.get().flags.fetch_or(PAGE_LOCKED, Ordering::SeqCst); + pub fn set_locked(&self) -> bool { + let prev = self.get().flags.fetch_or(PAGE_LOCKED, Ordering::Acquire); + prev & PAGE_LOCKED == 0 } pub fn clear_locked(&self) { - self.get().flags.fetch_and(!PAGE_LOCKED, Ordering::SeqCst); + self.get().flags.fetch_and(!PAGE_LOCKED, Ordering::Release); } pub fn is_error(&self) -> bool { - self.get().flags.load(Ordering::SeqCst) & PAGE_ERROR != 0 + self.get().flags.load(Ordering::Relaxed) & PAGE_ERROR != 0 } pub fn set_error(&self) { - self.get().flags.fetch_or(PAGE_ERROR, Ordering::SeqCst); + self.get().flags.fetch_or(PAGE_ERROR, Ordering::Release); } pub fn clear_error(&self) { - self.get().flags.fetch_and(!PAGE_ERROR, Ordering::SeqCst); + self.get().flags.fetch_and(!PAGE_ERROR, Ordering::Release); } pub fn is_dirty(&self) -> bool { - self.get().flags.load(Ordering::SeqCst) & PAGE_DIRTY != 0 + self.get().flags.load(Ordering::Acquire) & PAGE_DIRTY != 0 } pub fn set_dirty(&self) { tracing::debug!("set_dirty(page={})", self.get().id); - self.get().flags.fetch_or(PAGE_DIRTY, Ordering::SeqCst); + self.get().flags.fetch_or(PAGE_DIRTY, Ordering::Release); } pub fn clear_dirty(&self) { tracing::debug!("clear_dirty(page={})", self.get().id); - self.get().flags.fetch_and(!PAGE_DIRTY, Ordering::SeqCst); + self.get().flags.fetch_and(!PAGE_DIRTY, Ordering::Release); } pub fn is_loaded(&self) -> bool { - self.get().flags.load(Ordering::SeqCst) & PAGE_LOADED != 0 + self.get().flags.load(Ordering::Acquire) & PAGE_LOADED != 0 } pub fn set_loaded(&self) { - self.get().flags.fetch_or(PAGE_LOADED, Ordering::SeqCst); + self.get().flags.fetch_or(PAGE_LOADED, Ordering::Release); } pub fn clear_loaded(&self) { tracing::debug!("clear loaded {}", self.get().id); - self.get().flags.fetch_and(!PAGE_LOADED, Ordering::SeqCst); + self.get().flags.fetch_and(!PAGE_LOADED, Ordering::Release); } pub fn is_index(&self) -> bool { @@ -234,7 +235,7 @@ impl Page { pub fn try_unpin(&self) -> bool { self.get() .pin_count - .fetch_update(Ordering::SeqCst, Ordering::SeqCst, |current| { + .fetch_update(Ordering::Release, Ordering::Relaxed, |current| { if current == 0 { None } else { @@ -245,7 +246,7 @@ impl Page { } pub fn is_pinned(&self) -> bool { - self.get().pin_count.load(Ordering::SeqCst) > 0 + self.get().pin_count.load(Ordering::Acquire) > 0 } } @@ -336,12 +337,12 @@ impl AtomicDbState { #[inline] pub fn set(&self, state: DbState) { - self.0.store(state as usize, Ordering::SeqCst); + self.0.store(state as usize, Ordering::Release); } #[inline] pub fn get(&self) -> DbState { - let v = self.0.load(Ordering::SeqCst); + let v = self.0.load(Ordering::Acquire); match v { DbState::UNINITIALIZED => DbState::Uninitialized, DbState::INITIALIZING => DbState::Initializing, From f1475bd5acdc0bcc8e13058b9188cc2ced2d907b Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Wed, 13 Aug 2025 10:17:33 -0400 Subject: [PATCH 2/2] Remove bool return value from page set_locked --- core/storage/pager.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/core/storage/pager.rs b/core/storage/pager.rs index e72caeb61..7bd24b53c 100644 --- a/core/storage/pager.rs +++ b/core/storage/pager.rs @@ -159,9 +159,8 @@ impl Page { self.get().flags.load(Ordering::Acquire) & PAGE_LOCKED != 0 } - pub fn set_locked(&self) -> bool { - let prev = self.get().flags.fetch_or(PAGE_LOCKED, Ordering::Acquire); - prev & PAGE_LOCKED == 0 + pub fn set_locked(&self) { + self.get().flags.fetch_or(PAGE_LOCKED, Ordering::Acquire); } pub fn clear_locked(&self) { @@ -1011,13 +1010,13 @@ impl Pager { ) -> Result<(PageRef, Completion)> { tracing::trace!("read_page_no_cache(page_idx = {})", page_idx); let page = Arc::new(Page::new(page_idx)); - page.set_locked(); let Some(wal) = self.wal.as_ref() else { turso_assert!( matches!(frame_watermark, Some(0) | None), "frame_watermark must be either None or Some(0) because DB has no WAL and read with other watermark is invalid" ); + page.set_locked(); let c = self.begin_read_disk_page(page_idx, page.clone(), allow_empty_read)?; return Ok((page, c)); };