From 1b942e53ed5cb8476dcff6cee617b45fe7e49a22 Mon Sep 17 00:00:00 2001 From: Benjamin Woodruff Date: Thu, 29 Aug 2024 15:28:28 -0700 Subject: [PATCH 1/2] refactor(turbo-tasks): Move OutputContent from memory backend into turbo-tasks, represent Empty as None --- .../crates/turbo-tasks-memory/src/output.rs | 54 +++++-------------- .../crates/turbo-tasks-memory/src/task.rs | 4 +- turbopack/crates/turbo-tasks/src/lib.rs | 2 + turbopack/crates/turbo-tasks/src/output.rs | 40 ++++++++++++++ 4 files changed, 58 insertions(+), 42 deletions(-) create mode 100644 turbopack/crates/turbo-tasks/src/output.rs diff --git a/turbopack/crates/turbo-tasks-memory/src/output.rs b/turbopack/crates/turbo-tasks-memory/src/output.rs index 78d198b45fa24..a53b7397b155b 100644 --- a/turbopack/crates/turbo-tasks-memory/src/output.rs +++ b/turbopack/crates/turbo-tasks-memory/src/output.rs @@ -1,41 +1,18 @@ -use std::{ - borrow::Cow, - fmt::{Debug, Display}, - mem::take, -}; +use std::{borrow::Cow, fmt::Debug, mem::take}; use anyhow::{anyhow, Error, Result}; -use turbo_tasks::{util::SharedError, RawVc, TaskId, TaskIdSet, TurboTasksBackendApi}; +use turbo_tasks::{ + util::SharedError, OutputContent, RawVc, TaskId, TaskIdSet, TurboTasksBackendApi, +}; use crate::MemoryBackend; #[derive(Default, Debug)] pub struct Output { - pub(crate) content: OutputContent, + pub(crate) content: Option, pub(crate) dependent_tasks: TaskIdSet, } -#[derive(Clone, Debug, Default)] -pub enum OutputContent { - #[default] - Empty, - Link(RawVc), - Error(SharedError), - Panic(Option>>), -} - -impl Display for OutputContent { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - OutputContent::Empty => write!(f, "empty"), - OutputContent::Link(raw_vc) => write!(f, "link {:?}", raw_vc), - OutputContent::Error(err) => write!(f, "error {}", err), - OutputContent::Panic(Some(message)) => write!(f, "panic {}", message), - OutputContent::Panic(None) => write!(f, "panic"), - } - } -} - impl Output { pub fn read(&mut self, reader: TaskId) -> Result { self.dependent_tasks.insert(reader); @@ -44,25 +21,22 @@ impl Output { /// INVALIDATION: Be careful with this, it will not track dependencies, so /// using it could break cache invalidation. - pub fn read_untracked(&mut self) -> Result { + pub fn read_untracked(&self) -> Result { match &self.content { - OutputContent::Empty => Err(anyhow!("Output is empty")), - OutputContent::Error(err) => Err(anyhow::Error::new(err.clone())), - OutputContent::Link(raw_vc) => Ok(*raw_vc), - OutputContent::Panic(Some(message)) => Err(anyhow!("A task panicked: {message}")), - OutputContent::Panic(None) => Err(anyhow!("A task panicked")), + None => Err(anyhow!("Output is empty")), + Some(content) => content.read_untracked(), } } pub fn link(&mut self, target: RawVc, turbo_tasks: &dyn TurboTasksBackendApi) { debug_assert!(*self != target); - if let OutputContent::Link(old_target) = &self.content { + if let Some(OutputContent::Link(old_target)) = &self.content { if *old_target == target { // unchanged return; } } - self.content = OutputContent::Link(target); + self.content = Some(OutputContent::Link(target)); // notify if !self.dependent_tasks.is_empty() { turbo_tasks.schedule_notify_tasks_set(&take(&mut self.dependent_tasks)); @@ -70,7 +44,7 @@ impl Output { } pub fn error(&mut self, error: Error, turbo_tasks: &dyn TurboTasksBackendApi) { - self.content = OutputContent::Error(SharedError::new(error)); + self.content = Some(OutputContent::Error(SharedError::new(error))); // notify if !self.dependent_tasks.is_empty() { turbo_tasks.schedule_notify_tasks_set(&take(&mut self.dependent_tasks)); @@ -82,7 +56,7 @@ impl Output { message: Option>, turbo_tasks: &dyn TurboTasksBackendApi, ) { - self.content = OutputContent::Panic(message.map(Box::new)); + self.content = Some(OutputContent::Panic(message.map(Box::new))); // notify if !self.dependent_tasks.is_empty() { turbo_tasks.schedule_notify_tasks_set(&take(&mut self.dependent_tasks)); @@ -100,8 +74,8 @@ impl Output { impl PartialEq for Output { fn eq(&self, rhs: &RawVc) -> bool { match &self.content { - OutputContent::Link(old_target) => old_target == rhs, - OutputContent::Empty | OutputContent::Error(_) | OutputContent::Panic(_) => false, + Some(OutputContent::Link(old_target)) => old_target == rhs, + Some(OutputContent::Error(_) | OutputContent::Panic(_)) | None => false, } } } diff --git a/turbopack/crates/turbo-tasks-memory/src/task.rs b/turbopack/crates/turbo-tasks-memory/src/task.rs index 3689b75a872f8..aacb6f138f66e 100644 --- a/turbopack/crates/turbo-tasks-memory/src/task.rs +++ b/turbopack/crates/turbo-tasks-memory/src/task.rs @@ -32,7 +32,7 @@ use crate::{ cell::{Cell, ReadContentError}, edges_set::{TaskEdge, TaskEdgesList, TaskEdgesSet}, gc::{GcQueue, GcTaskState}, - output::{Output, OutputContent}, + output::Output, task::aggregation::{TaskAggregationContext, TaskChange}, MemoryBackend, }; @@ -876,7 +876,7 @@ impl Task { Ok(Ok(result)) => { if state.output != result { if cfg!(feature = "print_task_invalidation") - && !matches!(state.output.content, OutputContent::Empty) + && state.output.content.is_some() { println!( "Task {{ id: {}, name: {} }} invalidates:", diff --git a/turbopack/crates/turbo-tasks/src/lib.rs b/turbopack/crates/turbo-tasks/src/lib.rs index 9fce176ff3b8d..a289b5ad18aea 100644 --- a/turbopack/crates/turbo-tasks/src/lib.rs +++ b/turbopack/crates/turbo-tasks/src/lib.rs @@ -56,6 +56,7 @@ mod manager; mod native_function; mod no_move_vec; mod once_map; +mod output; pub mod persisted_graph; pub mod primitives; mod raw_vc; @@ -98,6 +99,7 @@ pub use manager::{ TurboTasksBackendApi, TurboTasksBackendApiExt, TurboTasksCallApi, Unused, UpdateInfo, }; pub use native_function::{FunctionMeta, NativeFunction}; +pub use output::OutputContent; pub use raw_vc::{CellId, RawVc, ReadRawVcFuture, ResolveTypeError}; pub use read_ref::ReadRef; use rustc_hash::FxHasher; diff --git a/turbopack/crates/turbo-tasks/src/output.rs b/turbopack/crates/turbo-tasks/src/output.rs new file mode 100644 index 0000000000000..5a0ad3a7882b6 --- /dev/null +++ b/turbopack/crates/turbo-tasks/src/output.rs @@ -0,0 +1,40 @@ +use std::{ + borrow::Cow, + fmt::{self, Display}, +}; + +use anyhow::anyhow; + +use crate::{util::SharedError, RawVc}; + +/// A helper type representing the output of a resolved task. +#[derive(Clone, Debug)] +pub enum OutputContent { + Link(RawVc), + Error(SharedError), + Panic(Option>>), +} + +impl OutputContent { + /// INVALIDATION: Be careful with this, it will not track dependencies, so + /// using it could break cache invalidation. + pub fn read_untracked(&self) -> anyhow::Result { + match &self { + Self::Error(err) => Err(anyhow::Error::new(err.clone())), + Self::Link(raw_vc) => Ok(*raw_vc), + Self::Panic(Some(message)) => Err(anyhow!("A task panicked: {message}")), + Self::Panic(None) => Err(anyhow!("A task panicked")), + } + } +} + +impl Display for OutputContent { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Link(raw_vc) => write!(f, "link {:?}", raw_vc), + Self::Error(err) => write!(f, "error {}", err), + Self::Panic(Some(message)) => write!(f, "panic {}", message), + Self::Panic(None) => write!(f, "panic"), + } + } +} From ebbfdd6fae86a3f885b05e9144aeaa3d3824b5a5 Mon Sep 17 00:00:00 2001 From: Benjamin Woodruff Date: Tue, 10 Sep 2024 11:20:53 -0700 Subject: [PATCH 2/2] Rename OutputContent::read_untracked to OutputContent::as_read_result --- turbopack/crates/turbo-tasks-memory/src/output.rs | 2 +- turbopack/crates/turbo-tasks/src/output.rs | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/turbopack/crates/turbo-tasks-memory/src/output.rs b/turbopack/crates/turbo-tasks-memory/src/output.rs index a53b7397b155b..b6dee0a245224 100644 --- a/turbopack/crates/turbo-tasks-memory/src/output.rs +++ b/turbopack/crates/turbo-tasks-memory/src/output.rs @@ -24,7 +24,7 @@ impl Output { pub fn read_untracked(&self) -> Result { match &self.content { None => Err(anyhow!("Output is empty")), - Some(content) => content.read_untracked(), + Some(content) => content.as_read_result(), } } diff --git a/turbopack/crates/turbo-tasks/src/output.rs b/turbopack/crates/turbo-tasks/src/output.rs index 5a0ad3a7882b6..ea3d3b5697ce2 100644 --- a/turbopack/crates/turbo-tasks/src/output.rs +++ b/turbopack/crates/turbo-tasks/src/output.rs @@ -16,9 +16,7 @@ pub enum OutputContent { } impl OutputContent { - /// INVALIDATION: Be careful with this, it will not track dependencies, so - /// using it could break cache invalidation. - pub fn read_untracked(&self) -> anyhow::Result { + pub fn as_read_result(&self) -> anyhow::Result { match &self { Self::Error(err) => Err(anyhow::Error::new(err.clone())), Self::Link(raw_vc) => Ok(*raw_vc),