Skip to content

Commit 5d4ba56

Browse files
sokrasokra
committed
Allow scope hoisting to process modules in multiple chunks
1 parent d6a7594 commit 5d4ba56

File tree

16 files changed

+254
-131
lines changed

16 files changed

+254
-131
lines changed

‎lib/Compilation.js‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -906,6 +906,15 @@ class Compilation extends Tapable {
906906
chunks
907907
}];
908908

909+
const filterFn = dep => {
910+
if(chunks.has(dep.chunk)) return false;
911+
for(const chunk of chunks) {
912+
if(chunk.containsModule(dep.module))
913+
return false;
914+
}
915+
return true;
916+
};
917+
909918
while(queue2.length) {
910919
const queueItem = queue2.pop();
911920
chunk = queueItem.chunk;
@@ -914,14 +923,7 @@ class Compilation extends Tapable {
914923
const deps = chunkDependencies.get(chunk);
915924
if(!deps) continue;
916925

917-
const depsFiltered = deps.filter(dep => {
918-
if(chunks.has(dep.chunk)) return false;
919-
for(const chunk of chunks) {
920-
if(chunk.containsModule(dep.module))
921-
return false;
922-
}
923-
return true;
924-
});
926+
const depsFiltered = deps.filter(filterFn);
925927

926928
for(let i = 0; i < depsFiltered.length; i++) {
927929
const dep = depsFiltered[i];

‎lib/Module.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,10 +127,28 @@ class Module extends DependenciesBlock {
127127
return Array.from(this._chunks, fn);
128128
}
129129

130+
getChunks() {
131+
return Array.from(this._chunks);
132+
}
133+
130134
getNumberOfChunks() {
131135
return this._chunks.size;
132136
}
133137

138+
hasEqualsChunks(otherModule) {
139+
if(this._chunks.size !== otherModule._chunks.size) return false;
140+
this._ensureChunksSortedByDebugId();
141+
otherModule._ensureChunksSortedByDebugId();
142+
const a = this._chunks[Symbol.iterator]();
143+
const b = otherModule._chunks[Symbol.iterator]();
144+
while(true) { // eslint-disable-line
145+
const aItem = a.next();
146+
const bItem = b.next();
147+
if(aItem.done) return true;
148+
if(aItem.value !== bItem.value) return false;
149+
}
150+
}
151+
134152
_ensureChunksSorted() {
135153
if(this._chunksIsSorted) return;
136154
this._chunks = new Set(Array.from(this._chunks).sort(byId));

‎lib/optimize/ModuleConcatenationPlugin.js‎

Lines changed: 124 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ class ModuleConcatenationPlugin {
2323
});
2424
const bailoutReasonMap = new Map();
2525

26-
function setBailoutReason(module, reason) {
26+
function setBailoutReason(module, prefix, reason) {
2727
bailoutReasonMap.set(module, reason);
28-
module.optimizationBailout.push(reason);
28+
module.optimizationBailout.push(typeof reason === "function" ? (rs) => `${prefix}: ${reason(rs)}` : `${prefix}: ${reason}`);
2929
}
3030

3131
function getBailoutReason(module, requestShortener) {
@@ -35,141 +35,135 @@ class ModuleConcatenationPlugin {
3535
}
3636

3737
compilation.plugin("optimize-chunk-modules", (chunks, modules) => {
38-
chunks.forEach(chunk => {
39-
const relevantModules = [];
40-
const possibleInners = new Set();
41-
for(const module of chunk.modulesIterable) {
42-
// Only harmony modules are valid for optimization
43-
if(!module.meta || !module.meta.harmonyModule) {
44-
continue;
45-
}
38+
const relevantModules = [];
39+
const possibleInners = new Set();
40+
for(const module of modules) {
41+
// Only harmony modules are valid for optimization
42+
if(!module.meta || !module.meta.harmonyModule) {
43+
continue;
44+
}
4645

47-
// Module must not be in other chunks
48-
// TODO add an option to allow module to be in other entry points
49-
if(module.getNumberOfChunks() !== 1) {
50-
setBailoutReason(module, "ModuleConcatenation: module is in multiple chunks");
51-
continue;
52-
}
46+
// Because of variable renaming we can't use modules with eval
47+
if(module.meta && module.meta.hasEval) {
48+
setBailoutReason(module, "ModuleConcatenation", "eval is used in the module");
49+
continue;
50+
}
5351

54-
// Because of variable renaming we can't use modules with eval
55-
if(module.meta && module.meta.hasEval) {
56-
setBailoutReason(module, "ModuleConcatenation: eval is used in the module");
57-
continue;
58-
}
52+
relevantModules.push(module);
5953

60-
relevantModules.push(module);
54+
// Module must not be the entry points
55+
if(module.getChunks().some(chunk => chunk.entryModule === module)) {
56+
setBailoutReason(module, "ModuleConcatenation (inner)", "module is an entrypoint");
57+
continue;
58+
}
6159

62-
// Module must not be the entry points
63-
if(chunk.entryModule === module) {
64-
setBailoutReason(module, "ModuleConcatenation (inner): module is an entrypoint");
65-
continue;
66-
}
60+
// Exports must be known (and not dynamic)
61+
if(!Array.isArray(module.providedExports)) {
62+
setBailoutReason(module, "ModuleConcatenation (inner)", "exports are not known");
63+
continue;
64+
}
6765

68-
// Exports must be known (and not dynamic)
69-
if(!Array.isArray(module.providedExports)) {
70-
setBailoutReason(module, "ModuleConcatenation (inner): exports are not known");
71-
continue;
72-
}
66+
// Using dependency variables is not possible as this wraps the code in a function
67+
if(module.variables.length > 0) {
68+
setBailoutReason(module, "ModuleConcatenation (inner)", "dependency variables are used (i. e. ProvidePlugin)");
69+
continue;
70+
}
7371

74-
// Using dependency variables is not possible as this wraps the code in a function
75-
if(module.variables.length > 0) {
76-
setBailoutReason(module, "ModuleConcatenation (inner): dependency variables are used (i. e. ProvidePlugin)");
77-
continue;
78-
}
72+
// Module must only be used by Harmony Imports
73+
const nonHarmonyReasons = module.reasons.filter(reason => !(reason.dependency instanceof HarmonyImportDependency));
74+
if(nonHarmonyReasons.length > 0) {
75+
const importingModules = new Set(nonHarmonyReasons.map(r => r.module));
76+
setBailoutReason(module, "ModuleConcatenation (inner)", (requestShortener) => {
77+
const names = Array.from(importingModules).map(m => m.readableIdentifier(requestShortener));
78+
return `module is used with non-harmony imports from ${names.join(", ")}`;
79+
});
80+
continue;
81+
}
7982

80-
// Module must only be used by Harmony Imports
81-
const nonHarmonyReasons = module.reasons.filter(reason => !(reason.dependency instanceof HarmonyImportDependency));
82-
if(nonHarmonyReasons.length > 0) {
83-
const importingModules = new Set(nonHarmonyReasons.map(r => r.module));
84-
setBailoutReason(module, (requestShortener) => {
85-
const names = Array.from(importingModules).map(m => m.readableIdentifier(requestShortener));
86-
return `ModuleConcatenation (inner): module is used with non-harmony imports from ${names.join(", ")}`;
87-
});
88-
continue;
83+
possibleInners.add(module);
84+
}
85+
// sort by depth
86+
// modules with lower depth are more likly suited as roots
87+
// this improves performance, because modules already selected as inner are skipped
88+
relevantModules.sort((a, b) => {
89+
return a.depth - b.depth;
90+
});
91+
const concatConfigurations = [];
92+
const usedAsInner = new Set();
93+
for(const currentRoot of relevantModules) {
94+
// when used by another configuration as inner:
95+
// the other configuration is better and we can skip this one
96+
if(usedAsInner.has(currentRoot))
97+
continue;
98+
99+
// create a configuration with the root
100+
const currentConfiguration = new ConcatConfiguration(currentRoot);
101+
102+
// cache failures to add modules
103+
const failureCache = new Map();
104+
105+
// try to add all imports
106+
for(const imp of this.getImports(currentRoot)) {
107+
const problem = this.tryToAdd(currentConfiguration, imp, possibleInners, failureCache);
108+
if(problem) {
109+
failureCache.set(imp, problem);
110+
currentConfiguration.addWarning(imp, problem);
89111
}
90-
91-
possibleInners.add(module);
92112
}
93-
// sort by depth
94-
// modules with lower depth are more likly suited as roots
95-
// this improves performance, because modules already selected as inner are skipped
96-
relevantModules.sort((a, b) => {
97-
return a.depth - b.depth;
98-
});
99-
const concatConfigurations = [];
100-
const usedAsInner = new Set();
101-
for(const currentRoot of relevantModules) {
102-
// when used by another configuration as inner:
103-
// the other configuration is better and we can skip this one
104-
if(usedAsInner.has(currentRoot))
105-
continue;
106-
107-
// create a configuration with the root
108-
const currentConfiguration = new ConcatConfiguration(currentRoot);
109-
110-
// cache failures to add modules
111-
const failureCache = new Map();
112-
113-
// try to add all imports
114-
for(const imp of this.getImports(currentRoot)) {
115-
const problem = this.tryToAdd(currentConfiguration, imp, possibleInners, failureCache);
116-
if(problem) {
117-
failureCache.set(imp, problem);
118-
currentConfiguration.addWarning(imp, problem);
119-
}
120-
}
121-
if(!currentConfiguration.isEmpty()) {
122-
concatConfigurations.push(currentConfiguration);
123-
for(const module of currentConfiguration.modules) {
124-
if(module !== currentConfiguration.rootModule)
125-
usedAsInner.add(module);
126-
}
113+
if(!currentConfiguration.isEmpty()) {
114+
concatConfigurations.push(currentConfiguration);
115+
for(const module of currentConfiguration.modules) {
116+
if(module !== currentConfiguration.rootModule)
117+
usedAsInner.add(module);
127118
}
128119
}
129-
// HACK: Sort configurations by length and start with the longest one
130-
// to get the biggers groups possible. Used modules are marked with usedModules
131-
// TODO: Allow to reuse existing configuration while trying to add dependencies.
132-
// This would improve performance. O(n^2) -> O(n)
133-
concatConfigurations.sort((a, b) => {
134-
return b.modules.size - a.modules.size;
135-
});
136-
const usedModules = new Set();
137-
for(const concatConfiguration of concatConfigurations) {
138-
if(usedModules.has(concatConfiguration.rootModule))
139-
continue;
140-
const orderedModules = new Set();
141-
this.addInOrder(concatConfiguration.rootModule, concatConfiguration.modules, orderedModules);
142-
const newModule = new ConcatenatedModule(concatConfiguration.rootModule, Array.from(orderedModules));
143-
for(const warning of concatConfiguration.warnings) {
144-
newModule.optimizationBailout.push((requestShortener) => {
145-
const reason = getBailoutReason(warning[0], requestShortener);
146-
const reasonPrefix = reason ? `: ${reason}` : "";
147-
if(warning[0] === warning[1])
148-
return `ModuleConcatenation: Cannot concat with ${warning[0].readableIdentifier(requestShortener)}${reasonPrefix}`;
149-
else
150-
return `ModuleConcatenation: Cannot concat with ${warning[0].readableIdentifier(requestShortener)} because of ${warning[1].readableIdentifier(requestShortener)}${reasonPrefix}`;
151-
});
152-
}
153-
for(const m of orderedModules) {
154-
usedModules.add(m);
155-
chunk.removeModule(m);
156-
}
120+
}
121+
// HACK: Sort configurations by length and start with the longest one
122+
// to get the biggers groups possible. Used modules are marked with usedModules
123+
// TODO: Allow to reuse existing configuration while trying to add dependencies.
124+
// This would improve performance. O(n^2) -> O(n)
125+
concatConfigurations.sort((a, b) => {
126+
return b.modules.size - a.modules.size;
127+
});
128+
const usedModules = new Set();
129+
for(const concatConfiguration of concatConfigurations) {
130+
if(usedModules.has(concatConfiguration.rootModule))
131+
continue;
132+
const orderedModules = new Set();
133+
this.addInOrder(concatConfiguration.rootModule, concatConfiguration.modules, orderedModules);
134+
const newModule = new ConcatenatedModule(concatConfiguration.rootModule, Array.from(orderedModules));
135+
for(const warning of concatConfiguration.warnings) {
136+
newModule.optimizationBailout.push((requestShortener) => {
137+
const reason = getBailoutReason(warning[0], requestShortener);
138+
const reasonPrefix = reason ? `: ${reason}` : "";
139+
if(warning[0] === warning[1])
140+
return `ModuleConcatenation: Cannot concat with ${warning[0].readableIdentifier(requestShortener)}${reasonPrefix}`;
141+
else
142+
return `ModuleConcatenation: Cannot concat with ${warning[0].readableIdentifier(requestShortener)} because of ${warning[1].readableIdentifier(requestShortener)}${reasonPrefix}`;
143+
});
144+
}
145+
const chunks = concatConfiguration.rootModule.getChunks();
146+
for(const m of orderedModules) {
147+
usedModules.add(m);
148+
chunks.forEach(chunk => chunk.removeModule(m));
149+
}
150+
chunks.forEach(chunk => {
157151
chunk.addModule(newModule);
158-
compilation.modules.push(newModule);
159152
if(chunk.entryModule === concatConfiguration.rootModule)
160153
chunk.entryModule = newModule;
161-
newModule.reasons.forEach(reason => reason.dependency.module = newModule);
162-
newModule.dependencies.forEach(dep => {
163-
if(dep.module) {
164-
dep.module.reasons.forEach(reason => {
165-
if(reason.dependency === dep)
166-
reason.module = newModule;
167-
});
168-
}
169-
});
170-
}
171-
compilation.modules = compilation.modules.filter(m => !usedModules.has(m));
172-
});
154+
});
155+
compilation.modules.push(newModule);
156+
newModule.reasons.forEach(reason => reason.dependency.module = newModule);
157+
newModule.dependencies.forEach(dep => {
158+
if(dep.module) {
159+
dep.module.reasons.forEach(reason => {
160+
if(reason.dependency === dep)
161+
reason.module = newModule;
162+
});
163+
}
164+
});
165+
}
166+
compilation.modules = compilation.modules.filter(m => !usedModules.has(m));
173167
});
174168
});
175169
}
@@ -208,6 +202,13 @@ class ModuleConcatenationPlugin {
208202

209203
// Not possible to add?
210204
if(!possibleModules.has(module)) {
205+
failureCache.set(module, module); // cache failures for performance
206+
return module;
207+
}
208+
209+
// module must be in the same chunks
210+
if(!config.rootModule.hasEqualsChunks(module)) {
211+
failureCache.set(module, module); // cache failures for performance
211212
return module;
212213
}
213214

‎test/statsCases/scope-hoisting-multi/common.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export { default } from "./common2";

‎test/statsCases/scope-hoisting-multi/common2.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export default "common";

‎test/statsCases/scope-hoisting-multi/common_lazy.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export default "common";

‎test/statsCases/scope-hoisting-multi/common_lazy_shared.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export default "common";

‎test/statsCases/scope-hoisting-multi/expected.txt‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
Hash: 731069e082cf620521ced84ccc10b5c4fc7695db
2+
Child
3+
Hash: 731069e082cf620521ce
4+
Time: Xms
5+
[0] (webpack)/test/statsCases/scope-hoisting-multi/common_lazy_shared.js 25 bytes {0} {1} {2} [built]
6+
[1] (webpack)/test/statsCases/scope-hoisting-multi/vendor.js 25 bytes {5} [built]
7+
[2] (webpack)/test/statsCases/scope-hoisting-multi/common.js 37 bytes {3} {4} [built]
8+
[3] (webpack)/test/statsCases/scope-hoisting-multi/common2.js 25 bytes {3} {4} [built]
9+
[4] (webpack)/test/statsCases/scope-hoisting-multi/common_lazy.js 25 bytes {1} {2} [built]
10+
[5] (webpack)/test/statsCases/scope-hoisting-multi/lazy_shared.js 31 bytes {0} [built]
11+
[6] (webpack)/test/statsCases/scope-hoisting-multi/first.js 207 bytes {3} [built]
12+
[7] (webpack)/test/statsCases/scope-hoisting-multi/module_first.js 31 bytes {3} [built]
13+
[8] (webpack)/test/statsCases/scope-hoisting-multi/lazy_first.js 55 bytes {2} [built]
14+
[9] (webpack)/test/statsCases/scope-hoisting-multi/second.js 177 bytes {4} [built]
15+
[10] (webpack)/test/statsCases/scope-hoisting-multi/lazy_second.js 55 bytes {1} [built]
16+
Child
17+
Hash: d84ccc10b5c4fc7695db
18+
Time: Xms
19+
[0] (webpack)/test/statsCases/scope-hoisting-multi/common_lazy_shared.js 25 bytes {0} {1} {2} [built]
20+
[1] (webpack)/test/statsCases/scope-hoisting-multi/vendor.js 25 bytes {5} [built]
21+
ModuleConcatenation (inner): module is an entrypoint
22+
[2] (webpack)/test/statsCases/scope-hoisting-multi/common.js + 1 modules 62 bytes {4} {3} [built]
23+
[3] (webpack)/test/statsCases/scope-hoisting-multi/common_lazy.js 25 bytes {1} {2} [built]
24+
[4] (webpack)/test/statsCases/scope-hoisting-multi/first.js + 1 modules 238 bytes {4} [built]
25+
ModuleConcatenation (inner): module is an entrypoint
26+
ModuleConcatenation: Cannot concat with (webpack)/test/statsCases/scope-hoisting-multi/vendor.js: module is an entrypoint
27+
ModuleConcatenation: Cannot concat with (webpack)/test/statsCases/scope-hoisting-multi/common.js
28+
[5] (webpack)/test/statsCases/scope-hoisting-multi/lazy_shared.js 31 bytes {0} [built]
29+
ModuleConcatenation (inner): module is used with non-harmony imports from (webpack)/test/statsCases/scope-hoisting-multi/first.js, (webpack)/test/statsCases/scope-hoisting-multi/second.js
30+
[6] (webpack)/test/statsCases/scope-hoisting-multi/second.js 177 bytes {3} [built]
31+
ModuleConcatenation (inner): module is an entrypoint
32+
[7] (webpack)/test/statsCases/scope-hoisting-multi/lazy_second.js 55 bytes {1} [built]
33+
ModuleConcatenation (inner): module is used with non-harmony imports from (webpack)/test/statsCases/scope-hoisting-multi/second.js
34+
[8] (webpack)/test/statsCases/scope-hoisting-multi/lazy_first.js 55 bytes {2} [built]
35+
ModuleConcatenation (inner): module is used with non-harmony imports from (webpack)/test/statsCases/scope-hoisting-multi/first.js

‎test/statsCases/scope-hoisting-multi/first.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import v from "./vendor";
2+
import c from "./common";
3+
import x from "./module_first";
4+
5+
import(/* webpackChunkName: "lazy_first" */"./lazy_first");
6+
import(/* webpackChunkName: "lazy_shared" */"./lazy_shared");

‎test/statsCases/scope-hoisting-multi/lazy_first.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
import "./common_lazy";
2+
import "./common_lazy_shared";

0 commit comments

Comments
 (0)