Skip to content

Commit

Permalink
Revert "perf: reuse js dependency" (#9389)
Browse files Browse the repository at this point in the history
Revert "perf: reuse js dependency (#9352)"

This reverts commit 6032d46.
  • Loading branch information
JSerFeng authored Feb 19, 2025
1 parent 3d61889 commit 39f13d9
Show file tree
Hide file tree
Showing 16 changed files with 334 additions and 404 deletions.
36 changes: 20 additions & 16 deletions crates/node_binding/binding.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,9 @@ export declare class ExternalObject<T> {
[K: symbol]: T
}
}
export declare class Dependency {

}
export type JsDependency = Dependency

export declare class EntryDataDto {
get dependencies(): Dependency[]
get includeDependencies(): Dependency[]
get dependencies(): JsDependency[]
get includeDependencies(): JsDependency[]
get options(): EntryOptionsDto
}
export type EntryDataDTO = EntryDataDto
Expand Down Expand Up @@ -174,7 +169,7 @@ export declare class JsContextModuleFactoryAfterResolveData {
set regExp(rawRegExp: RegExp | undefined)
get recursive(): boolean
set recursive(recursive: boolean)
get dependencies(): Dependency[]
get dependencies(): JsDependency[]
}

export declare class JsContextModuleFactoryBeforeResolveData {
Expand Down Expand Up @@ -204,10 +199,19 @@ export declare class JsDependencies {
}

export declare class JsDependenciesBlock {
get dependencies(): Dependency[]
get dependencies(): JsDependency[]
get blocks(): JsDependenciesBlock[]
}

export declare class JsDependency {
get type(): string
get category(): string
get request(): string | undefined
get critical(): boolean
set critical(val: boolean)
get ids(): Array<string> | undefined
}

export declare class JsEntries {
clear(): void
get size(): number
Expand Down Expand Up @@ -240,7 +244,7 @@ export declare class JsModule {
get type(): string
get layer(): string | undefined
get blocks(): JsDependenciesBlock[]
get dependencies(): Dependency[]
get dependencies(): JsDependency[]
size(ty?: string | undefined | null): number
get modules(): JsModule[] | undefined
get useSourceMap(): boolean
Expand All @@ -251,21 +255,21 @@ export declare class JsModule {
}

export declare class JsModuleGraph {
getModule(jsDependency: Dependency): JsModule | null
getResolvedModule(jsDependency: Dependency): JsModule | null
getModule(jsDependency: JsDependency): JsModule | null
getResolvedModule(jsDependency: JsDependency): JsModule | null
getUsedExports(jsModule: JsModule, jsRuntime: string | Array<string>): boolean | Array<string> | null
getIssuer(module: JsModule): JsModule | null
getExportsInfo(module: JsModule): JsExportsInfo
getConnection(dependency: Dependency): JsModuleGraphConnection | null
getConnection(dependency: JsDependency): JsModuleGraphConnection | null
getOutgoingConnections(module: JsModule): JsModuleGraphConnection[]
getIncomingConnections(module: JsModule): JsModuleGraphConnection[]
getParentModule(jsDependency: Dependency): JsModule | null
getParentBlockIndex(jsDependency: Dependency): number
getParentModule(jsDependency: JsDependency): JsModule | null
getParentBlockIndex(jsDependency: JsDependency): number
isAsync(module: JsModule): boolean
}

export declare class JsModuleGraphConnection {
get dependency(): Dependency
get dependency(): JsDependency
get module(): JsModule | null
get resolvedModule(): JsModule | null
get originModule(): JsModule | null
Expand Down
20 changes: 10 additions & 10 deletions crates/node_binding/src/compilation/entries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,32 +189,32 @@ pub struct EntryDataDTO {

#[napi]
impl EntryDataDTO {
#[napi(getter, ts_return_type = "Dependency[]")]
#[napi(getter, ts_return_type = "JsDependency[]")]
pub fn dependencies(&'static self) -> Vec<JsDependencyWrapper> {
let module_graph = self.compilation.get_module_graph();
self
.entry_data
.dependencies
.iter()
.filter_map(|dependency_id| {
module_graph
.dependency_by_id(dependency_id)
.map(|dep| JsDependencyWrapper::from_id(*dep.id(), self.compilation.compiler_id()))
.map(|dependency_id| {
#[allow(clippy::unwrap_used)]
let dep = module_graph.dependency_by_id(dependency_id).unwrap();
JsDependencyWrapper::new(dep.as_ref(), self.compilation.id(), Some(self.compilation))
})
.collect::<Vec<_>>()
}

#[napi(getter, ts_return_type = "Dependency[]")]
#[napi(getter, ts_return_type = "JsDependency[]")]
pub fn include_dependencies(&'static self) -> Vec<JsDependencyWrapper> {
let module_graph = self.compilation.get_module_graph();
self
.entry_data
.include_dependencies
.iter()
.filter_map(|dependency_id| {
module_graph
.dependency_by_id(dependency_id)
.map(|dep| JsDependencyWrapper::from_id(*dep.id(), self.compilation.compiler_id()))
.map(|dependency_id| {
#[allow(clippy::unwrap_used)]
let dep = module_graph.dependency_by_id(dependency_id).unwrap();
JsDependencyWrapper::new(dep.as_ref(), self.compilation.id(), Some(self.compilation))
})
.collect::<Vec<_>>()
}
Expand Down
151 changes: 38 additions & 113 deletions crates/node_binding/src/context_module_factory.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
use napi::{
bindgen_prelude::{
ClassInstance, Either, FromNapiValue, Object, ToNapiValue, TypeName, ValidateNapiValue,
},
Env,
use napi::bindgen_prelude::{
ClassInstance, Either, FromNapiValue, ToNapiValue, TypeName, ValidateNapiValue,
};
use napi_derive::napi;
use rspack_core::{AfterResolveData, BeforeResolveData};
use rspack_regex::RspackRegex;
use swc_core::common::util::take::Take;

use crate::{JsDependency, JsDependencyWrapper};
use crate::JsDependencyWrapper;

#[napi]
pub struct JsContextModuleFactoryBeforeResolveData(Box<BeforeResolveData>);
Expand Down Expand Up @@ -113,118 +109,74 @@ pub type JsContextModuleFactoryBeforeResolveResult =
Either<bool, JsContextModuleFactoryBeforeResolveDataWrapper>;

#[napi]
pub struct JsContextModuleFactoryAfterResolveData {
data: Option<Box<AfterResolveData>>,
js_dependencies: Option<Vec<ClassInstance<'static, JsDependency>>>,
}

impl JsContextModuleFactoryAfterResolveData {
fn as_ref(&self) -> napi::Result<&AfterResolveData> {
match &self.data {
Some(data) => {
Ok(data.as_ref())
},
None => Err(napi::Error::from_reason(
"Unable to access context module factory after resolve data now. The data has been taken on the Rust side."
)),
}
}

fn as_mut(&mut self) -> napi::Result<&mut AfterResolveData> {
match &mut self.data {
Some(data) => {
Ok(data.as_mut())
},
None => Err(napi::Error::from_reason(
"Unable to modify context module factory after resolve data now. The data has been taken on the Rust side."
)),
}
}
}
pub struct JsContextModuleFactoryAfterResolveData(Box<AfterResolveData>);

#[napi]
impl JsContextModuleFactoryAfterResolveData {
#[napi(getter)]
pub fn resource(&self) -> napi::Result<&str> {
let data = self.as_ref()?;
Ok(data.resource.as_str())
pub fn resource(&self) -> &str {
self.0.resource.as_str()
}

#[napi(setter)]
pub fn set_resource(&mut self, resource: String) -> napi::Result<()> {
let data = self.as_mut()?;
data.resource = resource.into();
Ok(())
pub fn set_resource(&mut self, resource: String) {
self.0.resource = resource.into();
}

#[napi(getter)]
pub fn context(&self) -> napi::Result<&str> {
let data = self.as_ref()?;
Ok(&data.context)
pub fn context(&self) -> &str {
&self.0.context
}

#[napi(setter)]
pub fn set_context(&mut self, context: String) -> napi::Result<()> {
let data = self.as_mut()?;
data.context = context;
Ok(())
pub fn set_context(&mut self, context: String) {
self.0.context = context;
}

#[napi(getter)]
pub fn request(&self) -> napi::Result<&str> {
let data = self.as_ref()?;
Ok(&data.request)
pub fn request(&self) -> &str {
&self.0.request
}

#[napi(setter)]
pub fn set_request(&mut self, request: String) -> napi::Result<()> {
let data = self.as_mut()?;
data.request = request;
Ok(())
pub fn set_request(&mut self, request: String) {
self.0.request = request;
}

#[napi(getter, ts_return_type = "RegExp | undefined")]
pub fn reg_exp(&self) -> napi::Result<Either<RspackRegex, ()>> {
let data = self.as_ref()?;
Ok(match &data.reg_exp {
pub fn reg_exp(&self) -> Either<RspackRegex, ()> {
match &self.0.reg_exp {
Some(r) => Either::A(r.clone()),
None => Either::B(()),
})
}
}

#[napi(setter, ts_args_type = "rawRegExp: RegExp | undefined")]
pub fn set_reg_exp(&mut self, raw_reg_exp: Either<RspackRegex, ()>) -> napi::Result<()> {
let data = self.as_mut()?;
data.reg_exp = match raw_reg_exp {
pub fn set_reg_exp(&mut self, raw_reg_exp: Either<RspackRegex, ()>) {
self.0.reg_exp = match raw_reg_exp {
Either::A(regex) => Some(regex),
Either::B(_) => None,
};
Ok(())
}

#[napi(getter)]
pub fn recursive(&self) -> napi::Result<bool> {
let data = self.as_ref()?;
Ok(data.recursive)
pub fn recursive(&self) -> bool {
self.0.recursive
}

#[napi(setter)]
pub fn set_recursive(&mut self, recursive: bool) -> napi::Result<()> {
let data = self.as_mut()?;
data.recursive = recursive;
Ok(())
pub fn set_recursive(&mut self, recursive: bool) {
self.0.recursive = recursive;
}

#[napi(getter, ts_return_type = "Dependency[]")]
pub fn dependencies(&self, env: Env) -> napi::Result<Vec<Object>> {
match &self.js_dependencies {
Some(js_dependencies) => {
Ok(js_dependencies.iter().map(|dep| dep.as_object(&env)).collect())
},
None => Err(napi::Error::from_reason(
"Unable to take dependencies in context module factory after resolve data now. The data has been taken on the Rust side."
)),
}
#[napi(getter, ts_return_type = "JsDependency[]")]
pub fn dependencies(&self) -> Vec<JsDependencyWrapper> {
self
.0
.dependencies
.iter()
.map(|dep| JsDependencyWrapper::new(dep.as_ref(), self.0.compilation_id, None))
.collect::<Vec<_>>()
}
}

Expand All @@ -245,55 +197,28 @@ impl FromNapiValue for JsContextModuleFactoryAfterResolveDataWrapper {
env: napi::sys::napi_env,
napi_val: napi::sys::napi_value,
) -> napi::Result<Self> {
let mut instance =
let instance =
<ClassInstance<JsContextModuleFactoryAfterResolveData> as FromNapiValue>::from_napi_value(
env, napi_val,
)?;
let Some(mut data) = instance.data.take() else {
return Err(napi::Error::from_reason(
"Unable to take context module factory after resolve data now. The data has been taken on the Rust side."
));
};
let Some(js_dependencies) = instance.js_dependencies.take() else {
return Err(napi::Error::from_reason(
"Unable to take dependencies in context module factory after resolve data now. The data has been taken on the Rust side."
));
};

data.dependencies = js_dependencies
.into_iter()
.filter_map(|mut dep| dep.dependency.take())
.collect::<Vec<_>>();

Ok(Self(data))
Ok(Self(instance.0.clone()))
}
}

impl ToNapiValue for JsContextModuleFactoryAfterResolveDataWrapper {
unsafe fn to_napi_value(
env: napi::sys::napi_env,
mut val: Self,
val: Self,
) -> napi::Result<napi::sys::napi_value> {
let dependencies = val.0.dependencies.take();
let compiler_id = val.0.compiler_id;
let js_data = JsContextModuleFactoryAfterResolveData {
data: Some(val.0),
js_dependencies: Some(
dependencies
.into_iter()
.map(|dep| JsDependencyWrapper::from_owned(dep, compiler_id).into_instance(env))
.collect::<napi::Result<Vec<_>>>()?,
),
};
ToNapiValue::to_napi_value(env, js_data)
let js_val = JsContextModuleFactoryAfterResolveData(val.0);
ToNapiValue::to_napi_value(env, js_val)
}
}

impl TypeName for JsContextModuleFactoryAfterResolveDataWrapper {
fn type_name() -> &'static str {
"JsContextModuleFactoryAfterResolveData"
}

fn value_type() -> napi::ValueType {
napi::ValueType::Object
}
Expand Down
Loading

2 comments on commit 39f13d9

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented on 39f13d9 Feb 19, 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 ❌ failure
rsbuild ❌ failure
rsdoctor ❌ failure
examples ✅ success
devserver ✅ success
nuxt ✅ success

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented on 39f13d9 Feb 19, 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-02-19 a81bd20) Current Change
10000_big_production-mode_disable-minimize + exec 36.9 s ± 476 ms 37.5 s ± 987 ms +1.61 %
10000_development-mode + exec 1.76 s ± 20 ms 1.76 s ± 170 ms 0.00 %
10000_development-mode_hmr + exec 672 ms ± 7 ms 682 ms ± 31 ms +1.48 %
10000_production-mode + exec 2.23 s ± 54 ms 2.19 s ± 50 ms -1.89 %
10000_production-mode_persistent-cold + exec 2.37 s ± 98 ms 2.36 s ± 170 ms -0.24 %
10000_production-mode_persistent-hot + exec 1.63 s ± 48 ms 1.6 s ± 59 ms -1.68 %
arco-pro_development-mode + exec 1.76 s ± 60 ms 1.75 s ± 71 ms -0.58 %
arco-pro_development-mode_hmr + exec 386 ms ± 3.5 ms 386 ms ± 3.3 ms -0.00 %
arco-pro_production-mode + exec 3.67 s ± 70 ms 3.5 s ± 249 ms -4.58 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.66 s ± 258 ms 3.57 s ± 234 ms -2.52 %
arco-pro_production-mode_persistent-cold + exec 3.71 s ± 205 ms 3.6 s ± 178 ms -3.01 %
arco-pro_production-mode_persistent-hot + exec 2.3 s ± 79 ms 2.35 s ± 192 ms +2.46 %
arco-pro_production-mode_traverse-chunk-modules + exec 3.56 s ± 131 ms 3.57 s ± 109 ms +0.42 %
large-dyn-imports_development-mode + exec 1.99 s ± 47 ms 1.99 s ± 42 ms +0.02 %
large-dyn-imports_production-mode + exec 2.04 s ± 31 ms 2.05 s ± 113 ms +0.54 %
threejs_development-mode_10x + exec 1.52 s ± 7.6 ms 1.54 s ± 20 ms +1.40 %
threejs_development-mode_10x_hmr + exec 774 ms ± 5.1 ms 826 ms ± 35 ms +6.72 %
threejs_production-mode_10x + exec 5.15 s ± 62 ms 5.21 s ± 235 ms +1.07 %
threejs_production-mode_10x_persistent-cold + exec 5.2 s ± 92 ms 5.22 s ± 116 ms +0.40 %
threejs_production-mode_10x_persistent-hot + exec 4.44 s ± 201 ms 4.42 s ± 67 ms -0.57 %
10000_big_production-mode_disable-minimize + rss memory 8666 MiB ± 41 MiB 8713 MiB ± 91.7 MiB +0.54 %
10000_development-mode + rss memory 647 MiB ± 23.7 MiB 663 MiB ± 10.6 MiB +2.55 %
10000_development-mode_hmr + rss memory 1326 MiB ± 157 MiB 1269 MiB ± 187 MiB -4.33 %
10000_production-mode + rss memory 617 MiB ± 8.92 MiB 653 MiB ± 29.4 MiB +5.95 %
10000_production-mode_persistent-cold + rss memory 724 MiB ± 23 MiB 756 MiB ± 29.3 MiB +4.42 %
10000_production-mode_persistent-hot + rss memory 690 MiB ± 28.4 MiB 728 MiB ± 27 MiB +5.64 %
arco-pro_development-mode + rss memory 566 MiB ± 42.3 MiB 592 MiB ± 32.4 MiB +4.47 %
arco-pro_development-mode_hmr + rss memory 654 MiB ± 47.4 MiB 689 MiB ± 49.2 MiB +5.32 %
arco-pro_production-mode + rss memory 716 MiB ± 21.3 MiB 739 MiB ± 43.4 MiB +3.10 %
arco-pro_production-mode_generate-package-json-webpack-plugin + rss memory 729 MiB ± 38.1 MiB 749 MiB ± 34.1 MiB +2.78 %
arco-pro_production-mode_persistent-cold + rss memory 798 MiB ± 61 MiB 812 MiB ± 40.2 MiB +1.78 %
arco-pro_production-mode_persistent-hot + rss memory 646 MiB ± 20.1 MiB 702 MiB ± 22.4 MiB +8.75 %
arco-pro_production-mode_traverse-chunk-modules + rss memory 724 MiB ± 42.3 MiB 730 MiB ± 23.3 MiB +0.92 %
large-dyn-imports_development-mode + rss memory 636 MiB ± 1.7 MiB 656 MiB ± 11.7 MiB +3.09 %
large-dyn-imports_production-mode + rss memory 519 MiB ± 4.18 MiB 544 MiB ± 5.01 MiB +4.79 %
threejs_development-mode_10x + rss memory 550 MiB ± 14.7 MiB 549 MiB ± 15.1 MiB -0.14 %
threejs_development-mode_10x_hmr + rss memory 1138 MiB ± 114 MiB 1179 MiB ± 114 MiB +3.60 %
threejs_production-mode_10x + rss memory 837 MiB ± 31.6 MiB 833 MiB ± 54.2 MiB -0.48 %
threejs_production-mode_10x_persistent-cold + rss memory 930 MiB ± 31.9 MiB 919 MiB ± 81.1 MiB -1.21 %
threejs_production-mode_10x_persistent-hot + rss memory 794 MiB ± 26.3 MiB 808 MiB ± 32.7 MiB +1.80 %

Threshold exceeded: ["threejs_development-mode_10x_hmr + exec"]

Please sign in to comment.