+BLOCKED, +UNBLOCKED, and +BLOCKING tags

These are somewhat expensive tags, as they require reference to values
outside of the task itself.

To accomplish this, the replica supplies a pre-computed DependencyMap
that is only calculated once per replica, and only from the working set.
This commit is contained in:
Dustin J. Mitchell 2022-02-22 02:33:22 +00:00
parent 47e213d6ec
commit 6f48f715ac
No known key found for this signature in database
8 changed files with 318 additions and 21 deletions

View file

@ -0,0 +1,81 @@
use uuid::Uuid;
/// DependencyMap stores information on task dependencies between pending tasks.
///
/// This information requires a scan of the working set to generate, so it is
/// typically calculated once and re-used.
#[derive(Debug, PartialEq)]
pub struct DependencyMap {
/// Edges of the dependency graph. If (a, b) is in this array, then task a depends on tsak b.
edges: Vec<(Uuid, Uuid)>,
}
impl DependencyMap {
/// Create a new, empty DependencyMap.
pub(super) fn new() -> Self {
Self { edges: Vec::new() }
}
/// Add a dependency of a on b.
pub(super) fn add_dependency(&mut self, a: Uuid, b: Uuid) {
self.edges.push((a, b));
}
/// Return an iterator of Uuids on which task `deps_of` depends. This is equivalent to
/// `task.get_dependencies()`.
pub fn dependencies(&self, dep_of: Uuid) -> impl Iterator<Item = Uuid> + '_ {
self.edges
.iter()
.filter_map(move |(a, b)| if a == &dep_of { Some(*b) } else { None })
}
/// Return an iterator of Uuids of tasks that depend on `dep_on`
/// `task.get_dependencies()`.
pub fn dependents(&self, dep_on: Uuid) -> impl Iterator<Item = Uuid> + '_ {
self.edges
.iter()
.filter_map(move |(a, b)| if b == &dep_on { Some(*a) } else { None })
}
}
#[cfg(test)]
mod test {
use super::*;
use pretty_assertions::assert_eq;
use std::collections::HashSet;
#[test]
fn dependencies() {
let t = Uuid::new_v4();
let uuid1 = Uuid::new_v4();
let uuid2 = Uuid::new_v4();
let mut dm = DependencyMap::new();
dm.add_dependency(t, uuid1);
dm.add_dependency(t, uuid2);
dm.add_dependency(Uuid::new_v4(), t);
dm.add_dependency(Uuid::new_v4(), uuid1);
dm.add_dependency(uuid2, Uuid::new_v4());
assert_eq!(
dm.dependencies(t).collect::<HashSet<_>>(),
set![uuid1, uuid2]
);
}
#[test]
fn dependents() {
let t = Uuid::new_v4();
let uuid1 = Uuid::new_v4();
let uuid2 = Uuid::new_v4();
let mut dm = DependencyMap::new();
dm.add_dependency(uuid1, t);
dm.add_dependency(uuid2, t);
dm.add_dependency(t, Uuid::new_v4());
dm.add_dependency(Uuid::new_v4(), uuid1);
dm.add_dependency(uuid2, Uuid::new_v4());
assert_eq!(dm.dependents(t).collect::<HashSet<_>>(), set![uuid1, uuid2]);
}
}

View file

@ -45,6 +45,10 @@ This crate supports Rust version 1.47 and higher.
*/
// NOTE: it's important that this 'mod' comes first so that the macros can be used in other modules
mod macros;
mod depmap;
mod errors;
mod replica;
pub mod server;
@ -54,6 +58,7 @@ mod taskdb;
mod utils;
mod workingset;
pub use depmap::DependencyMap;
pub use errors::Error;
pub use replica::Replica;
pub use server::{Server, ServerConfig};

View file

@ -0,0 +1,17 @@
#![macro_use]
/// Create a hashset, similar to vec!
// NOTE: in Rust 1.56.0, this can be changed to HashSet::from([..])
#[cfg(test)]
macro_rules! set(
{ $($key:expr),* $(,)? } => {
{
#[allow(unused_mut)]
let mut s = ::std::collections::HashSet::new();
$(
s.insert($key);
)*
s
}
};
);

View file

