Skip to content

Commit

Permalink
feat: support module.size function in cacheGroups.[i].test (#7482)
Browse files Browse the repository at this point in the history
* ModuleDTOSingleton

* feat: use ModuleDTO in JsCacheGroupTestCtx

* feat: JsCompilationSingleton

* refactor: rename to JsCompilationWrapper

* feat: clean COMPILATION_INSTANCE_REFS and MODULE_INSTANCE_REFS

* rename: ModuleDTOWrapper

* fix: ts type for ModuleDTOWrapper

* fix: use add_env_cleanup_hook

* test: add case

* chore: update snapshot

* fix

* refactor: code in Rspack

* chore: add SAFETY comment

* fix: cargo lint

* fix: problem in review

* feat: cleanup_last_compilation

* fix: remove unused files

* chore: rmove dispose method

* fix: cargo lint error
  • Loading branch information
SyMind authored Aug 9, 2024
1 parent 2e52ab4 commit ba260c5
Show file tree
Hide file tree
Showing 13 changed files with 283 additions and 112 deletions.
9 changes: 5 additions & 4 deletions crates/node_binding/binding.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ export class ModuleDto {
get type(): string
get layer(): string | undefined
get blocks(): Array<DependenciesBlockDto>
size(ty?: string | undefined | null): number
}
export type ModuleDTO = ModuleDto

Expand Down Expand Up @@ -351,6 +352,10 @@ export interface JsBuildTimeExecutionOption {
baseUri?: string
}

export interface JsCacheGroupTestCtx {
module: ModuleDTO
}

export interface JsChunk {
__inner_ukey: number
__inner_groups: Array<number>
Expand Down Expand Up @@ -993,10 +998,6 @@ export interface RawCacheGroupOptions {
usedExports?: boolean
}

export interface RawCacheGroupTestCtx {
module: JsModule
}

export interface RawCacheOptions {
type: string
maxGenerations: number
Expand Down
9 changes: 8 additions & 1 deletion crates/node_binding/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use std::sync::Mutex;
use compiler::{Compiler, CompilerState, CompilerStateGuard};
use napi::bindgen_prelude::*;
use rspack_binding_options::BuiltinPlugin;
use rspack_core::PluginExt;
use rspack_core::{Compilation, PluginExt};
use rspack_error::Diagnostic;
use rspack_fs_node::{AsyncNodeWritableFileSystem, ThreadsafeNodeFS};

Expand Down Expand Up @@ -164,6 +164,9 @@ impl Rspack {
// SAFETY: The mutable reference to `Compiler` is exclusive. It's guaranteed by the running state guard.
Ok(unsafe { s.compiler.as_mut().get_unchecked_mut() })
})?;

self.cleanup_last_compilation(&compiler.compilation);

// SAFETY:
// 1. `Compiler` is pinned and stored on the heap.
// 2. `JsReference` (NAPI internal mechanism) keeps `Compiler` alive until its instance getting garbage collected.
Expand All @@ -172,6 +175,10 @@ impl Rspack {
_guard,
)
}

fn cleanup_last_compilation(&self, compilation: &Compilation) {
JsCompilationWrapper::cleanup(compilation.id());
}
}

