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

[build-report] Optimise build-report to prevent memory hog over long running builds #148

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+116 −71
Open

[build-report] Optimise build-report to prevent memory hog over long running builds #148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
55a05e5
Optimise build-report to prevent memory hog over long running builds
yoannmoinet Mar 21, 2025
17ccea9
Remove useless `|| []`
yoannmoinet Mar 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/plugins/build-report/src/esbuild.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,14 @@ export const getEsbuildPlugin = (context: GlobalContext, log: Logger): PluginOpt
return {
setup(build) {
const entryNames = new Map();
const resolvedEntries: ResolvedEntry[] = [];
let resolvedEntries: ResolvedEntry[] = [];
const timeBuildReport = log.time('build report', { start: false });

build.onStart(async () => {
// Start from a clean slate.
entryNames.clear();
resolvedEntries = [];

timeBuildReport.resume();
const timeEntries = log.time('process entries');
// Store entry names based on the configuration.
Expand Down
6 changes: 5 additions & 1 deletion packages/plugins/build-report/src/rollup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,18 @@ import { cleanName, cleanPath, cleanReport, getType } from './helpers';

export const getRollupPlugin = (context: GlobalContext, log: Logger): PluginOptions['rollup'] => {
const timeBuildReport = log.time('build report', { start: false });
const importsReport: Record<
let importsReport: Record<
string,
{
dependencies: Set<string>;
dependents: Set<string>;
}
> = {};
return {
buildStart() {
// Start clean to avoid build up in case of multiple builds (eg. dev server).
importsReport = {};
},
onLog(level, logItem) {
if (level === 'warn') {
context.build.warnings.push(logItem.message || logItem.toString());
Expand Down
175 changes: 106 additions & 69 deletions packages/plugins/build-report/src/xpack.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import type {
PluginOptions,
} from '@dd/core/types';

import { cleanName, getType } from './helpers';
import { cleanName, cleanPath, getType } from './helpers';

export const getXpackPlugin =
(
Expand All @@ -23,23 +23,28 @@ export const getXpackPlugin =
log: Logger,
): PluginOptions['rspack'] & PluginOptions['webpack'] =>
(compiler) => {
const inputs: Input[] = [];
const outputs: Output[] = [];
const entries: Entry[] = [];
let inputs: Input[] = [];
let outputs: Output[] = [];
let entries: Entry[] = [];

// Types for the xpack hooks.
type Compilation = Parameters<Parameters<typeof compiler.hooks.thisCompilation.tap>[1]>[0];
type Module = IterableElement<
Parameters<Parameters<Compilation['hooks']['finishModules']['tap']>[1]>[0]
>;
type Module = IterableElement<Compilation['modules']>;
type Dependency = IterableElement<IterableElement<Module['blocks']>['dependencies']>;
type Chunk = IterableElement<Compilation['chunks']>;
type IndexedModule = {
identifier: Module['identifier'];
dependencies: Dependency[];
blocks: any[];
externalType: unknown;
external: unknown;
};

// Some indexes to help with the report generation.
const reportInputsIndexed: Map<string, Input> = new Map();
const reportOutputsIndexed: Map<string, Output> = new Map();
const modulesPerFile: Map<string, string[]> = new Map();
const moduleIndex: Map<string, Module> = new Map();
const modulesPerFile: Map<string, Set<string>> = new Map();
const moduleIndex: Map<string, IndexedModule> = new Map();

// Some temporary holders to later fill in more data.
const tempSourcemaps: Output[] = [];
Expand All @@ -66,6 +71,17 @@ export const getXpackPlugin =
* 2. Once the build is finished and emitted, we can compute the outputs and the entries.
*/

// Clear the data in case we have multiple compilations (dev server, etc...).
const clear = () => {
inputs = [];
outputs = [];
entries = [];
Comment on lines +76 to +78
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we prefer to do = [] or .length = 0? (to use the same array, but clear it, like what we do with the sets .clear() as we aren't redefining new ones with moduleIndex = new Set())

Copy link
Member Author

Choose a reason for hiding this comment

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

I was using .length = 0 but I kind of found it a bit hacky and not super explicit.

From what I read online, it's seemingly the same performance either = [] or .length = 0, so I went with the most explicit and comprehensible one.

reportInputsIndexed.clear();
reportOutputsIndexed.clear();
moduleIndex.clear();
tempDeps.clear();
};

const cleanExternalName = (name: string) => {
// Removes "external " prefix and surrounding quotes from external dependency names
// Example: 'external var "lodash"' -> 'lodash'
Expand All @@ -74,46 +90,63 @@ export const getXpackPlugin =

// Index the module by its identifier, resource, request, rawRequest, and userRequest.
const getKeysToIndex = (mod: Module): Set<string> => {
const values: Record<string, string> = {
identifier: mod.identifier(),
const indexes = new Set<string>();

const keysOfModuleToIndexOn: string[] = [
'rawRequest',
'resource',
'request',
'userRequest',
];

const indexValue = (value: string) => {
const valueToIndex = cleanPath(value);
indexes.add(valueToIndex);
// RSpack only use "external ..." for external dependencies.
// So we need to clean and add the actual name to the index too.
if (valueToIndex.startsWith('external ')) {
indexes.add(cleanExternalName(valueToIndex));
}
};

if ('resource' in mod && typeof mod.resource === 'string') {
values.resource = mod.resource;
}
if ('request' in mod && typeof mod.request === 'string') {
values.request = mod.request;
}
if ('rawRequest' in mod && typeof mod.rawRequest === 'string') {
values.rawRequest = mod.rawRequest;
}
if ('userRequest' in mod && typeof mod.userRequest === 'string') {
values.userRequest = mod.userRequest;
// Start by indexing the identifier.
indexValue(mod.identifier());

// Then index all the other keys.
for (const key of keysOfModuleToIndexOn) {
const value = mod[key as keyof Module];
if (key && key in mod && typeof value === 'string') {
indexValue(value);
}
}

const keysToIndex: Set<string> = new Set();
return indexes;
};

for (const [key, value] of Object.entries(values)) {
if (!value) {
continue;
}
const createIndexedModule = (mod: Module): IndexedModule => {
const id = mod.identifier();
return {
identifier: () => id,
dependencies: 'dependencies' in mod ? [...mod.dependencies] : [],
blocks: 'blocks' in mod ? [...mod.blocks] : [],
externalType: 'externalType' in mod ? mod.externalType : undefined,
external: 'external' in mod ? mod.external : undefined,
};
};

if (moduleIndex.has(value)) {
log.debug(`Module ${mod.identifier()} is already indexed by ${key}.`);
if (moduleIndex.get(value) !== mod) {
log.debug(`Module ${mod.identifier()} is indexed with a different value.`);
}
const indexModule = (mod: Module) => {
const moduleToIndex = createIndexedModule(mod);
const keysToIndex = getKeysToIndex(mod);
for (const key of keysToIndex) {
if (moduleIndex.has(key)) {
// Update the existing module.
const previousModule = moduleIndex.get(key)!;
previousModule.dependencies.push(...(moduleToIndex.dependencies || []));
previousModule.blocks.push(...(moduleToIndex.blocks || []));
} else {
keysToIndex.add(value);
// RSpack only use "external ..." for external dependencies.
// So we need to clean and add the actual name to the index too.
if (value.startsWith('external ')) {
keysToIndex.add(cleanExternalName(value));
}
moduleIndex.set(key, moduleToIndex);
}
}

return keysToIndex;
};

// Aggregate all dependencies from a module.
Expand All @@ -137,18 +170,22 @@ export const getXpackPlugin =
return dependencies;
};

const getModuleFromDep = (mod: Module, dep: Dependency): Module | undefined => {
const getModuleFromDep = (mod: Module, dep: Dependency): IndexedModule | undefined => {
if ('request' in dep && dep.request) {
if (moduleIndex.has(dep.request)) {
return moduleIndex.get(dep.request);
const cleanRequest = cleanPath(dep.request);
if (moduleIndex.has(cleanRequest)) {
return moduleIndex.get(cleanRequest);
}
if (mod.context && moduleIndex.has(getAbsolutePath(mod.context, dep.request))) {
return moduleIndex.get(getAbsolutePath(mod.context, dep.request));
if (mod.context) {
const cleanedPath = getAbsolutePath(cleanPath(mod.context), cleanRequest);
if (moduleIndex.has(cleanedPath)) {
return moduleIndex.get(cleanedPath);
}
}
}
};

const isExternal = (mod: Module) => {
const isExternal = (mod: Module | IndexedModule) => {
if ('externalType' in mod && mod.externalType) {
return true;
}
Expand All @@ -163,6 +200,7 @@ export const getXpackPlugin =

// Intercept the compilation to then get the modules.
compiler.hooks.thisCompilation.tap(PLUGIN_NAME, (compilation) => {
clear();
// Intercept the modules to build the dependency graph.
compilation.hooks.finishModules.tap(
PLUGIN_NAME,
Expand All @@ -172,10 +210,7 @@ export const getXpackPlugin =
// First loop to create indexes.
const timeIndex = log.time('indexing modules');
for (const module of finishedModules) {
const keysToIndex = getKeysToIndex(module);
for (const key of keysToIndex) {
moduleIndex.set(key, module);
}
indexModule(module);
}
timeIndex.end();

Expand Down Expand Up @@ -218,24 +253,20 @@ export const getXpackPlugin =
}

// Create dependents relationships.
const moduleDeps = tempDeps.get(moduleIdentifier) || {
dependents: new Set(),
dependencies: new Set(),
};
for (const depIdentifier of dependencies) {
const depDeps = tempDeps.get(depIdentifier) || {
dependencies: new Set(),
dependents: new Set(),
};
depDeps.dependents.add(moduleIdentifier);
moduleDeps.dependencies.add(depIdentifier);
tempDeps.set(depIdentifier, depDeps);
}

const moduleDeps = tempDeps.get(moduleIdentifier) || {
dependents: new Set(),
dependencies: new Set(),
};

for (const moduleDep of dependencies) {
moduleDeps.dependencies.add(moduleDep);
}

// Store the dependencies.
tempDeps.set(moduleIdentifier, moduleDeps);

Expand Down Expand Up @@ -340,8 +371,11 @@ export const getXpackPlugin =
if (getType(file) === 'map') {
continue;
}
const fileModules = modulesPerFile.get(file) || [];
modulesPerFile.set(file, [...fileModules, ...chunkModules]);
const fileModules = modulesPerFile.get(file) || new Set();
for (const module of chunkModules) {
fileModules.add(module);
}
modulesPerFile.set(file, fileModules);
}
}
timeChunks.end();
Expand Down Expand Up @@ -404,8 +438,8 @@ export const getXpackPlugin =
// Build entries
const timeEntries = log.time('building entries');
for (const [name, entrypoint] of result.entrypoints) {
const entryOutputs: Output[] = [];
const entryInputs: Input[] = [];
const entryOutputs: Map<string, Output> = new Map();
const entryInputs: Map<string, Input> = new Map();
let size = 0;
const entryFiles = entrypoint.chunks.flatMap(getChunkFiles);
// FIXME This is not a 100% reliable way to get the entry filename.
Expand Down Expand Up @@ -436,11 +470,14 @@ export const getXpackPlugin =
log.debug(`Could not find output of ${JSON.stringify(file)}`);
continue;
}

if (outputFound.type !== 'map' && !entryOutputs.includes(outputFound)) {
entryOutputs.push(outputFound);
if (outputFound.type !== 'map' && !entryOutputs.has(outputFound.name)) {
entryOutputs.set(outputFound.name, outputFound);
// We know it's not a map, so we cast it.
entryInputs.push(...(outputFound.inputs as Input[]));
for (const input of outputFound.inputs as Input[]) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is the cast necessary here?

Copy link
Member Author

@yoannmoinet yoannmoinet Mar 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

In the case of sourcemaps, file.inputs are Outputs as sourcemaps are built out of both Output and Input type of files.

So since I'm checking it's not a sourcemaps earlier, I'm casting it to make the rest of code type error free.

Very much looking for a better way.

if (!entryInputs.has(input.filepath)) {
entryInputs.set(input.filepath, input);
}
}
// We don't want to include sourcemaps in the sizing.
size += outputFound.size;
}
Expand All @@ -452,8 +489,8 @@ export const getXpackPlugin =
? getAbsolutePath(context.bundler.outDir, entryFilename)
: 'unknown',
size,
inputs: Array.from(new Set(entryInputs)),
outputs: entryOutputs,
inputs: Array.from(entryInputs.values()),
outputs: Array.from(entryOutputs.values()),
type: entryFilename ? getType(entryFilename) : 'unknown',
};

Expand Down