Skip to content

Commit 7836357

Browse files
lukastaegertlukastaegert
authored
Improve performance of chunk naming collision check (#4643)
* Improve performance of chunk naming collision check * Improve coverage
1 parent 71d20c9 commit 7836357

10 files changed

Lines changed: 105 additions & 53 deletions

File tree

‎src/Bundle.ts‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,9 @@ import type {
88
NormalizedOutputOptions,
99
OutputAsset,
1010
OutputBundle,
11-
OutputBundleWithPlaceholders,
1211
OutputChunk,
1312
WarningHandler
1413
} from './rollup/types';
15-
import { FILE_PLACEHOLDER } from './utils/FileEmitter';
1614
import type { PluginDriver } from './utils/PluginDriver';
1715
import { type Addons, createAddons } from './utils/addons';
1816
import { getChunkAssignments } from './utils/chunkAssignment';
@@ -26,6 +24,11 @@ import {
2624
} from './utils/error';
2725
import { sortByExecutionOrder } from './utils/executionOrder';
2826
import { type GenerateCodeSnippets, getGenerateCodeSnippets } from './utils/generateCodeSnippets';
27+
import {
28+
FILE_PLACEHOLDER,
29+
getOutputBundle,
30+
OutputBundleWithPlaceholders
31+
} from './utils/outputBundle';
2932
import { basename, isAbsolute } from './utils/path';
3033
import { timeEnd, timeStart } from './utils/timers';
3134

@@ -43,7 +46,8 @@ export default class Bundle {
4346

4447
async generate(isWrite: boolean): Promise<OutputBundle> {
4548
timeStart('GENERATE', 1);
46-
const outputBundle: OutputBundleWithPlaceholders = Object.create(null);
49+
const outputBundleBase: OutputBundle = Object.create(null);
50+
const outputBundle = getOutputBundle(outputBundleBase);
4751
this.pluginDriver.setOutputBundle(outputBundle, this.outputOptions, this.facadeChunkByModule);
4852
try {
4953
await this.pluginDriver.hookParallel('renderStart', [this.outputOptions, this.inputOptions]);
@@ -78,23 +82,23 @@ export default class Bundle {
7882
this.finaliseAssets(outputBundle);
7983

8084
timeEnd('GENERATE', 1);
81-
return outputBundle as OutputBundle;
85+
return outputBundleBase;
8286
}
8387

8488
private async addFinalizedChunksToBundle(
8589
chunks: readonly Chunk[],
8690
inputBase: string,
8791
addons: Addons,
88-
outputBundle: OutputBundleWithPlaceholders,
92+
bundle: OutputBundleWithPlaceholders,
8993
snippets: GenerateCodeSnippets
9094
): Promise<void> {
91-
this.assignChunkIds(chunks, inputBase, addons, outputBundle);
95+
this.assignChunkIds(chunks, inputBase, addons, bundle);
9296
for (const chunk of chunks) {
93-
outputBundle[chunk.id!] = chunk.getChunkInfoWithFileNames() as OutputChunk;
97+
bundle[chunk.id!] = chunk.getChunkInfoWithFileNames() as OutputChunk;
9498
}
9599
await Promise.all(
96100
chunks.map(async chunk => {
97-
const outputChunk = outputBundle[chunk.id!] as OutputChunk;
101+
const outputChunk = bundle[chunk.id!] as OutputChunk;
98102
Object.assign(
99103
outputChunk,
100104
await chunk.render(this.outputOptions, addons, outputChunk, snippets)

‎src/Chunk.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import {
5151
isDefaultAProperty,
5252
namespaceInteropHelpersByInteropType
5353
} from './utils/interopHelpers';
54+
import { OutputBundleWithPlaceholders } from './utils/outputBundle';
5455
import { dirname, extname, isAbsolute, normalize, resolve } from './utils/path';
5556
import relativeId, { getAliasName, getImportPath } from './utils/relativeId';
5657
import renderChunk from './utils/renderChunk';
@@ -410,7 +411,7 @@ export default class Chunk {
410411
generateId(
411412
addons: Addons,
412413
options: NormalizedOutputOptions,
413-
existingNames: Record<string, unknown>,
414+
bundle: OutputBundleWithPlaceholders,
414415
includeHash: boolean
415416
): string {
416417
if (this.fileName !== null) {
@@ -428,19 +429,19 @@ export default class Chunk {
428429
format: () => options.format,
429430
hash: () =>
430431
includeHash
431-
? this.computeContentHashWithDependencies(addons, options, existingNames)
432+
? this.computeContentHashWithDependencies(addons, options, bundle)
432433
: '[hash]',
433434
name: () => this.getChunkName()
434435
}
435436
),
436-
existingNames
437+
bundle
437438
);
438439
}
439440

440441
generateIdPreserveModules(
441442
preserveModulesRelativeDir: string,
442443
options: NormalizedOutputOptions,
443-
existingNames: Record<string, unknown>,
444+
bundle: OutputBundleWithPlaceholders,
444445
unsetOptions: ReadonlySet<string>
445446
): string {
446447
const [{ id }] = this.orderedModules;
@@ -480,7 +481,7 @@ export default class Chunk {
480481
});
481482
path = `_virtual/${fileName}`;
482483
}
483-
return makeUnique(normalize(path), existingNames);
484+
return makeUnique(normalize(path), bundle);
484485
}
485486

486487
getChunkInfo(): PreRenderedChunk {
@@ -885,7 +886,7 @@ export default class Chunk {
885886
private computeContentHashWithDependencies(
886887
addons: Addons,
887888
options: NormalizedOutputOptions,
888-
existingNames: Record<string, unknown>
889+
bundle: OutputBundleWithPlaceholders
889890
): string {
890891
const hash = createHash();
891892
hash.update([addons.intro, addons.outro, addons.banner, addons.footer].join(':'));
@@ -896,7 +897,7 @@ export default class Chunk {
896897
hash.update(`:${current.renderPath}`);
897898
} else {
898899
hash.update(current.getRenderedHash());
899-
hash.update(current.generateId(addons, options, existingNames, false));
900+
hash.update(current.generateId(addons, options, bundle, false));
900901
}
901902
if (current instanceof ExternalModule) continue;
902903
for (const dependency of [...current.dependencies, ...current.dynamicDependencies]) {

‎src/rollup/rollup.ts‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import { version as rollupVersion } from 'package.json';
22
import Bundle from '../Bundle';
33
import Graph from '../Graph';
4-
import { getSortedValidatedPlugins } from '../utils/PluginDriver';
54
import type { PluginDriver } from '../utils/PluginDriver';
5+
import { getSortedValidatedPlugins } from '../utils/PluginDriver';
66
import { ensureArray } from '../utils/ensureArray';
77
import { errAlreadyClosed, errCannotEmitFromOptionsHook, error } from '../utils/error';
88
import { promises as fs } from '../utils/fs';
@@ -26,6 +26,7 @@ import type {
2626
RollupOutput,
2727
RollupWatcher
2828
} from './types';
29+
import { OutputBundle } from './types';
2930

3031
export default function rollup(rawInputOptions: GenericConfigObject): Promise<RollupBuild> {
3132
return rollupInternal(rawInputOptions, null);
@@ -233,21 +234,17 @@ function getOutputOptions(
233234
);
234235
}
235236

236-
function createOutput(
237-
outputBundle: Record<string, OutputChunk | OutputAsset | Record<string, never>>
238-
): RollupOutput {
237+
function createOutput(outputBundle: OutputBundle): RollupOutput {
239238
return {
240239
output: (
241240
Object.values(outputBundle).filter(outputFile => Object.keys(outputFile).length > 0) as (
242241
| OutputChunk
243242
| OutputAsset
244243
)[]
245-
).sort((outputFileA, outputFileB) => {
246-
const fileTypeA = getSortingFileType(outputFileA);
247-
const fileTypeB = getSortingFileType(outputFileB);
248-
if (fileTypeA === fileTypeB) return 0;
249-
return fileTypeA < fileTypeB ? -1 : 1;
250-
}) as [OutputChunk, ...(OutputChunk | OutputAsset)[]]
244+
).sort(
245+
(outputFileA, outputFileB) =>
246+
getSortingFileType(outputFileA) - getSortingFileType(outputFileB)
247+
) as [OutputChunk, ...(OutputChunk | OutputAsset)[]]
251248
};
252249
}
253250

‎src/rollup/types.d.ts‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -358,14 +358,6 @@ export interface OutputBundle {
358358
[fileName: string]: OutputAsset | OutputChunk;
359359
}
360360

361-
export interface FilePlaceholder {
362-
type: 'placeholder';
363-
}
364-
365-
export interface OutputBundleWithPlaceholders {
366-
[fileName: string]: OutputAsset | OutputChunk | FilePlaceholder;
367-
}
368-
369361
export interface FunctionPluginHooks {
370362
augmentChunkHash: (this: PluginContext, chunk: PreRenderedChunk) => string | void;
371363
buildEnd: (this: PluginContext, err?: Error) => void;

‎src/utils/FileEmitter.ts‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,8 @@ import type Graph from '../Graph';
33
import type Module from '../Module';
44
import type {
55
EmittedChunk,
6-
FilePlaceholder,
76
NormalizedInputOptions,
87
NormalizedOutputOptions,
9-
OutputBundleWithPlaceholders,
108
WarningHandler
119
} from '../rollup/types';
1210
import { BuildPhase } from './buildPhase';
@@ -24,6 +22,11 @@ import {
2422
error,
2523
warnDeprecation
2624
} from './error';
25+
import {
26+
FILE_PLACEHOLDER,
27+
lowercaseBundleKeys,
28+
OutputBundleWithPlaceholders
29+
} from './outputBundle';
2730
import { extname } from './path';
2831
import { isPathFragment } from './relativeId';
2932
import { makeUnique, renderNamePattern } from './renderNamePattern';
@@ -64,10 +67,12 @@ function reserveFileNameInBundle(
6467
bundle: OutputBundleWithPlaceholders,
6568
warn: WarningHandler
6669
) {
67-
if (fileName in bundle) {
70+
const lowercaseFileName = fileName.toLowerCase();
71+
if (bundle[lowercaseBundleKeys].has(lowercaseFileName)) {
6872
warn(errFileNameConflict(fileName));
73+
} else {
74+
bundle[fileName] = FILE_PLACEHOLDER;
6975
}
70-
bundle[fileName] = FILE_PLACEHOLDER;
7176
}
7277

7378
interface ConsumedChunk {
@@ -93,10 +98,6 @@ interface EmittedFile {
9398

9499
type ConsumedFile = ConsumedChunk | ConsumedAsset;
95100

96-
export const FILE_PLACEHOLDER: FilePlaceholder = {
97-
type: 'placeholder'
98-
};
99-
100101
function hasValidType(
101102
emittedFile: unknown
102103
): emittedFile is { [key: string]: unknown; type: 'asset' | 'chunk' } {
@@ -228,21 +229,21 @@ export class FileEmitter {
228229
};
229230

230231
public setOutputBundle = (
231-
outputBundle: OutputBundleWithPlaceholders,
232+
bundle: OutputBundleWithPlaceholders,
232233
outputOptions: NormalizedOutputOptions,
233234
facadeChunkByModule: ReadonlyMap<Module, Chunk>
234235
): void => {
235236
this.outputOptions = outputOptions;
236-
this.bundle = outputBundle;
237+
this.bundle = bundle;
237238
this.facadeChunkByModule = facadeChunkByModule;
238-
for (const emittedFile of this.filesByReferenceId.values()) {
239-
if (emittedFile.fileName) {
240-
reserveFileNameInBundle(emittedFile.fileName, this.bundle, this.options.onwarn);
239+
for (const { fileName } of this.filesByReferenceId.values()) {
240+
if (fileName) {
241+
reserveFileNameInBundle(fileName, bundle, this.options.onwarn);
241242
}
242243
}
243244
for (const [referenceId, consumedFile] of this.filesByReferenceId) {
244245
if (consumedFile.type === 'asset' && consumedFile.source !== undefined) {
245-
this.finalizeAsset(consumedFile, consumedFile.source, referenceId, this.bundle);
246+
this.finalizeAsset(consumedFile, consumedFile.source, referenceId, bundle);
246247
}
247248
}
248249
};
@@ -348,6 +349,9 @@ export class FileEmitter {
348349
}
349350
}
350351

352+
// TODO This can lead to a performance problem when many assets are emitted.
353+
// Instead, we should only deduplicate string assets and use their sources as
354+
// object keys for better performance.
351355
function findExistingAssetFileNameWithSource(
352356
bundle: OutputBundleWithPlaceholders,
353357
source: string | Uint8Array

‎src/utils/PluginDriver.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import type {
1010
FunctionPluginHooks,
1111
NormalizedInputOptions,
1212
NormalizedOutputOptions,
13-
OutputBundleWithPlaceholders,
1413
ParallelPluginHooks,
1514
Plugin,
1615
PluginContext,
@@ -28,6 +27,7 @@ import {
2827
error
2928
} from './error';
3029
import { getOrCreate } from './getOrCreate';
30+
import { OutputBundleWithPlaceholders } from './outputBundle';
3131
import { throwPluginError, warnDeprecatedHooks } from './pluginUtils';
3232

3333
/**

‎src/utils/outputBundle.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import { OutputAsset, OutputBundle, OutputChunk } from '../rollup/types';
2+
3+
export const lowercaseBundleKeys = Symbol('bundleKeys');
4+
5+
export const FILE_PLACEHOLDER = {
6+
type: 'placeholder' as const
7+
};
8+
9+
export interface OutputBundleWithPlaceholders {
10+
[fileName: string]: OutputAsset | OutputChunk | typeof FILE_PLACEHOLDER;
11+
[lowercaseBundleKeys]: Set<string>;
12+
}
13+
14+
export const getOutputBundle = (outputBundleBase: OutputBundle): OutputBundleWithPlaceholders => {
15+
const reservedLowercaseBundleKeys = new Set<string>();
16+
return new Proxy(outputBundleBase, {
17+
deleteProperty(target, key) {
18+
if (typeof key === 'string') {
19+
reservedLowercaseBundleKeys.delete(key.toLowerCase());
20+
}
21+
return Reflect.deleteProperty(target, key);
22+
},
23+
get(target, key) {
24+
if (key === lowercaseBundleKeys) {
25+
return reservedLowercaseBundleKeys;
26+
}
27+
return Reflect.get(target, key);
28+
},
29+
set(target, key, value) {
30+
if (typeof key === 'string') {
31+
reservedLowercaseBundleKeys.add(key.toLowerCase());
32+
}
33+
return Reflect.set(target, key, value);
34+
}
35+
}) as OutputBundleWithPlaceholders;
36+
};

‎src/utils/renderNamePattern.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { errFailedValidation, error } from './error';
2+
import { lowercaseBundleKeys, OutputBundleWithPlaceholders } from './outputBundle';
23
import { extname } from './path';
34
import { isPathFragment } from './relativeId';
45

@@ -30,14 +31,15 @@ export function renderNamePattern(
3031
});
3132
}
3233

33-
export function makeUnique(name: string, existingNames: Record<string, unknown>): string {
34-
const existingNamesLowercase = new Set(Object.keys(existingNames).map(key => key.toLowerCase()));
35-
if (!existingNamesLowercase.has(name.toLocaleLowerCase())) return name;
36-
34+
export function makeUnique(
35+
name: string,
36+
{ [lowercaseBundleKeys]: reservedLowercaseBundleKeys }: OutputBundleWithPlaceholders
37+
): string {
38+
if (!reservedLowercaseBundleKeys.has(name.toLowerCase())) return name;
3739
const ext = extname(name);
3840
name = name.substring(0, name.length - ext.length);
3941
let uniqueName: string,
4042
uniqueIndex = 1;
41-
while (existingNamesLowercase.has((uniqueName = name + ++uniqueIndex + ext).toLowerCase()));
43+
while (reservedLowercaseBundleKeys.has((uniqueName = name + ++uniqueIndex + ext).toLowerCase()));
4244
return uniqueName;
4345
}

‎test/function/samples/generate-bundle-mutation/_config.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
module.exports = {
2+
description: 'handles adding or deleting symbols in generateBundle',
3+
options: {
4+
plugins: [
5+
{
6+
name: 'test',
7+
generateBundle(options, bundle) {
8+
const myKey = Symbol('test');
9+
bundle[myKey] = 42;
10+
delete bundle[myKey];
11+
}
12+
}
13+
]
14+
}
15+
};

‎test/function/samples/generate-bundle-mutation/main.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
assert.ok(true);

0 commit comments

Comments
 (0)