Skip to content

Commit

Permalink
fix: amd require array (#9113)
Browse files Browse the repository at this point in the history
  • Loading branch information
ahabhgk authored Jan 24, 2025
1 parent 79acbfd commit f6401df
Show file tree
Hide file tree
Showing 10 changed files with 177 additions and 75 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ impl Branch {

#[cacheable]
#[derive(Debug, Clone)]
pub struct AmdDefineDependency {
pub struct AMDDefineDependency {
id: DependencyId,
range: (u32, u32),
array_range: Option<(u32, u32)>,
Expand All @@ -175,7 +175,7 @@ pub struct AmdDefineDependency {
local_module: Option<LocalModule>,
}

impl AmdDefineDependency {
impl AMDDefineDependency {
pub fn new(
range: (u32, u32),
array_range: Option<(u32, u32)>,
Expand All @@ -201,7 +201,7 @@ impl AmdDefineDependency {
}

#[cacheable_dyn]
impl Dependency for AmdDefineDependency {
impl Dependency for AMDDefineDependency {
fn id(&self) -> &DependencyId {
&self.id
}
Expand All @@ -219,7 +219,7 @@ impl Dependency for AmdDefineDependency {
}
}

impl AmdDefineDependency {
impl AMDDefineDependency {
fn local_module_var(&self) -> Option<String> {
self.local_module.as_ref().and_then(|m| {
if m.is_used() {
Expand Down Expand Up @@ -249,7 +249,7 @@ impl AmdDefineDependency {
}

#[cacheable_dyn]
impl DependencyTemplate for AmdDefineDependency {
impl DependencyTemplate for AMDDefineDependency {
fn apply(
&self,
source: &mut TemplateReplaceSource,
Expand Down Expand Up @@ -305,6 +305,6 @@ impl DependencyTemplate for AmdDefineDependency {
}
}

impl AsModuleDependency for AmdDefineDependency {}
impl AsModuleDependency for AMDDefineDependency {}

impl AsContextDependency for AmdDefineDependency {}
impl AsContextDependency for AMDDefineDependency {}
Original file line number Diff line number Diff line change
@@ -1,35 +1,33 @@
use std::borrow::Cow;

use itertools::Itertools;
use rspack_cacheable::{cacheable, cacheable_dyn, with::AsPreset};
use rspack_cacheable::{cacheable, cacheable_dyn};
use rspack_core::{
module_raw, AffectType, AsContextDependency, AsModuleDependency, Compilation, Dependency,
DependencyCategory, DependencyId, DependencyTemplate, DependencyType, ModuleDependency,
RuntimeSpec, TemplateContext, TemplateReplaceSource,
};
use rspack_util::atom::Atom;

use super::{
amd_require_item_dependency::AMDRequireItemDependency,
local_module_dependency::LocalModuleDependency,
};
use super::amd_require_item_dependency::AMDRequireItemDependency;

#[cacheable]
#[derive(Debug, Clone)]
pub enum AmdDep {
String(#[cacheable(with=AsPreset)] Atom),
LocalModuleDependency(LocalModuleDependency),
AMDRequireItemDependency(AMDRequireItemDependency),
pub enum AMDRequireArrayItem {
String(String),
LocalModuleDependency { local_module_variable_name: String },
AMDRequireItemDependency { dep_id: DependencyId },
}

#[cacheable]
#[derive(Debug, Clone)]
pub struct AmdRequireArrayDependency {
pub struct AMDRequireArrayDependency {
id: DependencyId,
deps_array: Vec<AmdDep>,
deps_array: Vec<AMDRequireArrayItem>,
range: (u32, u32),
}

impl AmdRequireArrayDependency {
pub fn new(deps_array: Vec<AmdDep>, range: (u32, u32)) -> Self {
impl AMDRequireArrayDependency {
pub fn new(deps_array: Vec<AMDRequireArrayItem>, range: (u32, u32)) -> Self {
Self {
id: DependencyId::new(),
deps_array,
Expand All @@ -39,7 +37,7 @@ impl AmdRequireArrayDependency {
}

#[cacheable_dyn]
impl Dependency for AmdRequireArrayDependency {
impl Dependency for AMDRequireArrayDependency {
fn id(&self) -> &DependencyId {
&self.id
}
Expand All @@ -57,7 +55,7 @@ impl Dependency for AmdRequireArrayDependency {
}
}

impl AmdRequireArrayDependency {
impl AMDRequireArrayDependency {
fn get_content(&self, code_generatable_context: &mut TemplateContext) -> String {
format!(
"[{}]",
Expand All @@ -69,26 +67,36 @@ impl AmdRequireArrayDependency {
)
}

fn content_for_dependency(
dep: &AmdDep,
fn content_for_dependency<'a>(
dep: &'a AMDRequireArrayItem,
code_generatable_context: &mut TemplateContext,
) -> String {
) -> Cow<'a, str> {
match dep {
AmdDep::String(name) => name.to_string(),
AmdDep::LocalModuleDependency(dep) => dep.get_variable_name(),
AmdDep::AMDRequireItemDependency(dep) => module_raw(
code_generatable_context.compilation,
code_generatable_context.runtime_requirements,
dep.id(),
dep.request(),
dep.weak(),
),
AMDRequireArrayItem::String(name) => name.into(),
AMDRequireArrayItem::LocalModuleDependency {
local_module_variable_name,
} => local_module_variable_name.into(),
AMDRequireArrayItem::AMDRequireItemDependency { dep_id } => {
let mg = code_generatable_context.compilation.get_module_graph();
let dep = mg
.dependency_by_id(dep_id)
.and_then(|dep| dep.downcast_ref::<AMDRequireItemDependency>())
.expect("should have AMDRequireItemDependency");
module_raw(
code_generatable_context.compilation,
code_generatable_context.runtime_requirements,
dep_id,
dep.request(),
dep.weak(),
)
.into()
}
}
}
}

#[cacheable_dyn]
impl DependencyTemplate for AmdRequireArrayDependency {
impl DependencyTemplate for AMDRequireArrayDependency {
fn apply(
&self,
source: &mut TemplateReplaceSource,
Expand All @@ -111,6 +119,6 @@ impl DependencyTemplate for AmdRequireArrayDependency {
}
}

impl AsModuleDependency for AmdRequireArrayDependency {}
impl AsModuleDependency for AMDRequireArrayDependency {}

impl AsContextDependency for AmdRequireArrayDependency {}
impl AsContextDependency for AMDRequireArrayDependency {}
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ pub struct AMDRequireItemDependency {
id: DependencyId,
#[cacheable(with=AsPreset)]
request: Atom,
range: (u32, u32),
range: Option<(u32, u32)>,
optional: bool,
}

impl AMDRequireItemDependency {
pub fn new(request: Atom, range: (u32, u32)) -> Self {
pub fn new(request: Atom, range: Option<(u32, u32)>) -> Self {
Self {
id: DependencyId::new(),
request,
Expand Down Expand Up @@ -57,6 +57,9 @@ impl DependencyTemplate for AMDRequireItemDependency {
source: &mut TemplateReplaceSource,
code_generatable_context: &mut TemplateContext,
) {
let Some(range) = &self.range else {
return;
};
// ModuleDependencyTemplateAsRequireId
let content = module_raw(
code_generatable_context.compilation,
Expand All @@ -65,7 +68,7 @@ impl DependencyTemplate for AMDRequireItemDependency {
&self.request,
self.weak(),
);
source.replace(self.range.0, self.range.1, &content, None);
source.replace(range.0, range.1, &content, None);
}

fn dependency_id(&self) -> Option<DependencyId> {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use std::borrow::Cow;

use rspack_core::{
BuildMetaDefaultObject, BuildMetaExportsType, ConstDependency, RuntimeGlobals, SpanExt,
BuildMetaDefaultObject, BuildMetaExportsType, ConstDependency, Dependency, RuntimeGlobals,
SpanExt,
};
use rspack_util::atom::Atom;
use rustc_hash::FxHashMap;
Expand All @@ -15,7 +16,8 @@ use swc_core::{

use crate::{
dependency::{
amd_define_dependency::AmdDefineDependency,
amd_define_dependency::AMDDefineDependency,
amd_require_array_dependency::{AMDRequireArrayDependency, AMDRequireArrayItem},
amd_require_item_dependency::AMDRequireItemDependency,
local_module_dependency::LocalModuleDependency,
},
Expand Down Expand Up @@ -65,21 +67,17 @@ fn is_callable(expr: &Expr) -> bool {
is_unbound_function_expression(expr) || is_bound_function_expression(expr)
}

/**
* lookup
*
* define('ui/foo/bar', ['./baz', '../qux'], ...);
* - 'ui/foo/baz'
* - 'ui/qux'
*/
fn resolve_mod_name(mod_name: &Option<Atom>, dep_name: &str) -> Atom {
if let Some(mod_name) = mod_name
&& dep_name.starts_with('.')
/// define('ui/foo/bar', ['./baz', '../qux'], ...);
/// - 'ui/foo/baz'
/// - 'ui/qux'
fn lookup(parent: &Option<Atom>, module: &str) -> Atom {
if let Some(parent) = parent
&& module.starts_with('.')
{
let mut path: Vec<&str> = mod_name.split('/').collect();
let mut path: Vec<&str> = parent.split('/').collect();
path.pop();

for seg in dep_name.split('.') {
for seg in module.split('.') {
if seg == ".." {
path.pop();
} else if seg != "." {
Expand All @@ -89,7 +87,7 @@ fn resolve_mod_name(mod_name: &Option<Atom>, dep_name: &str) -> Atom {

path.join("/").into()
} else {
dep_name.into()
module.into()
}
}

Expand Down Expand Up @@ -140,9 +138,38 @@ impl AMDDefineDependencyParserPlugin {
}
}
return Some(true);
} else if param.is_const_array() {
let mut deps: Vec<AMDRequireArrayItem> = vec![];
let array = param.array();
for (i, request) in array.iter().enumerate() {
if request == "require" {
identifiers.insert(i, REQUIRE);
deps.push(AMDRequireArrayItem::String(
RuntimeGlobals::REQUIRE.name().into(),
));
} else if request == "exports" {
identifiers.insert(i, EXPORTS);
deps.push(AMDRequireArrayItem::String(request.into()));
} else if request == "module" {
identifiers.insert(i, MODULE);
deps.push(AMDRequireArrayItem::String(request.into()));
} else if let Some(local_module) = parser.get_local_module_mut(request) {
local_module.flag_used();
deps.push(AMDRequireArrayItem::LocalModuleDependency {
local_module_variable_name: local_module.variable_name(),
})
} else {
let mut dep = AMDRequireItemDependency::new(request.as_str().into(), None);
dep.set_optional(parser.in_try);
deps.push(AMDRequireArrayItem::AMDRequireItemDependency { dep_id: *dep.id() });
parser.dependencies.push(Box::new(dep));
}
}
let range = param.range();
let dep = AMDRequireArrayDependency::new(deps, (range.0, range.1 - 1));
parser.presentational_dependencies.push(Box::new(dep));
return Some(true);
}
// currently, there is no ConstArray in rspack
// TODO: check if `param` is a const string array
None
}

Expand Down Expand Up @@ -193,7 +220,7 @@ impl AMDDefineDependencyParserPlugin {
Some(RuntimeGlobals::MODULE),
))
} else if let Some(local_module) =
parser.get_local_module_mut(&resolve_mod_name(named_module, param_str))
parser.get_local_module_mut(&lookup(named_module, param_str))
{
local_module.flag_used();
let dep = Box::new(LocalModuleDependency::new(
Expand All @@ -206,7 +233,7 @@ impl AMDDefineDependencyParserPlugin {
} else {
let mut dep = Box::new(AMDRequireItemDependency::new(
Atom::new(param_str.as_str()),
range,
Some(range),
));
dep.set_optional(parser.in_try);
parser.dependencies.push(dep);
Expand Down Expand Up @@ -542,7 +569,7 @@ impl AMDDefineDependencyParserPlugin {
.as_ref()
.map(|name| parser.add_local_module(name.as_str()));

let dep = Box::new(AmdDefineDependency::new(
let dep = Box::new(AMDDefineDependency::new(
(call_expr.span.real_lo(), call_expr.span.real_hi()),
array.map(|expr| span_to_range(expr.span())),
func.map(|expr| span_to_range(expr.span())),
Expand Down Expand Up @@ -578,7 +605,7 @@ impl JavascriptParserPlugin for AMDDefineDependencyParserPlugin {
*/
fn finish(&self, parser: &mut JavascriptParser) -> Option<bool> {
for dep in parser.presentational_dependencies.iter_mut() {
if let Some(define_dep) = dep.as_any_mut().downcast_mut::<AmdDefineDependency>()
if let Some(define_dep) = dep.as_any_mut().downcast_mut::<AMDDefineDependency>()
&& let Some(local_module) = define_dep.get_local_module_mut()
&& parser
.local_modules
Expand Down
Loading

2 comments on commit f6401df

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented on f6401df Jan 24, 2025

Choose a reason for hiding this comment

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

📝 Ecosystem CI detail: Open

suite result
modernjs ❌ failure
rspress ✅ success
rslib ✅ success
rsbuild ✅ success
rsdoctor ✅ success
examples ✅ success
devserver ✅ success
nuxt ✅ success

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented on f6401df Jan 24, 2025

Choose a reason for hiding this comment

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

📝 Benchmark detail: Open

Name Base (2025-01-24 494aaa1) Current Change
10000_big_production-mode_disable-minimize + exec 37.9 s ± 1.01 s 38.8 s ± 1.09 s +2.49 %
10000_development-mode + exec 1.85 s ± 22 ms 1.82 s ± 21 ms -1.60 %
10000_development-mode_hmr + exec 682 ms ± 6 ms 690 ms ± 23 ms +1.18 %
10000_production-mode + exec 2.41 s ± 50 ms 2.39 s ± 107 ms -0.79 %
10000_production-mode_persistent-cold + exec 2.55 s ± 46 ms 2.59 s ± 182 ms +1.44 %
10000_production-mode_persistent-hot + exec 1.79 s ± 147 ms 1.75 s ± 47 ms -2.71 %
arco-pro_development-mode + exec 1.78 s ± 73 ms 1.76 s ± 177 ms -0.98 %
arco-pro_development-mode_hmr + exec 388 ms ± 0.94 ms 389 ms ± 5.2 ms +0.35 %
arco-pro_production-mode + exec 3.77 s ± 100 ms 3.9 s ± 157 ms +3.52 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.88 s ± 132 ms 3.86 s ± 289 ms -0.59 %
arco-pro_production-mode_persistent-cold + exec 3.93 s ± 182 ms 3.96 s ± 252 ms +0.76 %
arco-pro_production-mode_persistent-hot + exec 2.52 s ± 192 ms 2.54 s ± 94 ms +0.79 %
arco-pro_production-mode_traverse-chunk-modules + exec 3.79 s ± 184 ms 3.79 s ± 271 ms -0.08 %
large-dyn-imports_development-mode + exec 2.1 s ± 120 ms 2.08 s ± 42 ms -1.13 %
large-dyn-imports_production-mode + exec 2.14 s ± 51 ms 2.13 s ± 32 ms -0.45 %
threejs_development-mode_10x + exec 1.53 s ± 21 ms 1.53 s ± 47 ms +0.01 %
threejs_development-mode_10x_hmr + exec 777 ms ± 13 ms 787 ms ± 15 ms +1.28 %
threejs_production-mode_10x + exec 5.51 s ± 237 ms 5.45 s ± 386 ms -1.14 %
threejs_production-mode_10x_persistent-cold + exec 5.57 s ± 287 ms 5.65 s ± 438 ms +1.37 %
threejs_production-mode_10x_persistent-hot + exec 4.76 s ± 206 ms 4.77 s ± 276 ms +0.13 %
10000_big_production-mode_disable-minimize + rss memory 8775 MiB ± 358 MiB 8694 MiB ± 28.8 MiB -0.92 %
10000_development-mode + rss memory 643 MiB ± 21 MiB 685 MiB ± 20 MiB +6.57 %
10000_development-mode_hmr + rss memory 1283 MiB ± 196 MiB 1368 MiB ± 121 MiB +6.58 %
10000_production-mode + rss memory 636 MiB ± 20.7 MiB 699 MiB ± 15.9 MiB +9.99 %
10000_production-mode_persistent-cold + rss memory 746 MiB ± 12 MiB 789 MiB ± 43.7 MiB +5.81 %
10000_production-mode_persistent-hot + rss memory 723 MiB ± 22 MiB 767 MiB ± 24.7 MiB +6.03 %
arco-pro_development-mode + rss memory 564 MiB ± 22.8 MiB 626 MiB ± 34.9 MiB +11.06 %
arco-pro_development-mode_hmr + rss memory 660 MiB ± 64.2 MiB 700 MiB ± 51.9 MiB +6.05 %
arco-pro_production-mode + rss memory 737 MiB ± 23.7 MiB 774 MiB ± 17.5 MiB +5.06 %
arco-pro_production-mode_generate-package-json-webpack-plugin + rss memory 737 MiB ± 28.8 MiB 790 MiB ± 39.9 MiB +7.27 %
arco-pro_production-mode_persistent-cold + rss memory 865 MiB ± 47.4 MiB 903 MiB ± 41.8 MiB +4.38 %
arco-pro_production-mode_persistent-hot + rss memory 752 MiB ± 20.5 MiB 811 MiB ± 29.5 MiB +7.81 %
arco-pro_production-mode_traverse-chunk-modules + rss memory 743 MiB ± 32.7 MiB 784 MiB ± 14.1 MiB +5.42 %
large-dyn-imports_development-mode + rss memory 653 MiB ± 4.28 MiB 676 MiB ± 10.6 MiB +3.61 %
large-dyn-imports_production-mode + rss memory 537 MiB ± 6.2 MiB 563 MiB ± 8.84 MiB +4.73 %
threejs_development-mode_10x + rss memory 552 MiB ± 5.19 MiB 586 MiB ± 26.7 MiB +6.14 %
threejs_development-mode_10x_hmr + rss memory 1139 MiB ± 62.6 MiB 1129 MiB ± 185 MiB -0.82 %
threejs_production-mode_10x + rss memory 829 MiB ± 44.9 MiB 862 MiB ± 45.2 MiB +4.05 %
threejs_production-mode_10x_persistent-cold + rss memory 955 MiB ± 63.2 MiB 938 MiB ± 81 MiB -1.75 %
threejs_production-mode_10x_persistent-hot + rss memory 879 MiB ± 22.8 MiB 892 MiB ± 62.1 MiB +1.44 %

Please sign in to comment.