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

Turbopack: find client references layout segment optimization #70792

Merged
merged 7 commits into from
Oct 9, 2024
+175 −35
Merged

Turbopack: find client references layout segment optimization #70792

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
161b588
fix
mischnic Oct 4, 2024
00e6cca
fixups
mischnic Oct 4, 2024
3e04e55
Cleanup
mischnic Oct 4, 2024
7ac962c
Remove typecast hack
mischnic Oct 4, 2024
e25f33b
Merge branch 'canary' into mischnic/perf-find-client-refs-graph-walking
mischnic Oct 7, 2024
d4b05a4
Merge branch 'canary' into mischnic/perf-find-client-refs-graph-walking
mischnic Oct 8, 2024
cc22dc4
Merge branch 'canary' into mischnic/perf-find-client-refs-graph-walking
mischnic Oct 9, 2024
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
56 changes: 45 additions & 11 deletions crates/next-api/src/app.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ use next_core::{
get_client_module_options_context, get_client_resolve_options_context,
get_client_runtime_entries, ClientContextType, RuntimeEntries,
},
next_client_reference::{client_reference_graph, NextEcmascriptClientReferenceTransition},
next_client_reference::{
client_reference_graph, ClientReferenceGraphResult, NextEcmascriptClientReferenceTransition,
},
next_config::NextConfig,
next_dynamic::NextDynamicTransition,
next_edge::route_regex::get_named_middleware_regex,
Expand Down Expand Up @@ -62,7 +64,7 @@ use turbopack_core::{
source::Source,
virtual_output::VirtualOutputAsset,
};
use turbopack_ecmascript::resolve::cjs_resolve;
use turbopack_ecmascript::{resolve::cjs_resolve, tree_shake::asset::EcmascriptModulePartAsset};

