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

Incremental Symbol Propagation #8723

Merged
merged 47 commits into from
Mar 26, 2023
Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
8be1197
ReadOnlySet in utils
mischnic Dec 25, 2022
1e6f884
Speed up integration tests
mischnic Dec 27, 2022
6ddd315
Cleanup
mischnic Dec 16, 2022
eb82d90
Fix invalid fixtures in incremental-bundling test
mischnic Dec 28, 2022
a916af6
Set fallback symbol for empty assets (dev)
mischnic Dec 26, 2022
5b7c485
Add star symbol to dynamic imports in dev
mischnic Dec 27, 2022
19a5ba3
Always run symbol propagation
mischnic Dec 17, 2022
339477d
Always run symbol propagation: adjust tests
mischnic Dec 26, 2022
7ef34c0
Split out SymbolPropagation.js
mischnic Dec 24, 2022
da35108
Incremental propagate down
mischnic Dec 24, 2022
a36b95f
Incremental propagate up comment
mischnic Dec 24, 2022
b64cfef
comments
mischnic Dec 25, 2022
623a41b
setEqual
mischnic Dec 25, 2022
b8fc88b
wip
mischnic Dec 26, 2022
334cea0
Unit tests
mischnic Dec 26, 2022
decd941
Cache queues on error
mischnic Dec 27, 2022
01772ee
Fix for single-asset library builds
mischnic Dec 28, 2022
879635d
Cleanup
mischnic Dec 28, 2022
fd90843
Run tree up traversal on large rebuilds
mischnic Dec 28, 2022
c441fd2
Fix unit tests on Windows
mischnic Dec 29, 2022
ed42205
Move back into AssetGraphRequest for now
mischnic Dec 31, 2022
26b9d94
Cleanup
mischnic Dec 31, 2022
c63a516
Cleanup
mischnic Dec 31, 2022
62384ac
Merge branch 'v2' into symbol-propagation-changedAssets
mischnic Jan 7, 2023
c38a1da
Merge branch 'v2' into symbol-propagation-changedAssets
mischnic Jan 18, 2023
e38c9f6
Merge branch 'v2' into symbol-propagation-changedAssets
mischnic Feb 26, 2023
28ec95e
Rename to assetGroupsWithRemovedParents
mischnic Feb 26, 2023
00dc701
changedAssetsPropagation
mischnic Feb 26, 2023
812bce3
Fix
mischnic Feb 27, 2023
bb8de92
Fixup test
mischnic Feb 27, 2023
3f0ca1c
Cleanup tests
mischnic Feb 28, 2023
7be47ad
Add test
mischnic Feb 28, 2023
4781c73
Add test for error on removing export
mischnic Feb 28, 2023
a99c840
Fix up-traversal when changing exports
mischnic Feb 28, 2023
3390e15
Dirty flags in graph dump
mischnic Feb 28, 2023
af58fc1
Don't retarget dependencies in dev mode
mischnic Feb 28, 2023
5ad82aa
Copy over the whole node in incremental
mischnic Feb 28, 2023
4b47954
Fixup unit test
mischnic Feb 28, 2023
00f6cec
Merge branch 'v2' into symbol-propagation-changedAssets
mischnic Mar 1, 2023
2e1bfb3
Add test to ignore unused imports in dev
mischnic Mar 1, 2023
2c4e52e
Only register used imports in dev
mischnic Mar 1, 2023
166a043
Better handle cleared incoming deps
mischnic Mar 1, 2023
13fbfd4
Fixup used import detection in dev
mischnic Mar 2, 2023
d4209ef
Merge branch 'v2' into symbol-propagation-changedAssets
mischnic Mar 24, 2023
795fa9b
Add comments
mischnic Mar 26, 2023
d7d0d01
Add comment about down revisiting
mischnic Mar 26, 2023
4076990
Merge branch 'v2' into symbol-propagation-changedAssets
mischnic Mar 26, 2023
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
23 changes: 3 additions & 20 deletions packages/core/core/src/AssetGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,12 @@ type InitOpts = {|

type AssetGraphOpts = {|
...ContentGraphOpts<AssetGraphNode>,
symbolPropagationRan: boolean,
hash?: ?string,
|};

type SerializedAssetGraph = {|
...SerializedContentGraph<AssetGraphNode>,
hash?: ?string,
symbolPropagationRan: boolean,
|};

export function nodeFromDep(dep: Dependency): DependencyNode {
Expand Down Expand Up @@ -113,15 +111,13 @@ export default class AssetGraph extends ContentGraph<AssetGraphNode> {
onNodeRemoved: ?(nodeId: NodeId) => mixed;
hash: ?string;
envCache: Map<string, Environment>;
symbolPropagationRan: boolean;
safeToIncrementallyBundle: boolean = true;

constructor(opts: ?AssetGraphOpts) {
if (opts) {
let {hash, symbolPropagationRan, ...rest} = opts;
let {hash, ...rest} = opts;
super(rest);
this.hash = hash;
this.symbolPropagationRan = symbolPropagationRan;
} else {
super();
this.setRootNodeId(
Expand All @@ -133,7 +129,6 @@ export default class AssetGraph extends ContentGraph<AssetGraphNode> {
);
}
this.envCache = new Map();
this.symbolPropagationRan = false;
}

// $FlowFixMe[prop-missing]
Expand All @@ -146,7 +141,6 @@ export default class AssetGraph extends ContentGraph<AssetGraphNode> {
return {
...super.serialize(),
hash: this.hash,
symbolPropagationRan: this.symbolPropagationRan,
};
}

Expand Down Expand Up @@ -199,19 +193,6 @@ export default class AssetGraph extends ContentGraph<AssetGraphNode> {
removeNode(nodeId: NodeId): void {
this.hash = null;
this.onNodeRemoved && this.onNodeRemoved(nodeId);
// This needs to mark all connected nodes that doesn't become orphaned
// due to replaceNodesConnectedTo to make sure that the symbols of
// nodes from which at least one parent was removed are updated.
let node = nullthrows(this.getNode(nodeId));
if (this.isOrphanedNode(nodeId) && node.type === 'dependency') {
let children = this.getNodeIdsConnectedFrom(nodeId).map(id =>
nullthrows(this.getNode(id)),
);
for (let n of children) {
invariant(n.type === 'asset_group' || n.type === 'asset');
n.usedSymbolsDownDirty = true;
}
}
return super.removeNode(nodeId);
}

Expand Down Expand Up @@ -258,6 +239,7 @@ export default class AssetGraph extends ContentGraph<AssetGraphNode> {
if (node.value.env.isLibrary) {
// in library mode, all of the entry's symbols are "used"
node.usedSymbolsDown.add('*');
node.usedSymbolsUp.set('*', undefined);
}
return node;
});
Expand Down Expand Up @@ -526,6 +508,7 @@ export default class AssetGraph extends ContentGraph<AssetGraphNode> {
depNodeIds.push(this.addNode(depNode));
}

assetNode.usedSymbolsUpDirty = true;
assetNode.usedSymbolsDownDirty = true;
this.replaceNodeIdsConnectedTo(
this.getNodeIdByContentKey(assetNode.id),
Expand Down
31 changes: 11 additions & 20 deletions packages/core/core/src/BundleGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import type {
} from './types';
import type AssetGraph from './AssetGraph';
import type {ProjectPath} from './projectPath';
import {nodeFromAsset} from './AssetGraph';

import assert from 'assert';
import invariant from 'assert';
Expand Down Expand Up @@ -88,7 +87,6 @@ type BundleGraphOpts = {|
bundleContentHashes: Map<string, string>,
assetPublicIds: Set<string>,
publicIdByAssetId: Map<string, string>,
symbolPropagationRan: boolean,
|};

type SerializedBundleGraph = {|
Expand All @@ -97,7 +95,6 @@ type SerializedBundleGraph = {|
bundleContentHashes: Map<string, string>,
assetPublicIds: Set<string>,
publicIdByAssetId: Map<string, string>,
symbolPropagationRan: boolean,
|};

function makeReadOnlySet<T>(set: Set<T>): $ReadOnlySet<T> {
Expand Down Expand Up @@ -137,27 +134,23 @@ export default class BundleGraph {
_targetEntryRoots: Map<ProjectPath, FilePath> = new Map();
/** The internal core Graph structure */
_graph: ContentGraph<BundleGraphNode, BundleGraphEdgeType>;
_symbolPropagationRan /*: boolean*/;
_bundlePublicIds /*: Set<string> */ = new Set<string>();

constructor({
graph,
publicIdByAssetId,
assetPublicIds,
bundleContentHashes,
symbolPropagationRan,
}: {|
graph: ContentGraph<BundleGraphNode, BundleGraphEdgeType>,
publicIdByAssetId: Map<string, string>,
assetPublicIds: Set<string>,
bundleContentHashes: Map<string, string>,
symbolPropagationRan: boolean,
|}) {
this._graph = graph;
this._assetPublicIds = assetPublicIds;
this._publicIdByAssetId = publicIdByAssetId;
this._bundleContentHashes = bundleContentHashes;
this._symbolPropagationRan = symbolPropagationRan;
}

/**
Expand All @@ -166,6 +159,7 @@ export default class BundleGraph {
*/
static fromAssetGraph(
assetGraph: AssetGraph,
isProduction: boolean,
publicIdByAssetId: Map<string, string> = new Map(),
assetPublicIds: Set<string> = new Set(),
): BundleGraph {
Expand Down Expand Up @@ -207,7 +201,9 @@ export default class BundleGraph {
if (
node.type === 'dependency' &&
node.value.symbols != null &&
node.value.env.shouldScopeHoist
node.value.env.shouldScopeHoist &&
// Disable in dev mode because this feature is at odds with safeToIncrementallyBundle
isProduction
) {
// asset -> symbols that should be imported directly from that asset
let targets = new DefaultMap<ContentKey, Map<Symbol, Symbol>>(
Expand Down Expand Up @@ -384,7 +380,6 @@ export default class BundleGraph {
assetPublicIds,
bundleContentHashes: new Map(),
publicIdByAssetId,
symbolPropagationRan: assetGraph.symbolPropagationRan,
});
}

Expand All @@ -395,7 +390,6 @@ export default class BundleGraph {
assetPublicIds: this._assetPublicIds,
bundleContentHashes: this._bundleContentHashes,
publicIdByAssetId: this._publicIdByAssetId,
symbolPropagationRan: this._symbolPropagationRan,
};
}

Expand All @@ -405,7 +399,6 @@ export default class BundleGraph {
assetPublicIds: serialized.assetPublicIds,
bundleContentHashes: serialized.bundleContentHashes,
publicIdByAssetId: serialized.publicIdByAssetId,
symbolPropagationRan: serialized.symbolPropagationRan,
});
}

Expand Down Expand Up @@ -1994,16 +1987,17 @@ export default class BundleGraph {
getUsedSymbolsAsset(asset: Asset): ?$ReadOnlySet<Symbol> {
let node = this._graph.getNodeByContentKey(asset.id);
invariant(node && node.type === 'asset');
return this._symbolPropagationRan
return node.value.symbols
? makeReadOnlySet(new Set(node.usedSymbols.keys()))
: null;
}

getUsedSymbolsDependency(dep: Dependency): ?$ReadOnlySet<Symbol> {
let node = this._graph.getNodeByContentKey(dep.id);
invariant(node && node.type === 'dependency');
let result = new Set(node.usedSymbolsUp.keys());
return this._symbolPropagationRan ? makeReadOnlySet(result) : null;
return node.value.symbols
? makeReadOnlySet(new Set(node.usedSymbolsUp.keys()))
: null;
}

merge(other: BundleGraph) {
Expand Down Expand Up @@ -2069,12 +2063,9 @@ export default class BundleGraph {
/**
* Update the asset in a Bundle Graph and clear the associated Bundle hash.
*/
updateAsset(asset: Asset) {
this._graph.updateNode(
this._graph.getNodeIdByContentKey(asset.id),
nodeFromAsset(asset),
);
let bundles = this.getBundlesWithAsset(asset);
updateAsset(asset: AssetNode) {
this._graph.updateNode(this._graph.getNodeIdByContentKey(asset.id), asset);
let bundles = this.getBundlesWithAsset(asset.value);
for (let bundle of bundles) {
// the bundle content will change with a modified asset
this._bundleContentHashes.delete(bundle.id);
Expand Down
2 changes: 1 addition & 1 deletion packages/core/core/src/Dependency.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ type DependencyOpts = {|
resolveFrom?: FilePath,
range?: SemverRange,
target?: Target,
symbols?: Map<
symbols?: ?Map<
Symbol,
{|local: Symbol, loc: ?SourceLocation, isWeak: boolean, meta?: ?Meta|},
>,
Expand Down
43 changes: 43 additions & 0 deletions packages/core/core/src/SymbolPropagation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// @flow

import type {Diagnostic} from '@parcel/diagnostic';
import type {NodeId} from '@parcel/graph';
import type {ParcelOptions} from './types';

import {type default as AssetGraph} from './AssetGraph';
import {AssetGraphBuilder} from './requests/AssetGraphRequest';

export function propagateSymbols({
mischnic marked this conversation as resolved.
Show resolved Hide resolved
options,
assetGraph,
changedAssetsPropagation,
assetGroupsWithRemovedParents,
previousErrors,
}: {|
options: ParcelOptions,
assetGraph: AssetGraph,
changedAssetsPropagation: Set<string>,
assetGroupsWithRemovedParents: Set<NodeId>,
previousErrors?: ?Map<NodeId, Array<Diagnostic>>,
|}): Map<NodeId, Array<Diagnostic>> {
// TODO move functions from AssetGraphRequest to here

let builder = new AssetGraphBuilder({
input: ({} /*: any */),
farm: ({} /*: any */),
invalidateReason: ({} /*: any */),
// $FlowFixMe
api: {getInvalidSubRequests: () => []},
options,
});
builder.assetGraph = assetGraph;
builder.changedAssetsPropagation = changedAssetsPropagation;

return builder.propagateSymbols({
options: builder.options,
assetGraph: builder.assetGraph,
changedAssetsPropagation: builder.changedAssetsPropagation,
assetGroupsWithRemovedParents: assetGroupsWithRemovedParents,
previousErrors: previousErrors,
});
}
1 change: 1 addition & 0 deletions packages/core/core/src/applyRuntimes.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ export default async function applyRuntimes<TResult>({

let runtimesGraph = InternalBundleGraph.fromAssetGraph(
runtimesAssetGraph,
options.mode === 'production',
bundleGraph._publicIdByAssetId,
bundleGraph._assetPublicIds,
);
Expand Down
6 changes: 6 additions & 0 deletions packages/core/core/src/dumpGraphToGraphViz.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,10 @@ export default async function dumpGraphToGraphViz(
label +=
'\\nusedSymbolsDown: ' + [...node.usedSymbolsDown].join(',');
}
// if (node.usedSymbolsDownDirty) label += '\\nusedSymbolsDownDirty';
// if (node.usedSymbolsUpDirtyDown)
// label += '\\nusedSymbolsUpDirtyDown';
// if (node.usedSymbolsUpDirtyUp) label += '\\nusedSymbolsUpDirtyUp';
} else {
label += '\\nsymbols: cleared';
}
Expand All @@ -149,6 +153,8 @@ export default async function dumpGraphToGraphViz(
if (node.usedSymbols.size) {
label += '\\nusedSymbols: ' + [...node.usedSymbols].join(',');
}
// if (node.usedSymbolsDownDirty) label += '\\nusedSymbolsDownDirty';
// if (node.usedSymbolsUpDirty) label += '\\nusedSymbolsUpDirty';
} else {
label += '\\nsymbols: cleared';
}
Expand Down
Loading