fn concurrent_compiler_error() -> Error {
Expand Down
88 changes: 25 additions & 63 deletions crates/node_binding/src/plugins/interceptor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use napi::{
use rspack_binding_values::{
CompatSource, JsAdditionalTreeRuntimeRequirementsArg, JsAdditionalTreeRuntimeRequirementsResult,
JsAfterResolveData, JsAfterResolveOutput, JsAssetEmittedArgs, JsBeforeResolveArgs,
JsBeforeResolveOutput, JsChunk, JsChunkAssetArgs, JsCompilation,
JsBeforeResolveOutput, JsChunk, JsChunkAssetArgs, JsCompilationWrapper,
JsContextModuleFactoryAfterResolveData, JsContextModuleFactoryAfterResolveResult,
JsContextModuleFactoryBeforeResolveData, JsContextModuleFactoryBeforeResolveResult, JsCreateData,
JsExecuteModuleArg, JsFactorizeArgs, JsFactorizeOutput, JsModule,
Expand Down Expand Up @@ -350,23 +350,23 @@ pub struct RegisterJsTaps {
#[napi(
ts_type = "(stages: Array<number>) => Array<{ function: ((arg: JsCompilation) => void); stage: number; }>"
)]
pub register_compiler_this_compilation_taps: RegisterFunction<JsCompilation, ()>,
pub register_compiler_this_compilation_taps: RegisterFunction<JsCompilationWrapper, ()>,
#[napi(
ts_type = "(stages: Array<number>) => Array<{ function: ((arg: JsCompilation) => void); stage: number; }>"
)]
pub register_compiler_compilation_taps: RegisterFunction<JsCompilation, ()>,
pub register_compiler_compilation_taps: RegisterFunction<JsCompilationWrapper, ()>,
#[napi(
ts_type = "(stages: Array<number>) => Array<{ function: ((arg: JsCompilation) => Promise<void>); stage: number; }>"
)]
pub register_compiler_make_taps: RegisterFunction<JsCompilation, Promise<()>>,
pub register_compiler_make_taps: RegisterFunction<JsCompilationWrapper, Promise<()>>,
#[napi(
ts_type = "(stages: Array<number>) => Array<{ function: ((arg: JsCompilation) => void); stage: number; }>"
)]
pub register_compiler_finish_make_taps: RegisterFunction<JsCompilation, Promise<()>>,
pub register_compiler_finish_make_taps: RegisterFunction<JsCompilationWrapper, Promise<()>>,
#[napi(
ts_type = "(stages: Array<number>) => Array<{ function: ((arg: JsCompilation) => boolean | undefined); stage: number; }>"
)]
pub register_compiler_should_emit_taps: RegisterFunction<JsCompilation, Option<bool>>,
pub register_compiler_should_emit_taps: RegisterFunction<JsCompilationWrapper, Option<bool>>,
#[napi(
ts_type = "(stages: Array<number>) => Array<{ function: (() => Promise<void>); stage: number; }>"
)]
Expand Down Expand Up @@ -410,7 +410,7 @@ pub struct RegisterJsTaps {
#[napi(
ts_type = "(stages: Array<number>) => Array<{ function: ((arg: JsCompilation) => Promise<void>); stage: number; }>"
)]
pub register_compilation_finish_modules_taps: RegisterFunction<JsCompilation, Promise<()>>,
pub register_compilation_finish_modules_taps: RegisterFunction<JsCompilationWrapper, Promise<()>>,
#[napi(
ts_type = "(stages: Array<number>) => Array<{ function: (() => boolean | undefined); stage: number; }>"
)]
Expand All @@ -436,11 +436,11 @@ pub struct RegisterJsTaps {
#[napi(
ts_type = "(stages: Array<number>) => Array<{ function: ((arg: JsCompilation) => Promise<void>); stage: number; }>"
)]
pub register_compilation_process_assets_taps: RegisterFunction<JsCompilation, Promise<()>>,
pub register_compilation_process_assets_taps: RegisterFunction<JsCompilationWrapper, Promise<()>>,
#[napi(
ts_type = "(stages: Array<number>) => Array<{ function: ((arg: JsCompilation) => void); stage: number; }>"
)]
pub register_compilation_after_process_assets_taps: RegisterFunction<JsCompilation, ()>,
pub register_compilation_after_process_assets_taps: RegisterFunction<JsCompilationWrapper, ()>,
#[napi(ts_type = "(stages: Array<number>) => Array<{ function: (() => void); stage: number; }>")]
pub register_compilation_seal_taps: RegisterFunction<(), ()>,
#[napi(
Expand Down Expand Up @@ -500,39 +500,39 @@ pub struct RegisterJsTaps {
/* Compiler Hooks */
define_register!(
RegisterCompilerThisCompilationTaps,
tap = CompilerThisCompilationTap<JsCompilation, ()> @ CompilerThisCompilationHook,
tap = CompilerThisCompilationTap<JsCompilationWrapper, ()> @ CompilerThisCompilationHook,
cache = false,
sync = false,
kind = RegisterJsTapKind::CompilerThisCompilation,
skip = false,
);
define_register!(
RegisterCompilerCompilationTaps,
tap = CompilerCompilationTap<JsCompilation, ()> @ CompilerCompilationHook,
tap = CompilerCompilationTap<JsCompilationWrapper, ()> @ CompilerCompilationHook,
cache = false,
sync = false,
kind = RegisterJsTapKind::CompilerCompilation,
skip = true,
);
define_register!(
RegisterCompilerMakeTaps,
tap = CompilerMakeTap<JsCompilation, Promise<()>> @ CompilerMakeHook,
tap = CompilerMakeTap<JsCompilationWrapper, Promise<()>> @ CompilerMakeHook,
cache = false,
sync = false,
kind = RegisterJsTapKind::CompilerMake,
skip = true,
);
define_register!(
RegisterCompilerFinishMakeTaps,
tap = CompilerFinishMakeTap<JsCompilation, Promise<()>> @ CompilerFinishMakeHook,
tap = CompilerFinishMakeTap<JsCompilationWrapper, Promise<()>> @ CompilerFinishMakeHook,
cache = false,
sync = false,
kind = RegisterJsTapKind::CompilerFinishMake,
skip = true,
);
define_register!(
RegisterCompilerShouldEmitTaps,
tap = CompilerShouldEmitTap<JsCompilation, Option<bool>> @ CompilerShouldEmitHook,
tap = CompilerShouldEmitTap<JsCompilationWrapper, Option<bool>> @ CompilerShouldEmitHook,
cache = false,
sync = false,
kind = RegisterJsTapKind::CompilerShouldEmit,
Expand Down Expand Up @@ -598,7 +598,7 @@ define_register!(
);
define_register!(
RegisterCompilationFinishModulesTaps,
tap = CompilationFinishModulesTap<JsCompilation, Promise<()>> @ CompilationFinishModulesHook,
tap = CompilationFinishModulesTap<JsCompilationWrapper, Promise<()>> @ CompilationFinishModulesHook,
cache = false,
sync = false,
kind = RegisterJsTapKind::CompilationFinishModules,
Expand Down Expand Up @@ -670,15 +670,15 @@ define_register!(
);
define_register!(
RegisterCompilationProcessAssetsTaps,
tap = CompilationProcessAssetsTap<JsCompilation, Promise<()>> @ CompilationProcessAssetsHook,
tap = CompilationProcessAssetsTap<JsCompilationWrapper, Promise<()>> @ CompilationProcessAssetsHook,
cache = false,
sync = false,
kind = RegisterJsTapKind::CompilationProcessAssets,
skip = true,
);
define_register!(
RegisterCompilationAfterProcessAssetsTaps,
tap = CompilationAfterProcessAssetsTap<JsCompilation, ()> @ CompilationAfterProcessAssetsHook,
tap = CompilationAfterProcessAssetsTap<JsCompilationWrapper, ()> @ CompilationAfterProcessAssetsHook,
cache = false,
sync = false,
kind = RegisterJsTapKind::CompilationAfterProcessAssets,
Expand Down Expand Up @@ -786,11 +786,7 @@ impl CompilerThisCompilation for CompilerThisCompilationTap {
compilation: &mut Compilation,
_: &mut CompilationParams,
) -> rspack_error::Result<()> {
// SAFETY:
// 1. `Compiler` is stored on the heap and pinned in binding crate.
// 2. `Compilation` outlives `JsCompilation` and `Compiler` outlives `Compilation`.
// 3. `JsCompilation` was replaced everytime a new `Compilation` was created before getting accessed.
let compilation = unsafe { JsCompilation::from_compilation(compilation) };
let compilation = JsCompilationWrapper::new(compilation);
self.function.call_with_sync(compilation).await
}

Expand All @@ -806,11 +802,7 @@ impl CompilerCompilation for CompilerCompilationTap {
compilation: &mut Compilation,
_: &mut CompilationParams,
) -> rspack_error::Result<()> {
// SAFETY:
// 1. `Compiler` is stored on the heap and pinned in binding crate.
// 2. `Compilation` outlives `JsCompilation` and `Compiler` outlives `Compilation`.
// 3. `JsCompilation` was replaced everytime a new `Compilation` was created before getting accessed.
let compilation = unsafe { JsCompilation::from_compilation(compilation) };
let compilation = JsCompilationWrapper::new(compilation);
self.function.call_with_sync(compilation).await
}

Expand All @@ -822,12 +814,7 @@ impl CompilerCompilation for CompilerCompilationTap {
#[async_trait]
impl CompilerMake for CompilerMakeTap {
async fn run(&self, compilation: &mut Compilation) -> rspack_error::Result<()> {
// SAFETY:
// 1. `Compiler` is stored on the heap and pinned in binding crate.
// 2. `Compilation` outlives `JsCompilation` and `Compiler` outlives `Compilation`.
// 3. `JsCompilation` was replaced everytime a new `Compilation` was created before getting accessed.
let compilation = unsafe { JsCompilation::from_compilation(compilation) };

let compilation = JsCompilationWrapper::new(compilation);
self.function.call_with_promise(compilation).await
}

Expand All @@ -839,12 +826,7 @@ impl CompilerMake for CompilerMakeTap {
#[async_trait]
impl CompilerFinishMake for CompilerFinishMakeTap {
async fn run(&self, compilation: &mut Compilation) -> rspack_error::Result<()> {
// SAFETY:
// 1. `Compiler` is stored on the heap and pinned in binding crate.
// 2. `Compilation` outlives `JsCompilation` and `Compiler` outlives `Compilation`.
// 3. `JsCompilation` was replaced everytime a new `Compilation` was created before getting accessed.
let compilation = unsafe { JsCompilation::from_compilation(compilation) };

let compilation = JsCompilationWrapper::new(compilation);
self.function.call_with_promise(compilation).await
}

Expand All @@ -856,12 +838,7 @@ impl CompilerFinishMake for CompilerFinishMakeTap {
#[async_trait]
impl CompilerShouldEmit for CompilerShouldEmitTap {
async fn run(&self, compilation: &mut Compilation) -> rspack_error::Result<Option<bool>> {
// SAFETY:
// 1. `Compiler` is stored on the heap and pinned in binding crate.
// 2. `Compilation` outlives `JsCompilation` and `Compiler` outlives `Compilation`.
// 3. `JsCompilation` was replaced everytime a new `Compilation` was created before getting accessed.
let compilation = unsafe { JsCompilation::from_compilation(compilation) };

let compilation = JsCompilationWrapper::new(compilation);
self.function.call_with_sync(compilation).await
}

Expand Down Expand Up @@ -982,12 +959,7 @@ impl CompilationExecuteModule for CompilationExecuteModuleTap {
#[async_trait]
impl CompilationFinishModules for CompilationFinishModulesTap {
async fn run(&self, compilation: &mut Compilation) -> rspack_error::Result<()> {
// SAFETY:
// 1. `Compiler` is stored on the heap and pinned in binding crate.
// 2. `Compilation` outlives `JsCompilation` and `Compiler` outlives `Compilation`.
// 3. `JsCompilation` was replaced everytime a new `Compilation` was created before getting accessed.
let compilation = unsafe { JsCompilation::from_compilation(compilation) };

let compilation = JsCompilationWrapper::new(compilation);
self.function.call_with_promise(compilation).await
}

Expand Down Expand Up @@ -1149,12 +1121,7 @@ impl CompilationChunkAsset for CompilationChunkAssetTap {
#[async_trait]
impl CompilationProcessAssets for CompilationProcessAssetsTap {
async fn run(&self, compilation: &mut Compilation) -> rspack_error::Result<()> {
// SAFETY:
// 1. `Compiler` is stored on the heap and pinned in binding crate.
// 2. `Compilation` outlives `JsCompilation` and `Compiler` outlives `Compilation`.
// 3. `JsCompilation` was replaced everytime a new `Compilation` was created before getting accessed.
let compilation = unsafe { JsCompilation::from_compilation(compilation) };

let compilation = JsCompilationWrapper::new(compilation);
self.function.call_with_promise(compilation).await
}

Expand All @@ -1166,12 +1133,7 @@ impl CompilationProcessAssets for CompilationProcessAssetsTap {
#[async_trait]
impl CompilationAfterProcessAssets for CompilationAfterProcessAssetsTap {
async fn run(&self, compilation: &mut Compilation) -> rspack_error::Result<()> {
// SAFETY:
// 1. `Compiler` is stored on the heap and pinned in binding crate.
// 2. `Compilation` outlives `JsCompilation` and `Compiler` outlives `Compilation`.
// 3. `JsCompilation` was replaced everytime a new `Compilation` was created before getting accessed.
let compilation = unsafe { JsCompilation::from_compilation(compilation) };

let compilation = JsCompilationWrapper::new(compilation);
self.function.call_with_sync(compilation).await
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,28 +1,35 @@
use std::sync::Arc;

use napi::bindgen_prelude::Either3;
use napi::bindgen_prelude::{Either3, FromNapiValue};
use napi_derive::napi;
use rspack_binding_values::{JsModule, ToJsModule};
use rspack_binding_values::ModuleDTOWrapper;
use rspack_napi::regexp::{JsRegExp, JsRegExpExt};
use rspack_napi::threadsafe_function::ThreadsafeFunction;
use rspack_plugin_split_chunks::{CacheGroupTest, CacheGroupTestFnCtx};
use tokio::runtime::Handle;

pub(super) type RawCacheGroupTest =
Either3<String, JsRegExp, ThreadsafeFunction<RawCacheGroupTestCtx, Option<bool>>>;
Either3<String, JsRegExp, ThreadsafeFunction<JsCacheGroupTestCtx, Option<bool>>>;

#[napi(object)]
pub struct RawCacheGroupTestCtx {
pub module: JsModule,
#[napi(object, object_from_js = false)]
pub struct JsCacheGroupTestCtx {
#[napi(ts_type = "ModuleDTO")]
pub module: ModuleDTOWrapper,
}

impl<'a> From<CacheGroupTestFnCtx<'a>> for RawCacheGroupTestCtx {
impl FromNapiValue for JsCacheGroupTestCtx {
unsafe fn from_napi_value(
_env: napi::sys::napi_env,
_napi_val: napi::sys::napi_value,
) -> napi::Result<Self> {
unreachable!()
}
}

impl<'a> From<CacheGroupTestFnCtx<'a>> for JsCacheGroupTestCtx {
fn from(value: CacheGroupTestFnCtx<'a>) -> Self {
RawCacheGroupTestCtx {
module: value
.module
.to_js_module()
.expect("should convert js module success"),
JsCacheGroupTestCtx {
module: ModuleDTOWrapper::new(value.module.identifier(), value.compilation),
}
}
}
Expand Down
Loading

2 comments on commit ba260c5

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Ran ecosystem CI: Open

suite result
modernjs ❌ failure
_selftest ✅ success
nx ❌ failure
rspress ❌ failure
rsbuild ❌ failure
examples ❌ failure

@rspack-bot
Copy link

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 (2024-08-08 2bd905d) Current Change
10000_development-mode + exec 2.32 s ± 27 ms 2.31 s ± 30 ms -0.59 %
10000_development-mode_hmr + exec 701 ms ± 7.3 ms 695 ms ± 11 ms -0.81 %
10000_production-mode + exec 2.85 s ± 23 ms 2.88 s ± 47 ms +1.24 %
arco-pro_development-mode + exec 1.89 s ± 75 ms 1.86 s ± 42 ms -1.24 %
arco-pro_development-mode_hmr + exec 434 ms ± 2.4 ms 434 ms ± 2.2 ms 0.00 %
arco-pro_production-mode + exec 3.52 s ± 286 ms 3.45 s ± 67 ms -1.94 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.54 s ± 235 ms 3.5 s ± 78 ms -1.31 %
threejs_development-mode_10x + exec 1.72 s ± 15 ms 1.71 s ± 17 ms -0.58 %
threejs_development-mode_10x_hmr + exec 826 ms ± 9.5 ms 815 ms ± 12 ms -1.28 %
threejs_production-mode_10x + exec 5.53 s ± 29 ms 5.52 s ± 21 ms -0.24 %

Please sign in to comment.