Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Performance improvements (3) #6045

Merged
merged 6 commits into from Sep 30, 2023
Merged

Performance improvements (3) #6045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0bbf803
make i and id more clear
sokra Sep 29, 2023
eeb6b6b
use TaskIdSet and StackVec typedefs
sokra Sep 29, 2023
ea14961
Merge branch 'sokra/perf3' into sokra/perf
sokra Sep 29, 2023
a64a788
improve locking in bottom tree
sokra Sep 28, 2023
446649a
use rw lock to make get_root_info cheaper
sokra Sep 28, 2023
5858201
Merge branch 'main' into sokra/perf
sokra Sep 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 28 additions & 0 deletions crates/turbo-tasks-memory/src/aggregation_tree/bottom_connection.rs
View file
Open in desktop
Expand Up @@ -303,4 +303,32 @@ impl<T, I: IsEnabled + Eq + Hash + Clone> BottomUppers<T, I> {
}
}
}

pub fn get_root_info<C: AggregationContext<Info = T, ItemRef = I>>(
&self,
aggregation_context: &C,
root_info_type: &C::RootInfoType,
mut result: C::RootInfo,
) -> C::RootInfo {
match &self {
BottomUppers::Left(upper) => {
let info = upper.get_root_info(aggregation_context, root_info_type);
if aggregation_context.merge_root_info(&mut result, info) == ControlFlow::Break(())
{
return result;
}
}
BottomUppers::Inner(list) => {
for (BottomRef { upper }, _) in list.iter() {
let info = upper.get_root_info(aggregation_context, root_info_type);
if aggregation_context.merge_root_info(&mut result, info)
== ControlFlow::Break(())
{
return result;
}
}
}
}
result
}
}
65 changes: 35 additions & 30 deletions crates/turbo-tasks-memory/src/aggregation_tree/bottom_tree.rs
View file
Open in desktop
@@ -1,7 +1,7 @@
use std::{hash::Hash, ops::ControlFlow, sync::Arc};

use nohash_hasher::{BuildNoHashHasher, IsEnabled};
use parking_lot::{Mutex, MutexGuard};
use parking_lot::{RwLock, RwLockWriteGuard};
use ref_cast::RefCast;

