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.

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

Improve performance of chunk naming collision check #4643

Merged
merged 2 commits into from Sep 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 12 additions & 8 deletions src/Bundle.ts
Expand Up @@ -8,11 +8,9 @@ import type {
NormalizedOutputOptions,
OutputAsset,
OutputBundle,
OutputBundleWithPlaceholders,
OutputChunk,
WarningHandler
} from './rollup/types';
import { FILE_PLACEHOLDER } from './utils/FileEmitter';
import type { PluginDriver } from './utils/PluginDriver';
import { type Addons, createAddons } from './utils/addons';
import { getChunkAssignments } from './utils/chunkAssignment';
Expand All @@ -26,6 +24,11 @@ import {
} from './utils/error';
import { sortByExecutionOrder } from './utils/executionOrder';
import { type GenerateCodeSnippets, getGenerateCodeSnippets } from './utils/generateCodeSnippets';
import {
FILE_PLACEHOLDER,
getOutputBundle,
OutputBundleWithPlaceholders
} from './utils/outputBundle';
import { basename, isAbsolute } from './utils/path';
import { timeEnd, timeStart } from './utils/timers';

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

async generate(isWrite: boolean): Promise<OutputBundle> {
timeStart('GENERATE', 1);
const outputBundle: OutputBundleWithPlaceholders = Object.create(null);
const outputBundleBase: OutputBundle = Object.create(null);
const outputBundle = getOutputBundle(outputBundleBase);
this.pluginDriver.setOutputBundle(outputBundle, this.outputOptions, this.facadeChunkByModule);
try {
await this.pluginDriver.hookParallel('renderStart', [this.outputOptions, this.inputOptions]);
Expand Down Expand Up @@ -78,23 +82,23 @@ export default class Bundle {
this.finaliseAssets(outputBundle);

timeEnd('GENERATE', 1);
return outputBundle as OutputBundle;
return outputBundleBase;
}

private async addFinalizedChunksToBundle(
chunks: readonly Chunk[],
inputBase: string,
addons: Addons,
outputBundle: OutputBundleWithPlaceholders,
bundle: OutputBundleWithPlaceholders,
snippets: GenerateCodeSnippets
): Promise<void> {
this.assignChunkIds(chunks, inputBase, addons, outputBundle);
this.assignChunkIds(chunks, inputBase, addons, bundle);
for (const chunk of chunks) {
outputBundle[chunk.id!] = chunk.getChunkInfoWithFileNames() as OutputChunk;
bundle[chunk.id!] = chunk.getChunkInfoWithFileNames() as OutputChunk;
}
await Promise.all(
chunks.map(async chunk => {
const outputChunk = outputBundle[chunk.id!] as OutputChunk;
const outputChunk = bundle[chunk.id!] as OutputChunk;
Object.assign(
outputChunk,
await chunk.render(this.outputOptions, addons, outputChunk, snippets)
Expand Down
15 changes: 8 additions & 7 deletions src/Chunk.ts
Expand Up @@ -51,6 +51,7 @@ import {
isDefaultAProperty,
namespaceInteropHelpersByInteropType
} from './utils/interopHelpers';
import { OutputBundleWithPlaceholders } from './utils/outputBundle';
import { dirname, extname, isAbsolute, normalize, resolve } from './utils/path';
import relativeId, { getAliasName, getImportPath } from './utils/relativeId';
import renderChunk from './utils/renderChunk';
Expand Down Expand Up @@ -410,7 +411,7 @@ export default class Chunk {
generateId(
addons: Addons,
options: NormalizedOutputOptions,
existingNames: Record<string, unknown>,
bundle: OutputBundleWithPlaceholders,
includeHash: boolean
): string {
if (this.fileName !== null) {
Expand All @@ -428,19 +429,19 @@ export default class Chunk {
format: () => options.format,
hash: () =>
includeHash
? this.computeContentHashWithDependencies(addons, options, existingNames)
? this.computeContentHashWithDependencies(addons, options, bundle)
: '[hash]',
name: () => this.getChunkName()
}
),
existingNames
bundle
);
}

generateIdPreserveModules(
preserveModulesRelativeDir: string,
options: NormalizedOutputOptions,
existingNames: Record<string, unknown>,
bundle: OutputBundleWithPlaceholders,
unsetOptions: ReadonlySet<string>
): string {
const [{ id }] = this.orderedModules;
Expand Down Expand Up @@ -480,7 +481,7 @@ export default class Chunk {
});
path = `_virtual/${fileName}`;
}
return makeUnique(normalize(path), existingNames);
return makeUnique(normalize(path), bundle);
}

getChunkInfo(): PreRenderedChunk {
Expand Down Expand Up @@ -885,7 +886,7 @@ export default class Chunk {
private computeContentHashWithDependencies(
addons: Addons,
options: NormalizedOutputOptions,
existingNames: Record<string, unknown>
bundle: OutputBundleWithPlaceholders
): string {
const hash = createHash();
hash.update([addons.intro, addons.outro, addons.banner, addons.footer].join(':'));
Expand All @@ -896,7 +897,7 @@ export default class Chunk {
hash.update(`:${current.renderPath}`);
} else {
hash.update(current.getRenderedHash());
hash.update(current.generateId(addons, options, existingNames, false));
hash.update(current.generateId(addons, options, bundle, false));
}
if (current instanceof ExternalModule) continue;
for (const dependency of [...current.dependencies, ...current.dynamicDependencies]) {
Expand Down
17 changes: 7 additions & 10 deletions src/rollup/rollup.ts
@@ -1,8 +1,8 @@
import { version as rollupVersion } from 'package.json';
import Bundle from '../Bundle';
import Graph from '../Graph';
import { getSortedValidatedPlugins } from '../utils/PluginDriver';
import type { PluginDriver } from '../utils/PluginDriver';
import { getSortedValidatedPlugins } from '../utils/PluginDriver';
import { ensureArray } from '../utils/ensureArray';
import { errAlreadyClosed, errCannotEmitFromOptionsHook, error } from '../utils/error';
import { promises as fs } from '../utils/fs';
Expand All @@ -26,6 +26,7 @@ import type {
RollupOutput,
RollupWatcher
} from './types';
import { OutputBundle } from './types';

export default function rollup(rawInputOptions: GenericConfigObject): Promise<RollupBuild> {
return rollupInternal(rawInputOptions, null);
Expand Down Expand Up @@ -233,21 +234,17 @@ function getOutputOptions(
);
}

function createOutput(
outputBundle: Record<string, OutputChunk | OutputAsset | Record<string, never>>
): RollupOutput {
function createOutput(outputBundle: OutputBundle): RollupOutput {
return {
output: (
Object.values(outputBundle).filter(outputFile => Object.keys(outputFile).length > 0) as (
| OutputChunk
| OutputAsset
)[]
).sort((outputFileA, outputFileB) => {
const fileTypeA = getSortingFileType(outputFileA);
const fileTypeB = getSortingFileType(outputFileB);
if (fileTypeA === fileTypeB) return 0;
return fileTypeA < fileTypeB ? -1 : 1;
}) as [OutputChunk, ...(OutputChunk | OutputAsset)[]]
).sort(
(outputFileA, outputFileB) =>
getSortingFileType(outputFileA) - getSortingFileType(outputFileB)
) as [OutputChunk, ...(OutputChunk | OutputAsset)[]]
};
}

Expand Down
8 changes: 0 additions & 8 deletions src/rollup/types.d.ts
Expand Up @@ -358,14 +358,6 @@ export interface OutputBundle {
[fileName: string]: OutputAsset | OutputChunk;
}

export interface FilePlaceholder {
type: 'placeholder';
}

export interface OutputBundleWithPlaceholders {
[fileName: string]: OutputAsset | OutputChunk | FilePlaceholder;
}

export interface FunctionPluginHooks {
augmentChunkHash: (this: PluginContext, chunk: PreRenderedChunk) => string | void;
buildEnd: (this: PluginContext, err?: Error) => void;
Expand Down
32 changes: 18 additions & 14 deletions src/utils/FileEmitter.ts
Expand Up @@ -3,10 +3,8 @@ import type Graph from '../Graph';
import type Module from '../Module';
import type {
EmittedChunk,
FilePlaceholder,
NormalizedInputOptions,
NormalizedOutputOptions,
OutputBundleWithPlaceholders,
WarningHandler
} from '../rollup/types';
import { BuildPhase } from './buildPhase';
Expand All @@ -24,6 +22,11 @@ import {
error,
warnDeprecation
} from './error';
import {
FILE_PLACEHOLDER,
lowercaseBundleKeys,
OutputBundleWithPlaceholders
} from './outputBundle';
import { extname } from './path';
import { isPathFragment } from './relativeId';
import { makeUnique, renderNamePattern } from './renderNamePattern';
Expand Down Expand Up @@ -64,10 +67,12 @@ function reserveFileNameInBundle(
bundle: OutputBundleWithPlaceholders,
warn: WarningHandler
) {
if (fileName in bundle) {
const lowercaseFileName = fileName.toLowerCase();
if (bundle[lowercaseBundleKeys].has(lowercaseFileName)) {
warn(errFileNameConflict(fileName));
} else {
bundle[fileName] = FILE_PLACEHOLDER;
}
bundle[fileName] = FILE_PLACEHOLDER;
}

interface ConsumedChunk {
Expand All @@ -93,10 +98,6 @@ interface EmittedFile {

type ConsumedFile = ConsumedChunk | ConsumedAsset;

export const FILE_PLACEHOLDER: FilePlaceholder = {
type: 'placeholder'
};

function hasValidType(
emittedFile: unknown
): emittedFile is { [key: string]: unknown; type: 'asset' | 'chunk' } {
Expand Down Expand Up @@ -228,21 +229,21 @@ export class FileEmitter {
};

public setOutputBundle = (
outputBundle: OutputBundleWithPlaceholders,
bundle: OutputBundleWithPlaceholders,
outputOptions: NormalizedOutputOptions,
facadeChunkByModule: ReadonlyMap<Module, Chunk>
): void => {
this.outputOptions = outputOptions;
this.bundle = outputBundle;
this.bundle = bundle;
this.facadeChunkByModule = facadeChunkByModule;
for (const emittedFile of this.filesByReferenceId.values()) {
if (emittedFile.fileName) {
reserveFileNameInBundle(emittedFile.fileName, this.bundle, this.options.onwarn);
for (const { fileName } of this.filesByReferenceId.values()) {
if (fileName) {
reserveFileNameInBundle(fileName, bundle, this.options.onwarn);
}
}
for (const [referenceId, consumedFile] of this.filesByReferenceId) {
if (consumedFile.type === 'asset' && consumedFile.source !== undefined) {
this.finalizeAsset(consumedFile, consumedFile.source, referenceId, this.bundle);
this.finalizeAsset(consumedFile, consumedFile.source, referenceId, bundle);
}
}
};
Expand Down Expand Up @@ -348,6 +349,9 @@ export class FileEmitter {
}
}

// TODO This can lead to a performance problem when many assets are emitted.
// Instead, we should only deduplicate string assets and use their sources as
// object keys for better performance.
function findExistingAssetFileNameWithSource(
bundle: OutputBundleWithPlaceholders,
source: string | Uint8Array
Expand Down
2 changes: 1 addition & 1 deletion src/utils/PluginDriver.ts
Expand Up @@ -10,7 +10,6 @@ import type {
FunctionPluginHooks,
NormalizedInputOptions,
NormalizedOutputOptions,
OutputBundleWithPlaceholders,
ParallelPluginHooks,
Plugin,
PluginContext,
Expand All @@ -28,6 +27,7 @@ import {
error
} from './error';
import { getOrCreate } from './getOrCreate';
import { OutputBundleWithPlaceholders } from './outputBundle';
import { throwPluginError, warnDeprecatedHooks } from './pluginUtils';

/**
Expand Down
36 changes: 36 additions & 0 deletions src/utils/outputBundle.ts
@@ -0,0 +1,36 @@
import { OutputAsset, OutputBundle, OutputChunk } from '../rollup/types';

export const lowercaseBundleKeys = Symbol('bundleKeys');

export const FILE_PLACEHOLDER = {
type: 'placeholder' as const
};

export interface OutputBundleWithPlaceholders {
[fileName: string]: OutputAsset | OutputChunk | typeof FILE_PLACEHOLDER;
[lowercaseBundleKeys]: Set<string>;
}

export const getOutputBundle = (outputBundleBase: OutputBundle): OutputBundleWithPlaceholders => {
const reservedLowercaseBundleKeys = new Set<string>();
return new Proxy(outputBundleBase, {
deleteProperty(target, key) {
if (typeof key === 'string') {
reservedLowercaseBundleKeys.delete(key.toLowerCase());
}
return Reflect.deleteProperty(target, key);
},
get(target, key) {
if (key === lowercaseBundleKeys) {
return reservedLowercaseBundleKeys;
}
return Reflect.get(target, key);
},
set(target, key, value) {
if (typeof key === 'string') {
reservedLowercaseBundleKeys.add(key.toLowerCase());
}
return Reflect.set(target, key, value);
}
}) as OutputBundleWithPlaceholders;
};
12 changes: 7 additions & 5 deletions src/utils/renderNamePattern.ts
@@ -1,4 +1,5 @@
import { errFailedValidation, error } from './error';
import { lowercaseBundleKeys, OutputBundleWithPlaceholders } from './outputBundle';
import { extname } from './path';
import { isPathFragment } from './relativeId';

Expand Down Expand Up @@ -30,14 +31,15 @@ export function renderNamePattern(
});
}

export function makeUnique(name: string, existingNames: Record<string, unknown>): string {
const existingNamesLowercase = new Set(Object.keys(existingNames).map(key => key.toLowerCase()));
if (!existingNamesLowercase.has(name.toLocaleLowerCase())) return name;

export function makeUnique(
name: string,
{ [lowercaseBundleKeys]: reservedLowercaseBundleKeys }: OutputBundleWithPlaceholders
): string {
if (!reservedLowercaseBundleKeys.has(name.toLowerCase())) return name;
const ext = extname(name);
name = name.substring(0, name.length - ext.length);
let uniqueName: string,
uniqueIndex = 1;
while (existingNamesLowercase.has((uniqueName = name + ++uniqueIndex + ext).toLowerCase()));
while (reservedLowercaseBundleKeys.has((uniqueName = name + ++uniqueIndex + ext).toLowerCase()));
return uniqueName;
}
15 changes: 15 additions & 0 deletions test/function/samples/generate-bundle-mutation/_config.js
@@ -0,0 +1,15 @@
module.exports = {
description: 'handles adding or deleting symbols in generateBundle',
options: {
plugins: [
{
name: 'test',
generateBundle(options, bundle) {
const myKey = Symbol('test');
bundle[myKey] = 42;
delete bundle[myKey];
}
}
]
}
};
1 change: 1 addition & 0 deletions test/function/samples/generate-bundle-mutation/main.js
@@ -0,0 +1 @@
assert.ok(true);