Skip to content

Commit

Permalink
Tree Shaking grouping of items (vercel/turborepo#8420)
Browse files Browse the repository at this point in the history 
### Description

add test case

### Testing Instructions

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

---------

Co-authored-by: 강동윤 (Donny) <kdy1997.dev@gmail.com>
  • Loading branch information
@sokra @kdy1
sokra and kdy1 authored Jun 17, 2024
1 parent 054f8f6 commit 6ce9bf7
Show file tree
Hide file tree
Showing 53 changed files with 2,615 additions and 942 deletions.
13 changes: 12 additions & 1 deletion crates/turbopack-ecmascript/src/analyzer/imports.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ impl Visit for Analyzer<'_> {

let internal_symbol = parse_with(import.with.as_deref());

if internal_symbol.is_none() || import.specifiers.is_empty() {
if internal_symbol.is_none() {
self.ensure_reference(
import.span,
import.src.value.clone(),
Expand Down Expand Up @@ -310,6 +310,17 @@ impl Visit for Analyzer<'_> {

self.data.imports.insert(local, (i, orig_sym));
}

if import.specifiers.is_empty() {
if let Some(internal_symbol) = internal_symbol {
self.ensure_reference(
import.span,
import.src.value.clone(),
internal_symbol,
annotations,
);
}
}
}

fn visit_export_all(&mut self, export: &ExportAll) {
Expand Down
6 changes: 2 additions & 4 deletions crates/turbopack-ecmascript/src/references/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,7 @@ pub(crate) async fn analyse_ecmascript_module_internal(
},
import_externals,
);

import_references.push(r);
}

Expand Down Expand Up @@ -2571,10 +2572,7 @@ impl<'a> VisitAstPath for ModuleReferencesVisitor<'a> {
if export.src.is_none() {
for spec in export.specifiers.iter() {
fn to_string(name: &ModuleExportName) -> &JsWord {
match name {
ModuleExportName::Ident(ident) => &ident.sym,
ModuleExportName::Str(str) => &str.value,
}
name.atom()
}
match spec {
ExportSpecifier::Namespace(_) => {
Expand Down
83 changes: 39 additions & 44 deletions crates/turbopack-ecmascript/src/tree_shake/graph.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ impl DepGraph {
for dep_item in dep_items {
let data = data.get(dep_item).unwrap();

for var in data.var_decls.iter().chain(data.write_vars.iter()) {
for var in data.var_decls.iter() {
if required_vars.remove(var) {
specifiers.push(ImportSpecifier::Named(ImportNamedSpecifier {
span: DUMMY_SP,
Expand Down Expand Up @@ -330,43 +330,36 @@ impl DepGraph {
let data = data.get(g).unwrap();

// Emit `export { foo }`
for var in data.write_vars.iter() {
if required_vars.remove(var)
|| data.read_vars.contains(var)
|| data.var_decls.contains(var)
{
let assertion_prop =
PropOrSpread::Prop(Box::new(Prop::KeyValue(KeyValueProp {
key: quote_ident!("__turbopack_var__").into(),
value: Box::new(true.into()),
})));

chunk
.body
.push(ModuleItem::ModuleDecl(ModuleDecl::ExportNamed(
NamedExport {
for var in data.var_decls.iter() {
let assertion_prop =
PropOrSpread::Prop(Box::new(Prop::KeyValue(KeyValueProp {
key: quote_ident!("__turbopack_var__").into(),
value: Box::new(true.into()),
})));

chunk
.body
.push(ModuleItem::ModuleDecl(ModuleDecl::ExportNamed(
NamedExport {
span: DUMMY_SP,
specifiers: vec![ExportSpecifier::Named(ExportNamedSpecifier {
span: DUMMY_SP,
specifiers: vec![ExportSpecifier::Named(
ExportNamedSpecifier {
span: DUMMY_SP,
orig: ModuleExportName::Ident(var.clone().into()),
exported: None,
is_type_only: false,
},
)],
src: if cfg!(test) {
Some(Box::new("__TURBOPACK_VAR__".into()))
} else {
None
},
type_only: false,
with: Some(Box::new(ObjectLit {
span: DUMMY_SP,
props: vec![assertion_prop],
})),
orig: ModuleExportName::Ident(var.clone().into()),
exported: None,
is_type_only: false,
})],
src: if cfg!(test) {
Some(Box::new("__TURBOPACK_VAR__".into()))
} else {
None
},
)));
}
type_only: false,
with: Some(Box::new(ObjectLit {
span: DUMMY_SP,
props: vec![assertion_prop],
})),
},
)));
}
}

Expand Down Expand Up @@ -405,15 +398,15 @@ impl DepGraph {
.idx_graph
.neighbors_directed(start_ix, petgraph::Direction::Outgoing)
{
let dep_id = graph.graph_ix.get_index(dep_ix as _).unwrap().clone();
let dep_id = graph.graph_ix.get_index(dep_ix as _).unwrap();

if global_done.insert(dep_ix)
|| (data.get(&dep_id).map_or(false, |data| data.pure)
|| (data.get(dep_id).map_or(false, |data| data.pure)
&& group_done.insert(dep_ix))
{
changed = true;

group.push(dep_id);
group.push(dep_id.clone());

add_to_group(graph, data, group, dep_ix, global_done, group_done);
}
Expand Down Expand Up @@ -498,8 +491,8 @@ impl DepGraph {
let mut changed = false;

for (group, group_done) in &mut groups {
let start = group[0].clone();
let start_ix = self.g.get_node(&start);
let start = &group[0];
let start_ix = self.g.get_node(start);
changed |=
add_to_group(&self.g, data, group, start_ix, &mut global_done, group_done);
}
Expand Down Expand Up @@ -704,11 +697,12 @@ impl DepGraph {
});

{
let used_ids = ids_used_by_ignoring_nested(
let mut used_ids = ids_used_by_ignoring_nested(
&export.decl,
unresolved_ctxt,
top_level_ctxt,
);
used_ids.write.insert(default_var.to_id());
let captured_ids =
ids_captured_by(&export.decl, unresolved_ctxt, top_level_ctxt);
let data = ItemData {
Expand Down Expand Up @@ -1084,6 +1078,7 @@ impl DepGraph {
type_only: false,
with: None,
})),
read_vars: [name.clone()].into_iter().collect(),
export: Some(name.0),
..Default::default()
},
Expand Down Expand Up @@ -1123,13 +1118,13 @@ impl DepGraph {
}
}

pub(crate) fn has_strong_dep(&mut self, id: &ItemId, dep: &ItemId) -> bool {
pub(crate) fn has_dep(&mut self, id: &ItemId, dep: &ItemId, only_strong: bool) -> bool {
let from = self.g.node(id);
let to = self.g.node(dep);
self.g
.idx_graph
.edge_weight(from, to)
.map(|d| matches!(d, Dependency::Strong))
.map(|d| matches!(d, Dependency::Strong) || !only_strong)
.unwrap_or(false)
}

Expand Down
43 changes: 27 additions & 16 deletions crates/turbopack-ecmascript/src/tree_shake/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ pub struct Analyzer<'a> {

#[derive(Debug, Default, Clone)]
struct VarState {
declarator: Option<ItemId>,

/// The module items that might trigger side effects on that variable.
/// We also store if this is a `const` write, so no further change will
/// happen to this var.
Expand Down Expand Up @@ -130,15 +132,29 @@ impl Analyzer<'_> {
continue;
}

for id in item.var_decls.iter() {
let state = self.vars.entry(id.clone()).or_default();
if state.declarator.is_none() {
state.declarator = Some(item_id.clone());
}
}

// For each var in READ_VARS:
for id in item.read_vars.iter() {
// Create a strong dependency to all module items listed in LAST_WRITES for that
// var.

// (the writes need to be executed before this read)
let state = get_var(&self.vars, id);

self.g.add_strong_deps(item_id, state.last_writes.iter());

if let Some(declarator) = &state.declarator {
if declarator != item_id {
// A read also depends on the declaration.
self.g
.add_strong_deps(item_id, [declarator].iter().copied());
}
}
}

// For each var in WRITE_VARS:
Expand All @@ -151,6 +167,13 @@ impl Analyzer<'_> {

let state = get_var(&self.vars, id);
self.g.add_weak_deps(item_id, state.last_reads.iter());

if let Some(declarator) = &state.declarator {
if declarator != item_id {
// A write also depends on the declaration.
self.g.add_weak_deps(item_id, [declarator].iter().copied());
}
}
}

if item.side_effects {
Expand Down Expand Up @@ -181,7 +204,7 @@ impl Analyzer<'_> {

state
.last_writes
.retain(|last_write| !self.g.has_strong_dep(item_id, last_write));
.retain(|last_write| !self.g.has_dep(item_id, last_write, true));

// Drop all writes which are not reachable from this item.
//
Expand Down Expand Up @@ -211,7 +234,7 @@ impl Analyzer<'_> {

state
.last_reads
.retain(|last_read| !self.g.has_strong_dep(item_id, last_read));
.retain(|last_read| !self.g.has_dep(item_id, last_read, true));
}

if item.side_effects {
Expand Down Expand Up @@ -307,13 +330,6 @@ async fn get_part_id(result: &SplitResult, part: Vc<ModulePart>) -> Result<u32>
let part_id = match entrypoints.get(&key) {
Some(id) => *id,
None => {
// We need to handle `*` reexports specially.
if let ModulePart::Export(..) = &*part {
if let Some(&part_id) = entrypoints.get(&Key::ModuleEvaluation) {
return Ok(part_id);
}
}

bail!(
"could not find part id for module part {:?} in {:?}",
key,
Expand Down Expand Up @@ -406,14 +422,9 @@ pub(super) async fn split(
} = dep_graph.split_module(&items);

assert_ne!(modules.len(), 0, "modules.len() == 0;\nModule: {module:?}",);
assert_eq!(
entrypoints.get(&Key::ModuleEvaluation),
Some(&0),
"ModuleEvaluation is not the first module"
);

for &v in entrypoints.values() {
assert!(
debug_assert!(
v < modules.len() as u32,
"Invalid entrypoint '{}' while there are only '{}' modules",
v,
Expand Down
Loading

0 comments on commit 6ce9bf7

Please sign in to comment.