Skip to content

Commit

Permalink
Use an Arc for DependencyMap (#514)
Browse files Browse the repository at this point in the history
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](GothenburgBitFactory/taskchampion-py#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.
  • Loading branch information
djmitche authored Jan 1, 2025
1 parent 74b04e0 commit a0c9ad4
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 11 deletions.
4 changes: 2 additions & 2 deletions src/depmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)>,
}

Expand Down
8 changes: 4 additions & 4 deletions src/replica.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -37,7 +37,7 @@ pub struct Replica {
added_undo_point: bool,

/// The dependency map for this replica, if it has been calculated.
depmap: Option<Rc<DependencyMap>>,
depmap: Option<Arc<DependencyMap>>,
}

impl Replica {
Expand Down Expand Up @@ -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<Rc<DependencyMap>> {
pub fn dependency_map(&mut self, force: bool) -> Result<Arc<DependencyMap>> {
if force || self.depmap.is_none() {
// note: we can't use self.get_task here, as that depends on a
// DependencyMap
Expand Down Expand Up @@ -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(_)
Expand Down
10 changes: 5 additions & 5 deletions src/task/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -28,7 +28,7 @@ pub struct Task {
data: TaskData,

// The dependency map for this replica, for rapidly computing synthetic tags.
depmap: Rc<DependencyMap>,
depmap: Arc<DependencyMap>,

// True if an operation has alredy been emitted to update the `modified` property.
updated_modified: bool,
Expand Down Expand Up @@ -80,7 +80,7 @@ fn uda_tuple_to_string(namespace: impl AsRef<str>, key: impl AsRef<str>) -> Stri
}

impl Task {
pub(crate) fn new(data: TaskData, depmap: Rc<DependencyMap>) -> Task {
pub(crate) fn new(data: TaskData, depmap: Arc<DependencyMap>) -> Task {
Task {
data,
depmap,
Expand Down Expand Up @@ -538,8 +538,8 @@ mod test {
use pretty_assertions::assert_eq;
use std::collections::HashSet;

fn dm() -> Rc<DependencyMap> {
Rc::new(DependencyMap::new())
fn dm() -> Arc<DependencyMap> {
Arc::new(DependencyMap::new())
}

// Test task mutation by modifying a task and checking the assertions both on the
Expand Down

0 comments on commit a0c9ad4

Please sign in to comment.