Skip to content

Commit

Permalink
Task Edges Set/List (#8624)
Browse files Browse the repository at this point in the history
### Description

* Task children and dependencies are stored together in a single
datastructure -> task edges
* task edges is a map keyed by target task id and using a bitfield for
common edges (child, output, cell 0)
* removed lazy_remove_children feature flag -> this is always on now
(Disabling it doesn't make sense as it negatively affects performance,
we had the feature flag in case it it is broken, but seems fine)
* When a task becomes Done, we use a Vec instead of a HashMap for better
memory efficiency. It doesn't need to modified after that.

### Testing Instructions

<!--
  Give a quick description of steps to test your changes.
-->

---------

Co-authored-by: Benjamin Woodruff <[email protected]>
  • Loading branch information
sokra and bgw authored Jul 2, 2024
1 parent 78c44bd commit 52ed46b
Show file tree
Hide file tree
Showing 11 changed files with 920 additions and 279 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

53 changes: 30 additions & 23 deletions crates/turbo-tasks-auto-hash-map/src/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,7 @@ impl<K: Eq + Hash, V, H: BuildHasher + Default, const I: usize> AutoMap<K, V, H,
}
None
}
AutoMap::Map(map) => {
let result = map.remove(key);
if result.is_some() && map.len() < MIN_HASH_SIZE {
self.convert_to_list();
}
result
}
AutoMap::Map(map) => map.remove(key),
}
}

Expand Down Expand Up @@ -213,6 +207,33 @@ impl<K: Eq + Hash, V, H: BuildHasher + Default, const I: usize> AutoMap<K, V, H,
}
}

/// see [HashMap::retain](https://doc.rust-lang.org/std/collections/struct.HashMap.html#method.retain)
pub fn retain<F>(&mut self, mut f: F)
where
F: FnMut(&K, &mut V) -> bool,
{
match self {
AutoMap::List(list) => {
// Don't use `Vec::retain`, as that uses a slower algorithm to maintain order,
// which we don't care about
let mut len = list.len();
let mut i = 0;
while i < len {
let (key, value) = &mut list[i];
if !f(key, value) {
list.swap_remove(i);
len -= 1;
} else {
i += 1;
}
}
}
AutoMap::Map(map) => {
map.retain(f);
}
}
}

/// see [HashMap::shrink_to_fit](https://doc.rust-lang.org/std/collections/struct.HashMap.html#method.shrink_to_fit)
pub fn shrink_to_fit(&mut self) {
match self {
Expand Down Expand Up @@ -554,14 +575,7 @@ impl<'a, K: Eq + Hash, V, H: BuildHasher + Default, const I: usize> OccupiedEntr
pub fn remove(self) -> V {
match self {
OccupiedEntry::List { list, index } => list.swap_remove(index).1,
OccupiedEntry::Map { entry, this } => {
let v = entry.remove();
let this = unsafe { &mut *this };
if this.len() < MIN_HASH_SIZE {
this.convert_to_list();
}
v
}
OccupiedEntry::Map { entry, this: _ } => entry.remove(),
}
}
}
Expand Down Expand Up @@ -636,14 +650,7 @@ impl<'a, K: Eq + Hash, V, H: BuildHasher + Default, const I: usize>
pub fn remove(self) -> V {
match self {
OccupiedRawEntry::List { list, index } => list.swap_remove(index).1,
OccupiedRawEntry::Map { entry, this } => {
let v = entry.remove();
let this = unsafe { &mut *this };
if this.len() < MIN_HASH_SIZE {
this.convert_to_list();
}
v
}
OccupiedRawEntry::Map { entry, this: _ } => entry.remove(),
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/turbo-tasks-memory/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ anyhow = { workspace = true }
auto-hash-map = { workspace = true }
concurrent-queue = { workspace = true }
dashmap = { workspace = true }
either = { workspace = true }
indexmap = { workspace = true }
num_cpus = "1.13.1"
once_cell = { workspace = true }
Expand Down Expand Up @@ -58,8 +59,7 @@ print_scope_updates = []
print_task_invalidation = []
inline_add_to_scope = []
inline_remove_from_scope = []
lazy_remove_children = []
default = ["lazy_remove_children"]
default = []

[[bench]]
name = "mod"
Expand Down
10 changes: 0 additions & 10 deletions crates/turbo-tasks-memory/src/cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,6 @@ pub struct RecomputingCell {
}

impl Cell {
/// Returns true if the cell has a value.
pub fn has_value(&self) -> bool {
match self {
Cell::Empty => false,
Cell::Recomputing { .. } => false,
Cell::TrackedValueless { .. } => false,
Cell::Value { .. } => true,
}
}

/// Removes a task from the list of dependent tasks.
pub fn remove_dependent_task(&mut self, task: TaskId) {
match self {
Expand Down
Loading

0 comments on commit 52ed46b

Please sign in to comment.