@ -1,3 +1,4 @@
use crate::depmap::DependencyMap;
use crate::server::{Server, SyncOp};
use crate::storage::{Storage, TaskMap};
use crate::task::{Status, Task};
@ -7,6 +8,7 @@ use anyhow::Context;
use chrono::{Duration, Utc};
use log::trace;
use std::collections::HashMap;
use std::rc::Rc;
use uuid::Uuid;
/// A replica represents an instance of a user's task data, providing an easy interface
@ -28,7 +30,12 @@ use uuid::Uuid;
/// during the garbage-collection process.
pub struct Replica {
taskdb: TaskDb,
/// If true, this replica has already added an undo point.
added_undo_point: bool,
/// The dependency map for this replica, if it has been calculated.
depmap: Option<Rc<DependencyMap>>,
}
impl Replica {
@ -36,6 +43,7 @@ impl Replica {
Replica {
taskdb: TaskDb::new(storage),
added_undo_point: false,
depmap: None,
}
}
@ -76,9 +84,10 @@ impl Replica {
/// Get all tasks represented as a map keyed by UUID
pub fn all_tasks(&mut self) -> anyhow::Result<HashMap<Uuid, Task>> {
let depmap = self.dependency_map(false)?;
let mut res = HashMap::new();
for (uuid, tm) in self.taskdb.all_tasks()?.drain(..) {
res.insert(uuid, Task::new(uuid, tm));
res.insert(uuid, Task::new(uuid, tm, depmap.clone()));
}
Ok(res)
}
@ -94,12 +103,47 @@ impl Replica {
Ok(WorkingSet::new(self.taskdb.working_set()?))
}
/// Get the dependency map for all pending tasks.
///
/// The data in this map is cached when it is first requested and may not contain modifications
/// made locally in this Replica instance. The result is reference-counted and may
/// outlive the Replica.
///
/// If `force` is true, then the result is re-calculated from the current state of the replica,
/// although previously-returned dependency maps are not updated.
pub fn dependency_map(&mut self, force: bool) -> anyhow::Result<Rc<DependencyMap>> {
if force || self.depmap.is_none() {
let mut dm = DependencyMap::new();
let ws = self.working_set()?;
for i in 1..=ws.largest_index() {
if let Some(u) = ws.by_index(i) {
// note: we can't use self.get_task here, as that depends on a
// DependencyMap
if let Some(taskmap) = self.taskdb.get_task(u)? {
for p in taskmap.keys() {
if let Some(dep_str) = p.strip_prefix("dep_") {
if let Ok(dep) = Uuid::parse_str(dep_str) {
dm.add_dependency(u, dep);
}
}
}
}
}
}
self.depmap = Some(Rc::new(dm));
}
// at this point self.depmap is guaranteed to be Some(_)
Ok(self.depmap.as_ref().unwrap().clone())
}
/// Get an existing task by its UUID
pub fn get_task(&mut self, uuid: Uuid) -> anyhow::Result<Option<Task>> {
let depmap = self.dependency_map(false)?;
Ok(self
.taskdb
.get_task(uuid)?
.map(move |tm| Task::new(uuid, tm)))
.map(move |tm| Task::new(uuid, tm, depmap)))
}
/// Create a new task.
@ -107,7 +151,8 @@ impl Replica {
let uuid = Uuid::new_v4();
self.add_undo_point(false)?;
let taskmap = self.taskdb.apply(SyncOp::Create { uuid })?;
let mut task = Task::new(uuid, taskmap).into_mut(self);
let depmap = self.dependency_map(false)?;
let mut task = Task::new(uuid, taskmap, depmap).into_mut(self);
task.set_description(description)?;
task.set_status(status)?;
task.set_entry(Some(Utc::now()))?;
@ -121,7 +166,8 @@ impl Replica {
pub fn import_task_with_uuid(&mut self, uuid: Uuid) -> anyhow::Result<Task> {
self.add_undo_point(false)?;
let taskmap = self.taskdb.apply(SyncOp::Create { uuid })?;
Ok(Task::new(uuid, taskmap))
let depmap = self.dependency_map(false)?;
Ok(Task::new(uuid, taskmap, depmap))
}
/// Delete a task. The task must exist. Note that this is different from setting status to
@ -217,6 +263,7 @@ mod tests {
use crate::task::Status;
use chrono::TimeZone;
use pretty_assertions::assert_eq;
use std::collections::HashSet;
use uuid::Uuid;
#[test]
@ -445,4 +492,67 @@ mod tests {
assert!(t.get_description().starts_with("keeper"));
}
}
#[test]
fn dependency_map() {
let mut rep = Replica::new_inmemory();
let mut tasks = vec![];
for _ in 0..4 {
tasks.push(rep.new_task(Status::Pending, "t".into()).unwrap());
}
let uuids: Vec<_> = tasks.iter().map(|t| t.get_uuid()).collect();
// t[3] depends on t[2], and t[1]
{
let mut t = tasks.pop().unwrap().into_mut(&mut rep);
t.add_dependency(uuids[2]).unwrap();
t.add_dependency(uuids[1]).unwrap();
}
// t[2] depends on t[0]
{
let mut t = tasks.pop().unwrap().into_mut(&mut rep);
t.add_dependency(uuids[0]).unwrap();
}
// t[1] depends on t[0]
{
let mut t = tasks.pop().unwrap().into_mut(&mut rep);
t.add_dependency(uuids[0]).unwrap();
}
// generate the dependency map, forcing an update based on the newly-added
// dependencies
let dm = rep.dependency_map(true).unwrap();
assert_eq!(
dm.dependencies(uuids[3]).collect::<HashSet<_>>(),
set![uuids[1], uuids[2]]
);
assert_eq!(
dm.dependencies(uuids[2]).collect::<HashSet<_>>(),
set![uuids[0]]
);
assert_eq!(
dm.dependencies(uuids[1]).collect::<HashSet<_>>(),
set![uuids[0]]
);
assert_eq!(dm.dependencies(uuids[0]).collect::<HashSet<_>>(), set![]);
assert_eq!(dm.dependents(uuids[3]).collect::<HashSet<_>>(), set![]);
assert_eq!(
dm.dependents(uuids[2]).collect::<HashSet<_>>(),
set![uuids[3]]
);
assert_eq!(
dm.dependents(uuids[1]).collect::<HashSet<_>>(),
set![uuids[3]]
);
assert_eq!(
dm.dependents(uuids[0]).collect::<HashSet<_>>(),
set![uuids[1], uuids[2]]
);
}
}

View file

@ -134,6 +134,9 @@ pub(super) enum SyntheticTag {
Pending,
Completed,
Deleted,
Blocked,
Unblocked,
Blocking,
}
#[cfg(test)]

View file

@ -1,11 +1,13 @@
use super::tag::{SyntheticTag, TagInner};
use super::{Annotation, Priority, Status, Tag, Timestamp};
use crate::depmap::DependencyMap;
use crate::replica::Replica;
use crate::storage::TaskMap;
use chrono::prelude::*;
use log::trace;
use std::convert::AsRef;
use std::convert::TryInto;
use std::rc::Rc;
use std::str::FromStr;
use uuid::Uuid;
@ -29,10 +31,18 @@ use uuid::Uuid;
/// This struct contains only getters for various values on the task. The
/// [`into_mut`](Task::into_mut) method
/// returns a TaskMut which can be used to modify the task.
#[derive(Debug, Clone, PartialEq)]
#[derive(Debug, Clone)]
pub struct Task {
uuid: Uuid,
taskmap: TaskMap,
depmap: Rc<DependencyMap>,
}
impl PartialEq for Task {
fn eq(&self, other: &Task) -> bool {
// compare only the taskmap and uuid; depmap is just present for reference
self.uuid == other.uuid && self.taskmap == other.taskmap
}
}
/// A mutable task, with setter methods.
@ -84,8 +94,12 @@ fn uda_tuple_to_string(namespace: impl AsRef<str>, key: impl AsRef<str>) -> Stri
}
impl Task {
pub(crate) fn new(uuid: Uuid, taskmap: TaskMap) -> Task {
Task { uuid, taskmap }
pub(crate) fn new(uuid: Uuid, taskmap: TaskMap, depmap: Rc<DependencyMap>) -> Task {
Task {
uuid,
taskmap,
depmap,
}
}
pub fn get_uuid(&self) -> Uuid {
@ -151,6 +165,16 @@ impl Task {
self.taskmap.contains_key(Prop::Start.as_ref())
}
/// Determine whether this task is blocked -- that is, has at least one unresolved dependency.
pub fn is_blocked(&self) -> bool {
self.depmap.dependencies(self.uuid).next().is_some()
}
/// Determine whether this task is blocking -- that is, has at least one unresolved dependent.
pub fn is_blocking(&self) -> bool {
self.depmap.dependents(self.uuid).next().is_some()
}
/// Determine whether a given synthetic tag is present on this task. All other
/// synthetic tag calculations are based on this one.
fn has_synthetic_tag(&self, synth: &SyntheticTag) -> bool {
@ -160,6 +184,9 @@ impl Task {
SyntheticTag::Pending => self.get_status() == Status::Pending,
SyntheticTag::Completed => self.get_status() == Status::Completed,
SyntheticTag::Deleted => self.get_status() == Status::Deleted,
SyntheticTag::Blocked => self.is_blocked(),
SyntheticTag::Unblocked => !self.is_blocked(),
SyntheticTag::Blocking => self.is_blocking(),
}
}
@ -520,6 +547,11 @@ impl<'r> std::ops::Deref for TaskMut<'r> {
mod test {
use super::*;
use pretty_assertions::assert_eq;
use std::collections::HashSet;
fn dm() -> Rc<DependencyMap> {
Rc::new(DependencyMap::new())
}
fn with_mut_task<F: FnOnce(TaskMut)>(f: F) {
let mut replica = Replica::new_inmemory();
@ -540,7 +572,7 @@ mod test {
#[test]
fn test_is_active_never_started() {
let task = Task::new(Uuid::new_v4(), TaskMap::new());
let task = Task::new(Uuid::new_v4(), TaskMap::new(), dm());
assert!(!task.is_active());
}
@ -551,6 +583,7 @@ mod test {
vec![(String::from("start"), String::from("1234"))]
.drain(..)
.collect(),
dm(),
);
assert!(task.is_active());
@ -558,13 +591,13 @@ mod test {
#[test]
fn test_is_active_inactive() {
let task = Task::new(Uuid::new_v4(), Default::default());
let task = Task::new(Uuid::new_v4(), Default::default(), dm());
assert!(!task.is_active());
}
#[test]
fn test_entry_not_set() {
let task = Task::new(Uuid::new_v4(), TaskMap::new());
let task = Task::new(Uuid::new_v4(), TaskMap::new(), dm());
assert_eq!(task.get_entry(), None);
}
@ -576,13 +609,14 @@ mod test {
vec![(String::from("entry"), format!("{}", ts.timestamp()))]
.drain(..)
.collect(),
dm(),
);
assert_eq!(task.get_entry(), Some(ts));
}
#[test]
fn test_wait_not_set() {
let task = Task::new(Uuid::new_v4(), TaskMap::new());
let task = Task::new(Uuid::new_v4(), TaskMap::new(), dm());
assert!(!task.is_waiting());
assert_eq!(task.get_wait(), None);
@ -596,6 +630,7 @@ mod test {
vec![(String::from("wait"), format!("{}", ts.timestamp()))]
.drain(..)
.collect(),
dm(),
);
assert!(!task.is_waiting());
@ -610,6 +645,7 @@ mod test {
vec![(String::from("wait"), format!("{}", ts.timestamp()))]
.drain(..)
.collect(),
dm(),
);
assert!(task.is_waiting());
@ -626,6 +662,7 @@ mod test {
]
.drain(..)
.collect(),
dm(),
);
assert!(task.has_tag(&utag("abc")));
@ -647,17 +684,17 @@ mod test {
]
.drain(..)
.collect(),
dm(),
);
let mut tags: Vec<_> = task.get_tags().collect();
tags.sort();
let mut exp = vec![
let tags: HashSet<_> = task.get_tags().collect();
let exp = set![
utag("abc"),
utag("def"),
stag(SyntheticTag::Pending),
stag(SyntheticTag::Waiting),
stag(SyntheticTag::Unblocked),
];
exp.sort();
assert_eq!(tags, exp);
}
@ -673,11 +710,19 @@ mod test {
]
.drain(..)
.collect(),
dm(),
);
// only "ok" is OK
let tags: Vec<_> = task.get_tags().collect();
assert_eq!(tags, vec![utag("ok"), stag(SyntheticTag::Pending)]);
let tags: HashSet<_> = task.get_tags().collect();
assert_eq!(
tags,
set![
utag("ok"),
stag(SyntheticTag::Pending),
stag(SyntheticTag::Unblocked)
]
);
}
#[test]
@ -698,6 +743,7 @@ mod test {
]
.drain(..)
.collect(),
dm(),
);
let mut anns: Vec<_> = task.get_annotations().collect();
@ -913,6 +959,7 @@ mod test {
]
.drain(..)
.collect(),
dm(),
);
let mut udas: Vec<_> = task.get_udas().collect();
@ -934,6 +981,7 @@ mod test {
]
.drain(..)
.collect(),
dm(),
);
assert_eq!(task.get_uda("", "description"), None); // invalid UDA
@ -954,6 +1002,7 @@ mod test {
]
.drain(..)
.collect(),
dm(),
);
assert_eq!(task.get_legacy_uda("description"), None); // invalid UDA
@ -1061,4 +1110,32 @@ mod test {
assert_eq!(task.get_dependencies().collect::<Vec<_>>(), vec![dep2]);
})
}
#[test]
fn dependencies_tags() {
let mut rep = Replica::new_inmemory();
let uuid1;
let uuid2;
{
let t1 = rep.new_task(Status::Pending, "1".into()).unwrap();
uuid1 = t1.get_uuid();
let t2 = rep.new_task(Status::Pending, "2".into()).unwrap();
uuid2 = t2.get_uuid();
let mut t1 = t1.into_mut(&mut rep);
t1.add_dependency(t2.get_uuid()).unwrap();
}
// force-refresh depmap
rep.dependency_map(true).unwrap();
let t1 = rep.get_task(uuid1).unwrap().unwrap();
let t2 = rep.get_task(uuid2).unwrap().unwrap();
assert!(t1.has_tag(&stag(SyntheticTag::Blocked)));
assert!(!t1.has_tag(&stag(SyntheticTag::Unblocked)));
assert!(!t1.has_tag(&stag(SyntheticTag::Blocking)));
assert!(!t2.has_tag(&stag(SyntheticTag::Blocked)));
assert!(t2.has_tag(&stag(SyntheticTag::Unblocked)));
assert!(t2.has_tag(&stag(SyntheticTag::Blocking)));
}
}