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

Reduce redundancy in the RequestGraph's Request, Env, and Option nodes #9383

Merged
merged 22 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
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
122 changes: 71 additions & 51 deletions packages/core/core/src/RequestTracker.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ import {
import {hashString} from '@parcel/rust';
import {ContentGraph} from '@parcel/graph';
import {deserialize, serialize} from './serializer';
import {assertSignalNotAborted, hashFromOption} from './utils';
import {
assertSignalNotAborted,
hashFromOption,
keyFromEnvOrOptionContentKey,
} from './utils';
import {
type ProjectPath,
fromProjectPathRelative,
Expand Down Expand Up @@ -84,45 +88,61 @@ type SerializedRequestGraph = {|
|};

type FileNode = {|id: ContentKey, +type: 'file'|};

type GlobNode = {|id: ContentKey, +type: 'glob', value: InternalGlob|};

type FileNameNode = {|
id: ContentKey,
+type: 'file_name',
value: string,
|};

type EnvNode = {|
id: ContentKey,
+type: 'env',
value: {|key: string, value: string | void|},
value: string | void,
|};

type OptionNode = {|
id: ContentKey,
+type: 'option',
value: {|key: string, hash: string|},
hash: string,
|};

type Request<TInput, TResult> = {|
id: string,
+type: string,
+type: RequestType,
input: TInput,
run: ({|input: TInput, ...StaticRunOpts<TResult>|}) => Async<TResult>,
|};

type StoredRequest = {|
id: string,
+type: string,
result?: mixed,
resultCacheKey?: ?string,
|};

type InvalidateReason = number;
type RequestNode = {|
id: ContentKey,
+type: 'request',
value: StoredRequest,
+requestType?: RequestType,
invalidateReason: InvalidateReason,
result?: mixed,
resultCacheKey?: ?string,
hash?: string,
|};

export type RequestType =
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

| 'parcel_build_request'
| 'bundle_graph_request'
| 'asset_graph_request'
| 'entry_request'
| 'target_request'
| 'parcel_config_request'
| 'path_request'
| 'dev_dep_request'
| 'asset_request'
| 'config_request'
| 'write_bundles_request'
| 'package_request'
| 'write_bundle_request'
| 'validation_request'
| string;
gorakong marked this conversation as resolved.
Show resolved Hide resolved

type RequestGraphNode =
| RequestNode
| FileNode
Expand All @@ -143,8 +163,8 @@ export type RunAPI<TResult> = {|
storeResult(result: TResult, cacheKey?: string): void,
getRequestResult<T>(contentKey: ContentKey): Async<?T>,
getPreviousResult<T>(ifMatch?: string): Async<?T>,
getSubRequests(): Array<StoredRequest>,
getInvalidSubRequests(): Array<StoredRequest>,
getSubRequests(): Array<RequestNode>,
getInvalidSubRequests(): Array<RequestNode>,
canSkipSubrequest(ContentKey): boolean,
runRequest: <TInput, TResult>(
subRequest: Request<TInput, TResult>,
Expand Down Expand Up @@ -177,32 +197,25 @@ const nodeFromGlob = (glob: InternalGlob): RequestGraphNode => ({
const nodeFromFileName = (fileName: string): RequestGraphNode => ({
id: 'file_name:' + fileName,
type: 'file_name',
value: fileName,
});

const nodeFromRequest = (request: StoredRequest): RequestGraphNode => ({
const nodeFromRequest = (request: RequestNode): RequestGraphNode => ({
id: request.id,
type: 'request',
value: request,
requestType: request.requestType,
invalidateReason: INITIAL_BUILD,
});

const nodeFromEnv = (env: string, value: string | void): RequestGraphNode => ({
id: 'env:' + env,
type: 'env',
value: {
key: env,
value,
},
value,
});

const nodeFromOption = (option: string, value: mixed): RequestGraphNode => ({
id: 'option:' + option,
type: 'option',
value: {
key: option,
hash: hashFromOption(value),
},
hash: hashFromOption(value),
});

export class RequestGraph extends ContentGraph<
Expand Down Expand Up @@ -344,7 +357,7 @@ export class RequestGraph extends ContentGraph<
for (let nodeId of this.envNodeIds) {
let node = nullthrows(this.getNode(nodeId));
invariant(node.type === 'env');
if (env[node.value.key] !== node.value.value) {
if (env[keyFromEnvOrOptionContentKey(node.id)] !== node.value) {
let parentNodes = this.getNodeIdsConnectedTo(
nodeId,
requestGraphEdgeTypes.invalidated_by_update,
Expand All @@ -360,7 +373,10 @@ export class RequestGraph extends ContentGraph<
for (let nodeId of this.optionNodeIds) {
let node = nullthrows(this.getNode(nodeId));
invariant(node.type === 'option');
if (hashFromOption(options[node.value.key]) !== node.value.hash) {
if (
hashFromOption(options[keyFromEnvOrOptionContentKey(node.id)]) !==
node.hash
) {
let parentNodes = this.getNodeIdsConnectedTo(
nodeId,
requestGraphEdgeTypes.invalidated_by_update,
Expand Down Expand Up @@ -609,15 +625,18 @@ export class RequestGraph extends ContentGraph<
case 'file':
return {type: 'file', filePath: toProjectPathUnsafe(node.id)};
case 'env':
return {type: 'env', key: node.value.key};
return {type: 'env', key: keyFromEnvOrOptionContentKey(node.id)};
case 'option':
return {type: 'option', key: node.value.key};
return {
type: 'option',
key: keyFromEnvOrOptionContentKey(node.id),
};
}
})
.filter(Boolean);
}

getSubRequests(requestNodeId: NodeId): Array<StoredRequest> {
getSubRequests(requestNodeId: NodeId): Array<RequestNode> {
if (!this.hasNode(requestNodeId)) {
return [];
}
Expand All @@ -630,11 +649,11 @@ export class RequestGraph extends ContentGraph<
return subRequests.map(nodeId => {
let node = nullthrows(this.getNode(nodeId));
invariant(node.type === 'request');
return node.value;
return node;
});
}

getInvalidSubRequests(requestNodeId: NodeId): Array<StoredRequest> {
getInvalidSubRequests(requestNodeId: NodeId): Array<RequestNode> {
if (!this.hasNode(requestNodeId)) {
return [];
}
Expand All @@ -649,7 +668,7 @@ export class RequestGraph extends ContentGraph<
.map(nodeId => {
let node = nullthrows(this.getNode(nodeId));
invariant(node.type === 'request');
return node.value;
return node;
});
}

Expand Down Expand Up @@ -843,7 +862,7 @@ export default class RequestTracker {
this.signal = signal;
}

startRequest(request: StoredRequest): {|
startRequest(request: RequestNode): {|
requestNodeId: NodeId,
deferred: Deferred<boolean>,
|} {
Expand Down Expand Up @@ -871,8 +890,8 @@ export default class RequestTracker {
storeResult(nodeId: NodeId, result: mixed, cacheKey: ?string) {
let node = this.graph.getNode(nodeId);
if (node && node.type === 'request') {
node.value.result = result;
node.value.resultCacheKey = cacheKey;
node.result = result;
node.resultCacheKey = cacheKey;
}
}

Expand All @@ -891,21 +910,21 @@ export default class RequestTracker {
let node = nullthrows(this.graph.getNodeByContentKey(contentKey));
invariant(node.type === 'request');

if (ifMatch != null && node.value.resultCacheKey !== ifMatch) {
if (ifMatch != null && node.resultCacheKey !== ifMatch) {
return null;
}

if (node.value.result != undefined) {
if (node.result != undefined) {
// $FlowFixMe
let result: T = (node.value.result: any);
let result: T = (node.result: any);
return result;
} else if (node.value.resultCacheKey != null && ifMatch == null) {
let key = node.value.resultCacheKey;
} else if (node.resultCacheKey != null && ifMatch == null) {
let key = node.resultCacheKey;
invariant(this.options.cache.hasLargeBlob(key));
let cachedResult: T = deserialize(
await this.options.cache.getLargeBlob(key),
);
node.value.result = cachedResult;
node.result = cachedResult;
return cachedResult;
}
}
Expand Down Expand Up @@ -940,12 +959,12 @@ export default class RequestTracker {
return this.graph.invalidNodeIds.size > 0;
}

getInvalidRequests(): Array<StoredRequest> {
getInvalidRequests(): Array<RequestNode> {
let invalidRequests = [];
for (let id of this.graph.invalidNodeIds) {
let node = nullthrows(this.graph.getNode(id));
invariant(node.type === 'request');
invalidRequests.push(node.value);
invalidRequests.push(node);
}
return invalidRequests;
}
Expand Down Expand Up @@ -990,7 +1009,9 @@ export default class RequestTracker {
requestId != null ? this.graph.getInvalidations(requestId) : [];
let {requestNodeId, deferred} = this.startRequest({
id: request.id,
type: request.type,
type: 'request',
requestType: request.type,
invalidateReason: INITIAL_BUILD,
});

let {api, subRequestContentKeys} = this.createAPI(
Expand All @@ -1000,7 +1021,6 @@ export default class RequestTracker {

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

let result = await request.run({
input: request.input,
api,
Expand Down Expand Up @@ -1094,15 +1114,15 @@ export default class RequestTracker {
continue;
}

let resultCacheKey = node.value.resultCacheKey;
if (resultCacheKey != null && node.value.result != null) {
let resultCacheKey = node.resultCacheKey;
if (resultCacheKey != null && node.result != null) {
promises.push(
this.options.cache.setLargeBlob(
resultCacheKey,
serialize(node.value.result),
serialize(node.result),
),
);
delete node.value.result;
delete node.result;
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/core/core/src/dumpGraphToGraphViz.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ export default async function dumpGraphToGraphViz(
} else if (node.type === 'asset_group') {
if (node.deferred) label += '(deferred)';
} else if (node.type === 'file') {
label += path.basename(node.value.filePath);
label += path.basename(node.id);
} else if (node.type === 'transformer_request') {
label +=
path.basename(node.value.filePath) +
Expand All @@ -178,7 +178,7 @@ export default async function dumpGraphToGraphViz(
if (parts.length) label += ' (' + parts.join(', ') + ')';
if (node.value.env) label += ` (${getEnvDescription(node.value.env)})`;
} else if (node.type === 'request') {
label = node.value.type + ':' + node.id;
label = node.requestType + ':' + node.id;
}
}
n.set('label', label);
Expand Down
6 changes: 4 additions & 2 deletions packages/core/core/src/requests/AssetGraphRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,9 @@ export class AssetGraphBuilder {
);

this.isSingleChangeRebuild =
api.getInvalidSubRequests().filter(req => req.type === 'asset_request')
.length === 1;
api
.getInvalidSubRequests()
.filter(req => req.requestType === 'asset_request').length === 1;
this.queue = new PromiseQueue();

assetGraph.onNodeRemoved = nodeId => {
Expand Down Expand Up @@ -482,6 +483,7 @@ export class AssetGraphBuilder {
optionsRef: this.optionsRef,
isSingleChangeRebuild: this.isSingleChangeRebuild,
});

let assets = await this.api.runRequest<AssetRequestInput, Array<Asset>>(
request,
{force: true},
Expand Down
2 changes: 1 addition & 1 deletion packages/core/core/src/requests/AssetRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ async function run({input, api, farm, invalidateReason, options}) {
await Promise.all(
api
.getSubRequests()
.filter(req => req.type === 'dev_dep_request')
.filter(req => req.requestType === 'dev_dep_request')
.map(async req => [
req.id,
nullthrows(await api.getRequestResult<DevDepRequest>(req.id)),
Expand Down
2 changes: 1 addition & 1 deletion packages/core/core/src/requests/DevDepRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export async function getDevDepRequests<TResult>(
await Promise.all(
api
.getSubRequests()
.filter(req => req.type === 'dev_dep_request')
.filter(req => req.requestType === 'dev_dep_request')
.map(async req => [
req.id,
nullthrows(await api.getRequestResult<DevDepRequest>(req.id)),
Expand Down
3 changes: 2 additions & 1 deletion packages/core/core/src/requests/ParcelConfigRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {optionsProxy} from '../utils';
import ParcelConfig from '../ParcelConfig';
import {createBuildCache} from '../buildCache';
import {toProjectPath} from '../projectPath';
import type {RequestType} from '../RequestTracker';

type ConfigMap<K, V> = {[K]: V, ...};

Expand All @@ -52,7 +53,7 @@ type RunOpts<TResult> = {|

export type ParcelConfigRequest = {|
id: string,
type: string,
type: RequestType,
input: null,
run: (RunOpts<ConfigAndCachePath>) => Async<ConfigAndCachePath>,
|};
Expand Down
5 changes: 5 additions & 0 deletions packages/core/core/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type {PackageManager} from '@parcel/package-manager';
import invariant from 'assert';
import baseX from 'base-x';
import {Graph} from '@parcel/graph';
import type {ContentKey} from '@parcel/graph';
import {hashObject} from '@parcel/utils';

import {registerSerializableClass} from './serializer';
Expand Down Expand Up @@ -235,3 +236,7 @@ export function toInternalSymbols<T: {|loc: ?SourceLocation|}>(
]),
);
}

export function keyFromEnvOrOptionContentKey(contentKey: ContentKey): string {
return contentKey.slice(contentKey.indexOf(':') + 1);
gorakong marked this conversation as resolved.
Show resolved Hide resolved
}
Loading
Loading