Skip to content

Commit

Permalink
Improve performance of chunk naming collision check
Browse files Browse the repository at this point in the history
  • Loading branch information
lukastaegert committed Sep 21, 2022
1 parent eca6a6c commit 6709adf
Show file tree
Hide file tree
Showing 8 changed files with 89 additions and 53 deletions.
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;
}

0 comments on commit 6709adf

Please sign in to comment.