Skip to content

Commit

Permalink
fix: passively rebuild modules imported by importModule (#8595)
Browse files Browse the repository at this point in the history
* fix

* finish

* add test

* update tests

* ifx
  • Loading branch information
CPunisher authored Dec 7, 2024
1 parent f0462ea commit 8646ecf
Show file tree
Hide file tree
Showing 16 changed files with 234 additions and 8 deletions.
2 changes: 1 addition & 1 deletion crates/rspack_core/src/compiler/make/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
mod cutout;
pub mod cutout;
pub mod repair;

use rspack_collections::IdentifierSet;
Expand Down
54 changes: 47 additions & 7 deletions crates/rspack_core/src/compiler/module_executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use dashmap::{mapref::entry::Entry, DashSet};
pub use execute::ExecuteModuleId;
pub use execute::ExecutedRuntimeModule;
use rspack_collections::{Identifier, IdentifierDashMap, IdentifierDashSet};
use rustc_hash::FxHashSet as HashSet;
use tokio::sync::{
mpsc::{unbounded_channel, UnboundedSender},
oneshot,
Expand All @@ -19,17 +20,26 @@ use self::{
execute::{ExecuteModuleResult, ExecuteTask},
overwrite::OverwriteTask,
};
use super::make::{repair::MakeTaskContext, update_module_graph, MakeArtifact, MakeParam};
use super::make::cutout::Cutout;
use super::make::repair::repair;
use super::make::{repair::MakeTaskContext, MakeArtifact, MakeParam};
use crate::cache::new_cache;
use crate::incremental::Mutation;
use crate::{
task_loop::run_task_loop_with_event, Compilation, CompilationAsset, Context, Dependency,
DependencyId, LoaderImportDependency, PublicPath,
};

#[derive(Debug)]
struct DepStatus {
id: DependencyId,
should_update: bool,
}

#[derive(Debug, Default)]
pub struct ModuleExecutor {
request_dep_map: DashMap<(String, Option<String>), DependencyId>,
cutout: Cutout,
request_dep_map: DashMap<(String, Option<String>), DepStatus>,
pub make_artifact: MakeArtifact,

event_sender: Option<UnboundedSender<Event>>,
Expand Down Expand Up @@ -65,7 +75,20 @@ impl ModuleExecutor {
make_artifact.diagnostics = Default::default();
make_artifact.has_module_graph_change = false;

make_artifact = update_module_graph(compilation, make_artifact, params)
// Modules imported by `importModule` are passively loaded.
let mut build_dependencies = self.cutout.cutout_artifact(&mut make_artifact, params);
let mut build_dependencies_id = build_dependencies
.iter()
.map(|(id, _)| *id)
.collect::<HashSet<_>>();
for mut dep_status in self.request_dep_map.iter_mut() {
if build_dependencies_id.contains(&dep_status.id) {
dep_status.should_update = true;
build_dependencies_id.remove(&dep_status.id);
}
}
build_dependencies.retain(|dep| build_dependencies_id.contains(&dep.0));
make_artifact = repair(compilation, make_artifact, build_dependencies)
.await
.unwrap_or_default();

Expand Down Expand Up @@ -116,6 +139,9 @@ impl ModuleExecutor {
panic!("receive make artifact failed");
}

let cutout = std::mem::take(&mut self.cutout);
cutout.fix_artifact(&mut self.make_artifact);

let module_assets = std::mem::take(&mut self.module_assets);
for (original_module_identifier, files) in module_assets {
let assets = compilation
Expand Down Expand Up @@ -205,12 +231,26 @@ impl ModuleExecutor {
original_module_context.unwrap_or(Context::from("")),
);
let dep_id = *dep.id();
v.insert(dep_id);
v.insert(DepStatus {
id: dep_id,
should_update: false,
});
(ExecuteParam::Entry(Box::new(dep), layer.clone()), dep_id)
}
Entry::Occupied(v) => {
let dep_id = *v.get();
(ExecuteParam::DependencyId(dep_id), dep_id)
Entry::Occupied(mut v) => {
let dep_status = v.get_mut();
let dep_id = dep_status.id;
if dep_status.should_update {
let dep = LoaderImportDependency::new_with_id(
dep_id,
request.clone(),
original_module_context.unwrap_or(Context::from("")),
);
dep_status.should_update = false;
(ExecuteParam::Entry(Box::new(dep), layer.clone()), dep_id)
} else {
(ExecuteParam::DependencyId(dep_id), dep_id)
}
}
};

Expand Down
8 changes: 8 additions & 0 deletions crates/rspack_core/src/dependency/loader_import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ impl LoaderImportDependency {
id: DependencyId::new(),
}
}

pub fn new_with_id(id: DependencyId, request: String, context: Context) -> Self {
Self {
request,
context,
id,
}
}
}

impl AsDependencyTemplate for LoaderImportDependency {}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Case import-module-1: Step 0

## Changed Files


## Asset Files
- Bundle: bundle.js

## Manifest


## Update
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# Case import-module-1: Step 1

## Changed Files
- a.js

## Asset Files
- Bundle: bundle.js
- Manifest: main.LAST_HASH.hot-update.json, size: 28
- Update: main.LAST_HASH.hot-update.js, size: 532

## Manifest

### main.LAST_HASH.hot-update.json

```json
{"c":["main"],"r":[],"m":[]}
```


## Update


### main.LAST_HASH.hot-update.js

#### Changed Modules
- ./loader.js!./a.js

#### Changed Runtime Modules
- webpack/runtime/get_full_hash

#### Changed Content
```js
"use strict";
self["webpackHotUpdate"]('main', {
"./loader.js!./a.js": (function (__unused_webpack_module, __webpack_exports__, __webpack_require__) {
__webpack_require__.r(__webpack_exports__);
__webpack_require__.d(__webpack_exports__, {
"default": function() { return __WEBPACK_DEFAULT_EXPORT__; }
});
/* ESM default export */ const __WEBPACK_DEFAULT_EXPORT__ = (2);


}),

},function(__webpack_require__) {
// webpack/runtime/get_full_hash
(() => {
__webpack_require__.h = function () {
return "CURRENT_HASH";
};

})();

}
);
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default 1;
---
export default 2;
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import a from "./loader.js!./a";

it("module and its loader-referencing module should update in right order", (done) => {
expect(a).toBe(1);
NEXT(
require('../../update')(done, true, () => {
expect(a).toBe(2);
done();
}),
);
});

module.hot.accept('./loader.js!./a');
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module.exports = async function (content) {
if (!content.includes("2")) {
await this.importModule("./loader2.js!./a.js");
}
return content;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module.exports = async function (content) {
if (content.includes("2")) {
throw "should not throw";
}
return content;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Case import-module-2: Step 0

## Changed Files


## Asset Files
- Bundle: bundle.js

## Manifest


## Update
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# Case import-module-2: Step 1

## Changed Files
- import_module_sub.js

## Asset Files
- Bundle: bundle.js
- Manifest: main.LAST_HASH.hot-update.json, size: 28
- Update: main.LAST_HASH.hot-update.js, size: 257

## Manifest

### main.LAST_HASH.hot-update.json

```json
{"c":["main"],"r":[],"m":[]}
```


## Update


### main.LAST_HASH.hot-update.js

#### Changed Modules
- ./loader.js!./a.js

#### Changed Runtime Modules
- webpack/runtime/get_full_hash

#### Changed Content
```js
self["webpackHotUpdate"]('main', {
"./loader.js!./a.js": (function (module) {
module.exports = 3;

}),

},function(__webpack_require__) {
// webpack/runtime/get_full_hash
(() => {
__webpack_require__.h = function () {
return "CURRENT_HASH";
};

})();

}
);
```
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = 1;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
const loader = require("./import_module_sub.js");
module.exports = loader;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = 2;
---
module.exports = 3;
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
it("module and its loader-referencing module should update in right order", done => {
expect(require("./loader.js!./a")).toBe(2);
NEXT(
require("../../update")(done, true, () => {
expect(require("./loader.js!./a")).toBe(3);
done();
})
);
});
module.hot.accept("./loader.js!./a");
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
module.exports = async function (content) {
let res = await this.importModule("./import_module_root.js");
return content.replace("1", res);
};

2 comments on commit 8646ecf

@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-12-07 f0462ea) Current Change
10000_big_production-mode_disable-minimize + exec 37.1 s ± 367 ms 36.9 s ± 203 ms -0.36 %
10000_development-mode + exec 1.8 s ± 33 ms 1.78 s ± 22 ms -1.18 %
10000_development-mode_hmr + exec 643 ms ± 13 ms 637 ms ± 1.9 ms -1.01 %
10000_production-mode + exec 2.34 s ± 15 ms 2.35 s ± 23 ms +0.40 %
arco-pro_development-mode + exec 1.76 s ± 42 ms 1.75 s ± 46 ms -0.64 %
arco-pro_development-mode_hmr + exec 424 ms ± 1.7 ms 424 ms ± 2 ms +0.02 %
arco-pro_production-mode + exec 3.14 s ± 87 ms 3.12 s ± 66 ms -0.54 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.18 s ± 82 ms 3.15 s ± 79 ms -1.03 %
threejs_development-mode_10x + exec 1.62 s ± 23 ms 1.62 s ± 18 ms +0.14 %
threejs_development-mode_10x_hmr + exec 780 ms ± 6.7 ms 785 ms ± 11 ms +0.66 %
threejs_production-mode_10x + exec 4.89 s ± 31 ms 4.88 s ± 25 ms -0.21 %
10000_big_production-mode_disable-minimize + rss memory 9862 MiB ± 54.4 MiB 9884 MiB ± 36.7 MiB +0.23 %
10000_development-mode + rss memory 819 MiB ± 24.8 MiB 829 MiB ± 61.8 MiB +1.23 %
10000_development-mode_hmr + rss memory 1986 MiB ± 369 MiB 1733 MiB ± 246 MiB -12.74 %
10000_production-mode + rss memory 697 MiB ± 42.1 MiB 694 MiB ± 33.7 MiB -0.35 %
arco-pro_development-mode + rss memory 706 MiB ± 37.5 MiB 711 MiB ± 22.8 MiB +0.80 %
arco-pro_development-mode_hmr + rss memory 890 MiB ± 77 MiB 892 MiB ± 81.7 MiB +0.19 %
arco-pro_production-mode + rss memory 809 MiB ± 70.3 MiB 828 MiB ± 61.8 MiB +2.39 %
arco-pro_production-mode_generate-package-json-webpack-plugin + rss memory 813 MiB ± 63.6 MiB 835 MiB ± 52.3 MiB +2.73 %
threejs_development-mode_10x + rss memory 764 MiB ± 48.3 MiB 774 MiB ± 62.3 MiB +1.34 %
threejs_development-mode_10x_hmr + rss memory 1670 MiB ± 213 MiB 1690 MiB ± 261 MiB +1.22 %
threejs_production-mode_10x + rss memory 1078 MiB ± 78.3 MiB 1130 MiB ± 86.9 MiB +4.83 %

@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 ✅ success
_selftest ✅ success
rsdoctor ✅ success
rspress ✅ success
rslib ✅ success
rsbuild ✅ success
examples ✅ success
devserver ✅ success
nuxt ✅ success

Please sign in to comment.