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.

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

feat: add mechanism to bypass turbo engine for annotated methods #7675

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion crates/turbo-tasks-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,21 @@ use turbo_tasks_macros_shared::{
get_type_ident, GenericTypeInput, PrimitiveInput, ValueTraitArguments,
};

/// the same module exists in turbo-tasks-macros but due to its size we
/// just copy the relevant parts in both places
mod ignored {
use std::{cell::OnceCell, collections::HashSet};
// a newline-separated list of task names to opt-out of turbo tracking
const IGNORE_TASKS_RAW: Option<&str> = std::option_env!("TURBO_IGNORE_TASKS");
const IGNORE_TASKS: OnceCell<HashSet<&'static str>> = OnceCell::new();

pub fn task_ignored(name: &str) -> bool {
IGNORE_TASKS
.get_or_init(|| IGNORE_TASKS_RAW.unwrap_or_default().split(',').collect())
.contains(name)
}
}

pub fn generate_register() {
println!("cargo:rerun-if-changed=build.rs");

Expand Down Expand Up @@ -221,7 +236,9 @@ impl<'a> RegisterContext<'a> {
}

fn process_fn(&mut self, fn_item: ItemFn) -> Result<()> {
if has_attribute(&fn_item.attrs, "function") {
if has_attribute(&fn_item.attrs, "function")
&& !ignored::task_ignored(&fn_item.sig.ident.to_string())
Comment on lines +239 to +240
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A problem is probably that just ignoring the attribute is not enough. The attribute is also changing the signature, e. g. async fn X() -> Result<Vc<Y>> becomes fn X() -> Vc<Y>.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I am going to see if it is possible to rewrite the signatures as well, have been messing around with it today to see if we can do that without requiring upstream tasks to change

{
let ident = &fn_item.sig.ident;
let type_ident = get_native_function_ident(ident);

Expand Down
26 changes: 26 additions & 0 deletions crates/turbo-tasks-macros/src/function_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,35 @@

use crate::func::{DefinitionContext, NativeFn, TurboFn};

/// the same module exists in turbo-tasks-build but due to its size we
/// just copy the relevant parts in both places
mod ignored {
use std::{cell::OnceCell, collections::HashSet};
// a newline-separated list of task names to opt-out of turbo tracking
const IGNORE_TASKS_RAW: Option<&str> = std::option_env!("TURBO_IGNORE_TASKS");
const IGNORE_TASKS: OnceCell<HashSet<&'static str>> = OnceCell::new();

Check failure on line 15 in crates/turbo-tasks-macros/src/function_macro.rs

View workflow job for this annotation

GitHub Actions / Turbopack rust clippy

a `const` item should never be interior mutable

pub fn task_ignored(name: &str) -> bool {
IGNORE_TASKS

Check failure on line 18 in crates/turbo-tasks-macros/src/function_macro.rs

View workflow job for this annotation

GitHub Actions / Turbopack rust clippy

a `const` item with interior mutability should not be borrowed
.get_or_init(|| IGNORE_TASKS_RAW.unwrap_or_default().split(',').collect())
.contains(name)
}
}

pub fn function(_args: TokenStream, input: TokenStream) -> TokenStream {
let item = parse_macro_input!(input as ItemFn);

// TODO: ideally here we would be able to match on the entire module path
// however macros are evaluated in a different context and we don't
// have access to it. in the mean time, just use the function name.
// the likelihood that two functions with the same name exist is low
// and, if we do need to disable just one of them, can be mitigated
// through naming
let name = item.sig.ident.to_string();
if ignored::task_ignored(&name) {
return quote! { #item }.into();
}

let ItemFn {
attrs,
vis,
Expand Down
16 changes: 9 additions & 7 deletions crates/turbopack-trace-server/src/bottom_up.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use std::{collections::HashMap, env, sync::Arc};

use either::Either;

use crate::{
span::{SpanBottomUp, SpanIndex},
span_ref::SpanRef,
Expand Down Expand Up @@ -36,16 +34,20 @@ impl SpanBottomUpBuilder {
}

pub fn build_bottom_up_graph<'a>(
spans: impl IntoIterator<Item = SpanRef<'a>>,
spans: impl Iterator<Item = SpanRef<'a>>,
) -> Vec<Arc<SpanBottomUp>> {
let max_depth = env::var("BOTTOM_UP_DEPTH")
.ok()
.and_then(|s| s.parse().ok())
.unwrap_or(usize::MAX);
let mut roots = HashMap::new();
let mut current_iterators = vec![Either::Left(
spans.into_iter().flat_map(|span| span.children()),
)];

// unfortunately there is a rustc bug that fails the typechecking here
// when using Either<impl Iterator, impl Iterator>. This error appears
// in certain cases when building next-swc.
let mut current_iterators: Vec<Box<dyn Iterator<Item = SpanRef<'_>>>> =
vec![Box::new(spans.flat_map(|span| span.children()))];

let mut current_path: Vec<(&'_ str, SpanIndex)> = vec![];
while let Some(mut iter) = current_iterators.pop() {
if let Some(child) = iter.next() {
Expand Down Expand Up @@ -73,7 +75,7 @@ pub fn build_bottom_up_graph<'a>(
}

current_path.push((child.group_name(), child.index()));
current_iterators.push(Either::Right(child.children()));
current_iterators.push(Box::new(child.children()));
} else {
current_path.pop();
}
Expand Down
42 changes: 42 additions & 0 deletions crates/turbopack-trace-server/src/server.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::{
collections::HashSet,
net::{TcpListener, TcpStream},
num::NonZeroUsize,
sync::{Arc, Mutex},
thread::spawn,
};
Expand All @@ -9,6 +11,7 @@ use serde::{Deserialize, Serialize};
use tungstenite::{accept, Message};

use crate::{
span_ref::SpanRef,
store::SpanId,
store_container::StoreContainer,
u64_string,
Expand Down Expand Up @@ -80,6 +83,19 @@ pub struct SpanViewEvent {
pub id: Option<SpanId>,
}

#[derive(Serialize, Deserialize, Debug)]
pub struct Filter {
pub op: Op,
pub value: u64,
}

#[derive(Serialize, Deserialize, Debug)]
#[serde(rename_all = "snake_case")]
pub enum Op {
Gt,
Lt,
}

#[derive(Serialize, Deserialize, Debug)]
#[serde(rename_all = "camelCase")]
pub struct ViewRect {
Expand All @@ -91,6 +107,30 @@ pub struct ViewRect {
pub query: String,
pub view_mode: String,
pub value_mode: String,
pub value_filter: Option<Filter>,
pub count_filter: Option<Filter>,
}

impl ViewRect {
pub fn should_filter_ref(
&self,
span: &SpanRef,
highlighted_spans: &mut HashSet<NonZeroUsize>,
) -> bool {
let mut has_results = false;
for mut result in span.search(&self.query) {
has_results = true;
highlighted_spans.insert(result.id());
while let Some(parent) = result.parent() {
result = parent;
if !highlighted_spans.insert(result.id()) {
break;
}
}
}

!has_results
}
}

struct ConnectionState {
Expand Down Expand Up @@ -130,6 +170,8 @@ fn handle_connection(
query: String::new(),
view_mode: "aggregated".to_string(),
value_mode: "duration".to_string(),
count_filter: None,
value_filter: None,
},
last_update_generation: 0,
}));
Expand Down
4 changes: 2 additions & 2 deletions crates/turbopack-trace-server/src/span_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,9 +331,9 @@ impl<'a> SpanRef<'a> {
pub fn bottom_up(self) -> impl Iterator<Item = SpanBottomUpRef<'a>> {
self.extra()
.bottom_up
.get_or_init(|| build_bottom_up_graph([self]))
.get_or_init(|| build_bottom_up_graph([self].into_iter()))
.iter()
.map(|bottom_up| SpanBottomUpRef {
.map(move |bottom_up| SpanBottomUpRef {
bottom_up: bottom_up.clone(),
store: self.store,
})
Expand Down
Loading
Loading