From a0c9ad45e20c44f31cc0efa20a53b3cca8d009b5 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Wed, 1 Jan 2025 13:28:56 -0500 Subject: [PATCH] Use an Arc for DependencyMap (#514) Using Rc saves some atomic operations, at the cost of the result being usable only from one thread. But lots of things are multi-threaded, including Python, which causes issues trying to [wrap this type](https://github.com/GothenburgBitFactory/taskchampion-py/issues/11). This isn't performance-critical code, so let's just us Arc. This is a breaking change to the return type of `Replica::dependency_map`. This also adds a `Clone` derive to the type, just in case someone wants the data without sharing. --- src/depmap.rs | 4 ++-- src/replica.rs | 8 ++++---- src/task/task.rs | 10 +++++----- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/depmap.rs b/src/depmap.rs index 4742d0851..19a48aa09 100644 --- a/src/depmap.rs +++ b/src/depmap.rs @@ -4,9 +4,9 @@ use uuid::Uuid; /// /// This information requires a scan of the working set to generate, so it is /// typically calculated once and re-used. -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq, Clone)] pub struct DependencyMap { - /// Edges of the dependency graph. If (a, b) is in this array, then task a depends on tsak b. + /// Edges of the dependency graph. If (a, b) is in this array, then task a depends on task b. edges: Vec<(Uuid, Uuid)>, } diff --git a/src/replica.rs b/src/replica.rs index 2742361d7..1aa7684db 100644 --- a/src/replica.rs +++ b/src/replica.rs @@ -11,7 +11,7 @@ use anyhow::Context; use chrono::{DateTime, Duration, Utc}; use log::trace; use std::collections::HashMap; -use std::rc::Rc; +use std::sync::Arc; use uuid::Uuid; /// A replica represents an instance of a user's task data, providing an easy interface @@ -37,7 +37,7 @@ pub struct Replica { added_undo_point: bool, /// The dependency map for this replica, if it has been calculated. - depmap: Option>, + depmap: Option>, } impl Replica { @@ -151,7 +151,7 @@ impl Replica { /// /// Calculating this value requires a scan of the full working set and may not be performant. /// The [`TaskData`] API avoids generating this value. - pub fn dependency_map(&mut self, force: bool) -> Result> { + pub fn dependency_map(&mut self, force: bool) -> Result> { if force || self.depmap.is_none() { // note: we can't use self.get_task here, as that depends on a // DependencyMap @@ -203,7 +203,7 @@ impl Replica { } } } - self.depmap = Some(Rc::new(dm)); + self.depmap = Some(Arc::new(dm)); } // at this point self.depmap is guaranteed to be Some(_) diff --git a/src/task/task.rs b/src/task/task.rs index 2d774d845..79c78e25b 100644 --- a/src/task/task.rs +++ b/src/task/task.rs @@ -8,8 +8,8 @@ use chrono::prelude::*; use log::trace; use std::convert::AsRef; use std::convert::TryInto; -use std::rc::Rc; use std::str::FromStr; +use std::sync::Arc; use uuid::Uuid; /// A task, with a high-level interface. @@ -28,7 +28,7 @@ pub struct Task { data: TaskData, // The dependency map for this replica, for rapidly computing synthetic tags. - depmap: Rc, + depmap: Arc, // True if an operation has alredy been emitted to update the `modified` property. updated_modified: bool, @@ -80,7 +80,7 @@ fn uda_tuple_to_string(namespace: impl AsRef, key: impl AsRef) -> Stri } impl Task { - pub(crate) fn new(data: TaskData, depmap: Rc) -> Task { + pub(crate) fn new(data: TaskData, depmap: Arc) -> Task { Task { data, depmap, @@ -538,8 +538,8 @@ mod test { use pretty_assertions::assert_eq; use std::collections::HashSet; - fn dm() -> Rc { - Rc::new(DependencyMap::new()) + fn dm() -> Arc { + Arc::new(DependencyMap::new()) } // Test task mutation by modifying a task and checking the assertions both on the