use super::{
Expand All @@ -22,7 +22,7 @@ use crate::count_hash_set::{CountHashSet, RemoveIfEntryResult};
pub struct BottomTree<T, I: IsEnabled> {
height: u8,
item: I,
state: Mutex<BottomTreeState<T, I>>,
state: RwLock<BottomTreeState<T, I>>,
}

pub struct BottomTreeState<T, I: IsEnabled> {
Expand All @@ -38,7 +38,7 @@ impl<T: Default, I: IsEnabled> BottomTree<T, I> {
Self {
height,
item,
state: Mutex::new(BottomTreeState {
state: RwLock::new(BottomTreeState {
data: T::default(),
bottom_upper: BottomConnection::new(),
top_upper: CountHashSet::new(),
Expand Down Expand Up @@ -82,7 +82,7 @@ impl<T, I: Clone + Eq + Hash + IsEnabled> BottomTree<T, I> {
}

fn add_children_of_child_if_following(&self, children: &mut StackVec<&I>) {
let mut state = self.state.lock();
let mut state = self.state.write();
children.retain(|&mut child| !state.following.add_if_entry(child));
}

Expand All @@ -91,7 +91,7 @@ impl<T, I: Clone + Eq + Hash + IsEnabled> BottomTree<T, I> {
aggregation_context: &C,
mut children: StackVec<&I>,
) {
let mut state = self.state.lock();
let mut state = self.state.write();
children.retain(|&mut child| state.following.add_clonable(child));
if children.is_empty() {
return;
Expand Down Expand Up @@ -175,7 +175,7 @@ impl<T, I: Clone + Eq + Hash + IsEnabled> BottomTree<T, I> {
}

fn add_child_of_child_if_following(&self, child_of_child: &I) -> bool {
let mut state = self.state.lock();
let mut state = self.state.write();
state.following.add_if_entry(child_of_child)
}

Expand All @@ -184,7 +184,7 @@ impl<T, I: Clone + Eq + Hash + IsEnabled> BottomTree<T, I> {
aggregation_context: &C,
child_of_child: &I,
) {
let mut state = self.state.lock();
let mut state = self.state.write();
if !state.following.add_clonable(child_of_child) {
// Already connect, nothing more to do
return;
Expand Down Expand Up @@ -237,7 +237,7 @@ impl<T, I: Clone + Eq + Hash + IsEnabled> BottomTree<T, I> {
aggregation_context: &C,
child_of_child: &I,
) -> bool {
let mut state = self.state.lock();
let mut state = self.state.write();
match state.following.remove_if_entry(child_of_child) {
RemoveIfEntryResult::PartiallyRemoved => return true,
RemoveIfEntryResult::NotPresent => return false,
Expand All @@ -252,7 +252,7 @@ impl<T, I: Clone + Eq + Hash + IsEnabled> BottomTree<T, I> {
aggregation_context: &C,
children: &mut Vec<&'a I>,
) {
let mut state = self.state.lock();
let mut state = self.state.write();
let mut removed = StackVec::default();
children.retain(|&child| match state.following.remove_if_entry(child) {
RemoveIfEntryResult::PartiallyRemoved => false,
Expand All @@ -272,7 +272,7 @@ impl<T, I: Clone + Eq + Hash + IsEnabled> BottomTree<T, I> {
aggregation_context: &C,
child_of_child: &I,
) -> bool {
let mut state = self.state.lock();
let mut state = self.state.write();
if !state.following.remove_clonable(child_of_child) {
// no present, nothing to do
return false;
Expand All @@ -286,7 +286,7 @@ impl<T, I: Clone + Eq + Hash + IsEnabled> BottomTree<T, I> {
aggregation_context: &C,
mut children: StackVec<&I>,
) {
let mut state = self.state.lock();
let mut state = self.state.write();
children.retain(|&mut child| state.following.remove_clonable(child));
propagate_lost_followings_to_uppers(state, aggregation_context, children);
}
Expand Down Expand Up @@ -338,7 +338,7 @@ impl<T, I: Clone + Eq + Hash + IsEnabled> BottomTree<T, I> {
aggregation_context: &C,
upper: &Arc<BottomTree<T, I>>,
) {
let mut state = self.state.lock();
let mut state = self.state.write();
let old_inner = state.bottom_upper.set_left_upper(upper);
let add_change = aggregation_context.info_to_add_change(&state.data);
let children = state.following.iter().cloned().collect::<StackVec<_>>();
Expand Down Expand Up @@ -397,7 +397,7 @@ impl<T, I: Clone + Eq + Hash + IsEnabled> BottomTree<T, I> {
remove_change: &Option<C::ItemChange>,
following: &[I],
) {
let mut state = self.state.lock();
let mut state = self.state.write();
if count > 0 {
// add as following
if state.following.add_count(item.clone(), count as usize) {
Expand Down Expand Up @@ -425,7 +425,7 @@ impl<T, I: Clone + Eq + Hash + IsEnabled> BottomTree<T, I> {
upper: &Arc<BottomTree<T, I>>,
nesting_level: u8,
) -> bool {
let mut state = self.state.lock();
let mut state = self.state.write();
let number_of_following = state.following.len();
let BottomConnection::Inner(inner) = &mut state.bottom_upper else {
return false;
Expand Down Expand Up @@ -457,7 +457,7 @@ impl<T, I: Clone + Eq + Hash + IsEnabled> BottomTree<T, I> {
aggregation_context: &C,
upper: &Arc<BottomTree<T, I>>,
) {
let mut state = self.state.lock();
let mut state = self.state.write();
state.bottom_upper.unset_left_upper(upper);
if let Some(change) = aggregation_context.info_to_remove_change(&state.data) {
upper.child_change(aggregation_context, &change);
Expand All @@ -478,7 +478,7 @@ impl<T, I: Clone + Eq + Hash + IsEnabled> BottomTree<T, I> {
aggregation_context: &C,
upper: &Arc<BottomTree<T, I>>,
) -> bool {
let mut state = self.state.lock();
let mut state = self.state.write();
let BottomConnection::Inner(inner) = &mut state.bottom_upper else {
return false;
};
Expand All @@ -500,7 +500,7 @@ impl<T, I: Clone + Eq + Hash + IsEnabled> BottomTree<T, I> {
aggregation_context: &C,
upper: &Arc<TopTree<T>>,
) {
let mut state = self.state.lock();
let mut state = self.state.write();
let new = state.top_upper.add_clonable(TopRef::ref_cast(upper));
if new {
if let Some(change) = aggregation_context.info_to_add_change(&state.data) {
Expand All @@ -518,7 +518,7 @@ impl<T, I: Clone + Eq + Hash + IsEnabled> BottomTree<T, I> {
aggregation_context: &C,
upper: &Arc<TopTree<T>>,
) {
let mut state = self.state.lock();
let mut state = self.state.write();
let removed = state.top_upper.remove_clonable(TopRef::ref_cast(upper));
if removed {
if let Some(change) = aggregation_context.info_to_remove_change(&state.data) {
Expand Down Expand Up @@ -553,7 +553,7 @@ impl<T, I: Clone + Eq + Hash + IsEnabled> BottomTree<T, I> {
aggregation_context: &C,
change: &C::ItemChange,
) {
let mut state = self.state.lock();
let mut state = self.state.write();
let change = aggregation_context.apply_change(&mut state.data, change);
propagate_change_to_upper(&mut state, aggregation_context, change);
}
Expand All @@ -564,21 +564,26 @@ impl<T, I: Clone + Eq + Hash + IsEnabled> BottomTree<T, I> {
root_info_type: &C::RootInfoType,
) -> C::RootInfo {
let mut result = aggregation_context.new_root_info(root_info_type);
let state = self.state.lock();
for TopRef { upper } in state.top_upper.iter() {
let top_uppers = {
let state = self.state.read();
state.top_upper.iter().cloned().collect::<StackVec<_>>()
};
for TopRef { upper } in top_uppers.iter() {
let info = upper.get_root_info(aggregation_context, root_info_type);
if aggregation_context.merge_root_info(&mut result, info) == ControlFlow::Break(()) {
return result;
}
}
state
.bottom_upper
.get_root_info(aggregation_context, root_info_type, result)
let bottom_uppers = {
let state = self.state.read();
state.bottom_upper.as_cloned_uppers()
};
bottom_uppers.get_root_info(aggregation_context, root_info_type, result)
}
}

fn propagate_lost_following_to_uppers<C: AggregationContext>(
state: MutexGuard<'_, BottomTreeState<C::Info, C::ItemRef>>,
state: RwLockWriteGuard<'_, BottomTreeState<C::Info, C::ItemRef>>,
aggregation_context: &C,
child_of_child: &C::ItemRef,
) {
Expand All @@ -592,7 +597,7 @@ fn propagate_lost_following_to_uppers<C: AggregationContext>(
}

fn propagate_lost_followings_to_uppers<'a, C: AggregationContext>(
state: MutexGuard<'_, BottomTreeState<C::Info, C::ItemRef>>,
state: RwLockWriteGuard<'_, BottomTreeState<C::Info, C::ItemRef>>,
aggregation_context: &C,
children: impl IntoIterator<Item = &'a C::ItemRef> + Clone,
) where
Expand All @@ -608,7 +613,7 @@ fn propagate_lost_followings_to_uppers<'a, C: AggregationContext>(
}

fn propagate_new_following_to_uppers<C: AggregationContext>(
state: MutexGuard<'_, BottomTreeState<C::Info, C::ItemRef>>,
state: RwLockWriteGuard<'_, BottomTreeState<C::Info, C::ItemRef>>,
aggregation_context: &C,
child_of_child: &C::ItemRef,
) {
Expand All @@ -622,7 +627,7 @@ fn propagate_new_following_to_uppers<C: AggregationContext>(
}

fn propagate_change_to_upper<C: AggregationContext>(
state: &mut MutexGuard<BottomTreeState<C::Info, C::ItemRef>>,
state: &mut RwLockWriteGuard<BottomTreeState<C::Info, C::ItemRef>>,
aggregation_context: &C,
change: Option<C::ItemChange>,
) {
Expand Down Expand Up @@ -651,7 +656,7 @@ fn visit_graph<C: AggregationContext>(
let mut edges = 0;
while let Some(item) = queue.pop_front() {
let tree = bottom_tree(aggregation_context, &item, height);
let state = tree.state.lock();
let state = tree.state.read();
for next in state.following.iter() {
edges += 1;
if visited.insert(next.clone()) {
Expand Down Expand Up @@ -692,7 +697,7 @@ pub fn print_graph<C: AggregationContext>(
let tree = bottom_tree(aggregation_context, &item, height);
let name = name_fn(&item);
let label = format!("{}", name);
let state = tree.state.lock();
let state = tree.state.read();
if color_upper {
print!(r#""{} {}" [color=red];"#, height - 1, name);
} else {
Expand Down