Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor chunking algorithm #2575

Merged
merged 23 commits into from Dec 11, 2018
Merged

Refactor chunking algorithm #2575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
61e29b3
Split up misc tests
lukastaegert Nov 11, 2018
9f921ef
Create basic bundle information test
lukastaegert Nov 11, 2018
9ae9bd7
* Make sure "isEntry" is only true for entry facades
lukastaegert Nov 12, 2018
73474b2
* Mark dynamic entry points as such and separate this from actual ent…
lukastaegert Nov 15, 2018
4bdd438
Add entryModuleIds to both static and dynamic entry points
lukastaegert Nov 16, 2018
a5ca4c3
Add name to output
lukastaegert Nov 16, 2018
c4f9a18
Separate execution order from chunk colouring
lukastaegert Nov 17, 2018
4434d92
Prefer named export and do not mix for better IDE support
lukastaegert Nov 18, 2018
3a52ba6
Move chunk colouring behind tree-shaking and add more tests
lukastaegert Nov 20, 2018
1d2557b
Use dynamic import tree-shaking information to optimize chunks
lukastaegert Nov 26, 2018
55118bc
Create proper facades for dynamic imports if necessary that are actually
lukastaegert Nov 27, 2018
32aa5f4
As with normal imports, also use single quotes for dynamic imports
lukastaegert Nov 27, 2018
022fd6f
Test we provide the right chunk information for dynamic facades
lukastaegert Nov 27, 2018
1ae85d5
Move facadeChunk property to modules
lukastaegert Nov 28, 2018
f7723f3
Refactor entry export generation to prepare for multiple entry modules
lukastaegert Nov 28, 2018
c819a4f
Make entryModuleIds a Set on each module
lukastaegert Nov 29, 2018
7956bc7
Support manual chunks with multiple facades
lukastaegert Nov 30, 2018
e14d90f
Handle name conflicts between dynamic entries in manual chunks
lukastaegert Nov 30, 2018
dc675cd
Simplify tracing
lukastaegert Nov 30, 2018
7ef2af5
A chunk may only ever be facade for a single module to simplify the l…
lukastaegert Dec 1, 2018
27c4d04
Add information about dynamically imported chunks to bundle
lukastaegert Dec 2, 2018
a9290da
Make "optimizeImports" an experimental option to reflect that the logic
lukastaegert Dec 2, 2018
ae2d424
Make sure tree-shaken dynamic imports do not lead to the creation of
lukastaegert Dec 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
274 changes: 141 additions & 133 deletions src/Chunk.ts
View file
Open in desktop

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions src/ExternalModule.ts
View file
Open in desktop
Expand Up @@ -17,7 +17,7 @@ export default class ExternalModule {
renormalizeRenderPath = false;
isExternal = true;
isEntryPoint = false;
name: string;
variableName: string;
mostCommonSuggestion: number = 0;
nameSuggestions: { [name: string]: number };
reexported: boolean = false;
Expand All @@ -30,7 +30,7 @@ export default class ExternalModule {
this.execIndex = Infinity;

const parts = id.split(/[\\/]/);
this.name = makeLegal(parts.pop());
this.variableName = makeLegal(parts.pop());

this.nameSuggestions = Object.create(null);
this.declarations = Object.create(null);
Expand Down Expand Up @@ -58,7 +58,7 @@ export default class ExternalModule {

if (this.nameSuggestions[name] > this.mostCommonSuggestion) {
this.mostCommonSuggestion = this.nameSuggestions[name];
this.name = name;
this.variableName = name;
}
}

Expand Down
176 changes: 76 additions & 100 deletions src/Graph.ts
View file
Open in desktop
Expand Up @@ -23,9 +23,10 @@ import {
Watcher
} from './rollup/types';
import { finaliseAsset } from './utils/assetHooks';
import { assignChunkColouringHashes } from './utils/chunkColouring';
import { Uint8ArrayToHexString } from './utils/entryHashing';
import error from './utils/error';
import { analyzeModuleExecution, sortByExecutionOrder } from './utils/execution-order';
import { error } from './utils/error';
import { analyseModuleExecution, sortByExecutionOrder } from './utils/executionOrder';
import { isRelative, resolve } from './utils/path';
import { createPluginDriver, PluginDriver } from './utils/pluginDriver';
import relativeId, { getAliasName } from './utils/relativeId';
Expand All @@ -51,11 +52,11 @@ export default class Graph {
context: string;
externalModules: ExternalModule[] = [];
getModuleContext: (id: string) => string;
hasLoaders: boolean;
isPureExternalModule: (id: string) => boolean;
moduleById = new Map<string, Module | ExternalModule>();
assetsById = new Map<string, Asset>();
modules: Module[] = [];
needsTreeshakingPass: boolean = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you perhaps explain what this refactoring fixed in the process if there was an underlying fix with this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually there was though I no longer know the precise situation. Before, each module had its own needsTreeshakingPass. Now if a variable is declared in module 'a' but included by module 'b', it could happen that the flag would be set for module 'a' instead of module 'b'. If this module was already processed but no other modules request a new pass, it could happen that no new pass was triggered. Having a shared flag for all modules makes much more sense and fixes this kind of issue.
BTW as part of the refactorings of the other PR, I could get rid of requestTreeshakingPass on the AST context entirely and keep this logic to the module level.

onwarn: WarningHandler;
deoptimizationTracker: EntityPathTracker;
scope: GlobalScope;
Expand Down Expand Up @@ -154,7 +155,7 @@ export default class Graph {
this.shimMissingExports = options.shimMissingExports;

this.scope = new GlobalScope();
// TODO strictly speaking, this only applies with non-ES6, non-default-only bundles
// Strictly speaking, this only applies with non-ES6, non-default-only bundles
for (const name of ['module', 'exports', '_interopDefault']) {
this.scope.findVariable(name); // creates global variable as side-effect
}
Expand Down Expand Up @@ -262,18 +263,15 @@ export default class Graph {

includeMarked(modules: Module[]) {
if (this.treeshake) {
let needsTreeshakingPass,
treeshakingPass = 1;
let treeshakingPass = 1;
do {
timeStart(`treeshaking pass ${treeshakingPass}`, 3);
needsTreeshakingPass = false;
this.needsTreeshakingPass = false;
for (const module of modules) {
if (module.include()) {
needsTreeshakingPass = true;
}
if (module.isExecuted) module.include();
}
timeEnd(`treeshaking pass ${treeshakingPass++}`, 3);
} while (needsTreeshakingPass);
} while (this.needsTreeshakingPass);
} else {
// Necessary to properly replace namespace imports
for (const module of modules) module.includeAllInBundle();
Expand Down Expand Up @@ -371,17 +369,7 @@ export default class Graph {

this.link();

const {
orderedModules,
dynamicImports,
dynamicImportAliases,
cyclePaths
} = analyzeModuleExecution(
entryModules,
!preserveModules && !inlineDynamicImports,
inlineDynamicImports,
manualChunkModules
);
const { orderedModules, cyclePaths } = analyseModuleExecution(entryModules);
for (const cyclePath of cyclePaths) {
this.warn({
code: 'CIRCULAR_DEPENDENCY',
Expand All @@ -390,41 +378,18 @@ export default class Graph {
});
}

if (entryModuleAliases) {
for (let i = entryModules.length - 1; i >= 0; i--) {
entryModules[i].chunkAlias = entryModuleAliases[i];
}
}
timeEnd('analyse dependency graph', 2);

// Phase 3 – marking. We include all statements that should be included
timeStart('mark included statements', 2);

if (inlineDynamicImports) {
const entryModule = entryModules[0];
if (entryModules.length > 1)
throw new Error(
'Internal Error: can only inline dynamic imports for single-file builds.'
);
for (const dynamicImportModule of dynamicImports) {
if (entryModule !== dynamicImportModule) dynamicImportModule.markPublicExports();
dynamicImportModule.getOrCreateNamespace().include();
}
} else {
for (let i = 0; i < dynamicImports.length; i++) {
const dynamicImportModule = dynamicImports[i];
if (entryModules.indexOf(dynamicImportModule) === -1) {
entryModules.push(dynamicImportModule);
if (!dynamicImportModule.chunkAlias)
dynamicImportModule.chunkAlias = dynamicImportAliases[i];
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Great to see this all moved out of here.

}

timeEnd('analyse dependency graph', 2);

// Phase 3 – marking. We include all statements that should be included
timeStart('mark included statements', 2);

for (const entryModule of entryModules) entryModule.markPublicExports();

// only include statements that should appear in the bundle
for (const entryModule of entryModules) entryModule.includeAllExports();
this.includeMarked(orderedModules);

// check for unused external imports
Expand All @@ -436,15 +401,27 @@ export default class Graph {
// entry point graph colouring, before generating the import and export facades
timeStart('generate chunks', 2);

if (!preserveModules && !inlineDynamicImports) {
assignChunkColouringHashes(entryModules, manualChunkModules);
}

if (entryModuleAliases) {
for (let i = entryModules.length - 1; i >= 0; i--) {
entryModules[i].chunkAlias = entryModuleAliases[i];
}
}

// TODO: there is one special edge case unhandled here and that is that any module
// exposed as an unresolvable export * (to a graph external export *,
// either as a namespace import reexported or top-level export *)
// should be made to be its own entry point module before chunking
let chunks: Chunk[] = [];
if (preserveModules) {
for (const module of orderedModules) {
const chunk = new Chunk(this, [module]);
if (module.isEntryPoint || !chunk.isEmpty) chunk.entryModule = module;
const chunk = new Chunk(this, [module], inlineDynamicImports);
if (module.isEntryPoint || !chunk.isEmpty) {
chunk.entryModules = [module];
}
chunks.push(chunk);
}
} else {
Expand All @@ -462,7 +439,7 @@ export default class Graph {
for (const entryHashSum in chunkModules) {
const chunkModulesOrdered = chunkModules[entryHashSum];
sortByExecutionOrder(chunkModulesOrdered);
const chunk = new Chunk(this, chunkModulesOrdered);
const chunk = new Chunk(this, chunkModulesOrdered, inlineDynamicImports);
chunks.push(chunk);
}
}
Expand All @@ -474,30 +451,35 @@ export default class Graph {
}

// filter out empty dependencies
chunks = chunks.filter(chunk => !chunk.isEmpty || chunk.entryModule || chunk.isManualChunk);
chunks = chunks.filter(
chunk => !chunk.isEmpty || chunk.entryModules.length > 0 || chunk.isManualChunk
);

// then go over and ensure all entry chunks export their variables
for (const chunk of chunks) {
if (preserveModules || chunk.entryModule) {
chunk.populateEntryExports(preserveModules);
if (preserveModules || chunk.entryModules.length > 0) {
chunk.generateEntryExportsOrMarkAsTainted();
}
}

// create entry point facades for entry module chunks that have tainted exports
const facades = [];
if (!preserveModules) {
for (const entryModule of entryModules) {
if (!entryModule.chunk.isEntryModuleFacade) {
const entryPointFacade = new Chunk(this, []);
entryPointFacade.linkFacade(entryModule);
chunks.push(entryPointFacade);
for (const chunk of chunks) {
for (const entryModule of chunk.entryModules) {
if (chunk.facadeModule !== entryModule) {
const entryPointFacade = new Chunk(this, [], inlineDynamicImports);
entryPointFacade.turnIntoFacade(entryModule);
Copy link
Contributor

Choose a reason for hiding this comment

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

Nicer name!

facades.push(entryPointFacade);
}
}
}
}

timeEnd('generate chunks', 2);

this.finished = true;
return chunks;
return chunks.concat(facades);
}
);
}
Expand Down Expand Up @@ -605,47 +587,41 @@ export default class Graph {
}

private fetchAllDependencies(module: Module) {
// resolve and fetch dynamic imports where possible
const fetchDynamicImportsPromise = Promise.all(
module.getDynamicImportExpressions().map((dynamicImportExpression, index) => {
return Promise.resolve(
this.pluginDriver.hookFirst('resolveDynamicImport', [dynamicImportExpression, module.id])
).then(replacement => {
if (!replacement) {
module.dynamicImportResolutions[index] = {
alias: undefined,
resolution: undefined
};
return;
}
const alias = getAliasName(
replacement,
typeof dynamicImportExpression === 'string' ? dynamicImportExpression : undefined
);
if (typeof dynamicImportExpression !== 'string') {
module.dynamicImportResolutions[index] = { alias, resolution: replacement };
} else if (this.isExternal(replacement, module.id, true)) {
let externalModule;
if (!this.moduleById.has(replacement)) {
externalModule = new ExternalModule({
graph: this,
id: replacement
});
this.externalModules.push(externalModule);
this.moduleById.set(replacement, module);
module.getDynamicImportExpressions().map((dynamicImportExpression, index) =>
this.pluginDriver
.hookFirst('resolveDynamicImport', [dynamicImportExpression, module.id])
.then(replacement => {
if (!replacement) return;
const dynamicImport = module.dynamicImports[index];
dynamicImport.alias = getAliasName(
replacement,
typeof dynamicImportExpression === 'string' ? dynamicImportExpression : undefined
);
if (typeof dynamicImportExpression !== 'string') {
dynamicImport.resolution = replacement;
} else if (this.isExternal(replacement, module.id, true)) {
let externalModule;
if (!this.moduleById.has(replacement)) {
externalModule = new ExternalModule({
graph: this,
id: replacement
});
this.externalModules.push(externalModule);
this.moduleById.set(replacement, module);
} else {
externalModule = <ExternalModule>this.moduleById.get(replacement);
}
dynamicImport.resolution = externalModule;
externalModule.exportsNamespace = true;
} else {
externalModule = <ExternalModule>this.moduleById.get(replacement);
return this.fetchModule(replacement, module.id).then(depModule => {
dynamicImport.resolution = depModule;
});
}
module.dynamicImportResolutions[index] = { alias, resolution: externalModule };
externalModule.exportsNamespace = true;
} else {
return this.fetchModule(replacement, module.id).then(depModule => {
module.dynamicImportResolutions[index] = { alias, resolution: depModule };
});
}
});
})
).then(() => {});
})
)
);
fetchDynamicImportsPromise.catch(() => {});

return Promise.all(
Expand Down