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

Collect request stats #9633

Merged
merged 11 commits into from
Apr 12, 2024
3 changes: 3 additions & 0 deletions packages/core/core/src/Parcel.js
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@ export default class Parcel {
bundleGraph: event.bundleGraph,
buildTime: 0,
requestBundle: event.requestBundle,
unstable_requestStats: {},
};
}

Expand All @@ -383,6 +384,7 @@ export default class Parcel {

return result;
},
unstable_requestStats: this.#requestTracker.flushStats(),
};

await this.#reporterRunner.report(event);
Expand All @@ -400,6 +402,7 @@ export default class Parcel {
let event = {
type: 'buildFailure',
diagnostics: Array.isArray(diagnostic) ? diagnostic : [diagnostic],
unstable_requestStats: this.#requestTracker.flushStats(),
};

await this.#reporterRunner.report(event);
Expand Down
24 changes: 24 additions & 0 deletions packages/core/core/src/RequestTracker.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ export const requestTypes = {
};

type RequestType = $Values<typeof requestTypes>;
type RequestTypeName = $Keys<typeof requestTypes>;

type RequestGraphNode =
| RequestNode
Expand Down Expand Up @@ -1027,6 +1028,7 @@ export default class RequestTracker {
farm: WorkerFarm;
options: ParcelOptions;
signal: ?AbortSignal;
stats: Map<RequestType, number> = new Map();

constructor({
graph,
Expand Down Expand Up @@ -1205,6 +1207,9 @@ export default class RequestTracker {

try {
let node = this.graph.getRequestNode(requestNodeId);

this.stats.set(request.type, (this.stats.get(request.type) ?? 0) + 1);

let result = await request.run({
input: request.input,
api,
Expand All @@ -1227,6 +1232,25 @@ export default class RequestTracker {
}
}

flushStats(): {[requestType: string]: number} {
let requestTypeEntries = {};

for (let key of (Object.keys(requestTypes): RequestTypeName[])) {
requestTypeEntries[requestTypes[key]] = key;
}

let formattedStats = {};

for (let [requestType, count] of this.stats.entries()) {
let requestTypeName = requestTypeEntries[requestType];
formattedStats[requestTypeName] = count;
}

this.stats = new Map();

return formattedStats;
}

createAPI<TResult>(
requestId: NodeId,
previousInvalidations: Array<RequestInvalidation>,
Expand Down
15 changes: 15 additions & 0 deletions packages/core/core/src/requests/AssetGraphRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type {StaticRunOpts, RunAPI} from '../RequestTracker';
import type {EntryResult} from './EntryRequest';
import type {PathRequestInput} from './PathRequest';
import type {Diagnostic} from '@parcel/diagnostic';
import logger from '@parcel/logger';

import invariant from 'assert';
import nullthrows from 'nullthrows';
Expand Down Expand Up @@ -200,7 +201,9 @@ export class AssetGraphBuilder {
'A root node is required to traverse',
);

let visitedAssetGroups = new Set();
let visited = new Set([rootNodeId]);

const visit = (nodeId: NodeId) => {
if (errors.length > 0) {
return;
Expand All @@ -223,6 +226,10 @@ export class AssetGraphBuilder {
(!visited.has(childNodeId) || child.hasDeferred) &&
this.shouldVisitChild(nodeId, childNodeId)
) {
if (child.type === 'asset_group') {
visitedAssetGroups.add(childNodeId);
}

visited.add(childNodeId);
visit(childNodeId);
}
Expand All @@ -232,6 +239,14 @@ export class AssetGraphBuilder {
visit(rootNodeId);
await this.queue.run();

logger.verbose({
origin: '@parcel/core',
message: 'Asset graph walked',
meta: {
visitedAssetGroupsCount: visitedAssetGroups.size,
},
});

if (this.prevChangedAssetsPropagation) {
// Add any previously seen Assets that have not been propagated yet to
// 'this.changedAssetsPropagation', but only if they still remain in the graph
Expand Down
2 changes: 2 additions & 0 deletions packages/core/types-internal/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1976,6 +1976,7 @@ export type BuildSuccessEvent = {|
+buildTime: number,
+changedAssets: Map<string, Asset>,
+requestBundle: (bundle: NamedBundle) => Promise<BuildSuccessEvent>,
+unstable_requestStats: {[requestType: string]: number},
|};

/**
Expand All @@ -1985,6 +1986,7 @@ export type BuildSuccessEvent = {|
export type BuildFailureEvent = {|
+type: 'buildFailure',
+diagnostics: Array<Diagnostic>,
+unstable_requestStats: {[requestType: string]: number},
|};

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/reporters/tracer/src/TracerReporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,14 @@ export default (new Reporter({
case 'buildSuccess':
case 'buildFailure':
nullthrows(tracer).flush();
tracer = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

@marcins This actually fixes a race condition that showed up in the tests where it's possible that the tracer isn't cleared quick enough.

// We explicitly trigger `end` on the writeStream for the trace, then we need to wait for
// the `close` event before resolving the promise this report function returns to ensure
// that the file has been properly closed and moved from it's temp location before Parcel
// shuts down.
return new Promise((resolve, reject) => {
nullthrows(writeStream).once('close', err => {
writeStream = null;
tracer = null;
if (err) {
reject(err);
} else {
Expand Down
Loading