use crate::{
dynamic_imports::{
Expand Down Expand Up @@ -834,8 +836,6 @@ impl AppEndpoint {

let rsc_entry = app_entry.rsc_entry;

let rsc_entry_asset = Vc::upcast(rsc_entry);

let client_chunking_context = this.app_project.project().client_chunking_context();

let (app_server_reference_modules, client_dynamic_imports, client_references) =
Expand All @@ -862,7 +862,41 @@ impl AppEndpoint {
}
let client_shared_availability_info = client_shared_chunk_group.availability_info;

let client_references = client_reference_graph(Vc::cell(vec![rsc_entry_asset]));
let client_references = {
let modules = if let Some(module_part_asset) =
Vc::try_resolve_downcast_type::<EcmascriptModulePartAsset>(rsc_entry)
.await?
sokra marked this conversation as resolved.
Show resolved Hide resolved
{
let modules: Vec<_> = module_part_asset
.await?
.full_module
.await?
.inner_assets
.iter()
.try_join()
.await?
.iter()
.flat_map(|a| a.values())
.copied()
.chain(std::iter::once(Vc::upcast(module_part_asset)))
.collect();
modules
} else {
vec![rsc_entry]
};

let mut client_references: ClientReferenceGraphResult =
ClientReferenceGraphResult::default();

for module in modules {
sokra marked this conversation as resolved.
Show resolved Hide resolved
let current_client_references =
client_reference_graph(module, client_references.visited_nodes).await?;

client_references.extend(&current_client_references);
}
client_references
};
let client_references_cell = client_references.clone().cell();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let client_references_cell = client_references.clone().cell();
let client_references_cell = client_references.cell();

Don't we already own the value? Do we need to clone it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Various calls further down need a Vc<> to pass as an argument but the fields in client_references are also used multiple times.
So this clone isn't needed because the value is not owned, but because cell() consumes it and we still want to read client_references.


let ssr_chunking_context = if process_ssr {
Some(match runtime {
Expand All @@ -883,7 +917,6 @@ impl AppEndpoint {
let mut visited_modules = VisitedDynamicImportModules::empty();

for refs in client_references
.await?
.client_references_by_server_component
.values()
{
Expand All @@ -906,7 +939,7 @@ impl AppEndpoint {
};

let client_references_chunks = get_app_client_references_chunks(
client_references,
client_references_cell,
client_chunking_context,
Value::new(client_shared_availability_info),
ssr_chunking_context,
Expand Down Expand Up @@ -1006,7 +1039,7 @@ impl AppEndpoint {
node_root,
client_relative_path,
app_entry.original_name.clone(),
client_references,
client_references_cell,
client_references_chunks,
client_chunking_context,
ssr_chunking_context,
Expand Down Expand Up @@ -1034,7 +1067,9 @@ impl AppEndpoint {
}

(
Some(get_app_server_reference_modules(client_references.types())),
Some(get_app_server_reference_modules(
client_references_cell.types(),
)),
Some(client_dynamic_imports),
Some(client_references),
)
Expand Down Expand Up @@ -1271,8 +1306,7 @@ impl AppEndpoint {
let mut current_chunks = OutputAssets::empty();
let mut current_availability_info = AvailabilityInfo::Root;
if let Some(client_references) = client_references {
let client_references = client_references.await?;
let span = tracing::trace_span!("server utils",);
let span = tracing::trace_span!("server utils");
async {
let utils_module = IncludeModulesModule::new(
AssetIdent::from_path(this.app_project.project().project_path())
Expand Down
2 changes: 1 addition & 1 deletion crates/next-core/src/next_client_reference/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ pub use ecmascript_client_reference::{
};
pub use visit_client_reference::{
client_reference_graph, ClientReference, ClientReferenceGraphResult, ClientReferenceType,
ClientReferenceTypes,
ClientReferenceTypes, VisitedClientReferenceGraphNodes,
};
111 changes: 76 additions & 35 deletions crates/next-core/src/next_client_reference/visit_client_reference.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,18 @@
use std::future::Future;
use std::{collections::HashSet, future::Future};

use anyhow::Result;
use indexmap::{IndexMap, IndexSet};
use serde::{Deserialize, Serialize};
use tracing::Instrument;
use turbo_tasks::{
debug::ValueDebugFormat,
graph::{AdjacencyMap, GraphTraversal, Visit, VisitControlFlow},
graph::{AdjacencyMap, GraphTraversal, Visit, VisitControlFlow, VisitedNodes},
trace::TraceRawVcs,
RcStr, ReadRef, TryJoinIterExt, ValueToString, Vc,
};
use turbo_tasks_fs::FileSystemPath;
use turbopack::css::CssModuleAsset;
use turbopack_core::{
module::{Module, Modules},
reference::primary_referenced_modules,
};
use turbopack_core::{module::Module, reference::primary_referenced_modules};

use super::ecmascript_client_reference::ecmascript_client_reference_module::EcmascriptClientReferenceModule;
use crate::next_server_component::server_component_module::NextServerComponentModule;
Expand Down Expand Up @@ -46,8 +43,8 @@ pub enum ClientReferenceType {
CssClientReference(Vc<CssModuleAsset>),
}

#[turbo_tasks::value]
#[derive(Debug)]
#[turbo_tasks::value(shared)]
#[derive(Clone, Debug)]
pub struct ClientReferenceGraphResult {
pub client_references: Vec<ClientReference>,
/// Only the [`ClientReferenceType::EcmascriptClientReference`]s are listed in this map.
Expand All @@ -56,6 +53,30 @@ pub struct ClientReferenceGraphResult {
IndexMap<Option<Vc<NextServerComponentModule>>, Vec<Vc<Box<dyn Module>>>>,
pub server_component_entries: Vec<Vc<NextServerComponentModule>>,
pub server_utils: Vec<Vc<Box<dyn Module>>>,
pub visited_nodes: Vc<VisitedClientReferenceGraphNodes>,
}

impl Default for ClientReferenceGraphResult {
fn default() -> Self {
ClientReferenceGraphResult {
client_references: Default::default(),
client_references_by_server_component: Default::default(),
server_component_entries: Default::default(),
server_utils: Default::default(),
visited_nodes: VisitedClientReferenceGraphNodes::empty(),
}
}
}

#[turbo_tasks::value(shared)]
pub struct VisitedClientReferenceGraphNodes(HashSet<VisitClientReferenceNode>);

#[turbo_tasks::value_impl]
impl VisitedClientReferenceGraphNodes {
#[turbo_tasks::function]
pub fn empty() -> Vc<Self> {
VisitedClientReferenceGraphNodes(Default::default()).cell()
}
}

#[turbo_tasks::value(transparent)]
Expand All @@ -74,13 +95,31 @@ impl ClientReferenceGraphResult {
}
}

impl ClientReferenceGraphResult {
/// Merges multiple return values of client_reference_graph together.
pub fn extend(&mut self, other: &Self) {
self.client_references
.extend(other.client_references.iter().copied());
for (k, v) in other.client_references_by_server_component.iter() {
self.client_references_by_server_component
.entry(*k)
.or_insert_with(Vec::new)
.extend(v);
}
self.server_component_entries
.extend(other.server_component_entries.iter().copied());
self.server_utils.extend(other.server_utils.iter().copied());
// This is merged already by `client_reference_graph` itself
self.visited_nodes = other.visited_nodes;
}
}

#[turbo_tasks::function]
pub async fn client_reference_graph(
entries: Vc<Modules>,
entry: Vc<Box<dyn Module>>,
visited_nodes: Vc<VisitedClientReferenceGraphNodes>,
) -> Result<Vc<ClientReferenceGraphResult>> {
async move {
sokra marked this conversation as resolved.
Show resolved Hide resolved
let entries = entries.await?;

let mut client_references = vec![];
let mut server_component_entries = vec![];
let mut server_utils = vec![];
Expand All @@ -90,33 +129,34 @@ pub async fn client_reference_graph(
// first
client_references_by_server_component.insert(None, Vec::new());

let graph = AdjacencyMap::new()
.skip_duplicates()
let (graph, visited_nodes) = AdjacencyMap::new()
.skip_duplicates_with_visited_nodes(VisitedNodes(visited_nodes.await?.0.clone()))
.visit(
entries
.iter()
.copied()
.map(|module| async move {
Ok(VisitClientReferenceNode {
state: VisitClientReferenceNodeState::Entry {
entry_path: module.ident().path().resolve().await?,
},
ty: VisitClientReferenceNodeType::Internal(
module,
module.ident().to_string().await?,
),
})
})
.try_join()
.await?,
vec![VisitClientReferenceNode {
state: {
if let Some(server_component) =
Vc::try_resolve_downcast_type::<NextServerComponentModule>(entry)
.await?
{
VisitClientReferenceNodeState::InServerComponent { server_component }
} else {
VisitClientReferenceNodeState::Entry {
entry_path: entry.ident().path().resolve().await?,
}
}
},
ty: VisitClientReferenceNodeType::Internal(
entry,
entry.ident().to_string().await?,
),
}],
VisitClientReference,
)
.await
.completed()?
.into_inner()
.into_reverse_topological();
.into_inner_with_visited();

for node in graph {
for node in graph.into_reverse_topological() {
match &node.ty {
VisitClientReferenceNodeType::Internal(_asset, _) => {
// No-op. These nodes are only useful during graph
Expand Down Expand Up @@ -148,6 +188,7 @@ pub async fn client_reference_graph(
client_references_by_server_component,
server_component_entries,
server_utils,
visited_nodes: VisitedClientReferenceGraphNodes(visited_nodes.0).cell(),
}
.cell())
}
Expand Down Expand Up @@ -207,9 +248,9 @@ impl Visit<VisitClientReferenceNode> for VisitClientReference {
fn visit(&mut self, edge: Self::Edge) -> VisitControlFlow<VisitClientReferenceNode> {
match edge.ty {
VisitClientReferenceNodeType::ClientReference(..) => VisitControlFlow::Skip(edge),
VisitClientReferenceNodeType::Internal(..) => VisitControlFlow::Continue(edge),
VisitClientReferenceNodeType::ServerUtilEntry(..) => VisitControlFlow::Continue(edge),
VisitClientReferenceNodeType::ServerComponentEntry(..) => {
VisitClientReferenceNodeType::Internal(..)
| VisitClientReferenceNodeType::ServerUtilEntry(..)
| VisitClientReferenceNodeType::ServerComponentEntry(..) => {
VisitControlFlow::Continue(edge)
}
}
Expand Down
Loading