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

[compiler] Renames and no-op refactor for next PR #31036

Merged
merged 6 commits into from
Sep 30, 2024
Merged

[compiler] Renames and no-op refactor for next PR #31036

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2500367
Update
mofeiZ Sep 23, 2024
71aa14e
Update
mofeiZ Sep 25, 2024
48a9df2
Update
mofeiZ Sep 25, 2024
e5e5d6b
Update
mofeiZ Sep 26, 2024
b90b8b0
Update
mofeiZ Sep 26, 2024
7d77a79
Update
mofeiZ Sep 30, 2024
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
150 changes: 64 additions & 86 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {Set_intersect, Set_union, getOrInsertDefault} from '../Utils/utils';
import {
BasicBlock,
BlockId,
DependencyPathEntry,
GeneratedSource,
HIRFunction,
Identifier,
Expand Down Expand Up @@ -66,7 +67,9 @@ export function collectHoistablePropertyLoads(
fn: HIRFunction,
temporaries: ReadonlyMap<IdentifierId, ReactiveScopeDependency>,
): ReadonlyMap<ScopeId, BlockInfo> {
const nodes = collectNonNullsInBlocks(fn, temporaries);
const registry = new PropertyPathRegistry();

const nodes = collectNonNullsInBlocks(fn, temporaries, registry);
propagateNonNull(fn, nodes);

const nodesKeyedByScopeId = new Map<ScopeId, BlockInfo>();
Expand All @@ -84,33 +87,33 @@ export function collectHoistablePropertyLoads(

export type BlockInfo = {
block: BasicBlock;
assumedNonNullObjects: ReadonlySet<PropertyLoadNode>;
assumedNonNullObjects: ReadonlySet<PropertyPathNode>;
};

/**
* Tree data structure to dedupe property loads (e.g. a.b.c)
* PropertyLoadRegistry data structure to dedupe property loads (e.g. a.b.c)
* and make computing sets intersections simpler.
*/
type RootNode = {
properties: Map<string, PropertyLoadNode>;
properties: Map<string, PropertyPathNode>;
parent: null;
// Recorded to make later computations simpler
fullPath: ReactiveScopeDependency;
root: IdentifierId;
};

type PropertyLoadNode =
type PropertyPathNode =
| {
properties: Map<string, PropertyLoadNode>;
parent: PropertyLoadNode;
properties: Map<string, PropertyPathNode>;
parent: PropertyPathNode;
fullPath: ReactiveScopeDependency;
}
| RootNode;

class Tree {
class PropertyPathRegistry {
roots: Map<IdentifierId, RootNode> = new Map();

getOrCreateRoot(identifier: Identifier): PropertyLoadNode {
getOrCreateIdentifier(identifier: Identifier): PropertyPathNode {
/**
* Reads from a statically scoped variable are always safe in JS,
* with the exception of TDZ (not addressed by this pass).
Expand All @@ -132,48 +135,60 @@ class Tree {
return rootNode;
}

static #getOrCreateProperty(
node: PropertyLoadNode,
property: string,
): PropertyLoadNode {
let child = node.properties.get(property);
static getOrCreatePropertyEntry(
parent: PropertyPathNode,
entry: DependencyPathEntry,
): PropertyPathNode {
if (entry.optional) {
CompilerError.throwTodo({
reason: 'handle optional nodes',
loc: GeneratedSource,
});
}
let child = parent.properties.get(entry.property);
if (child == null) {
child = {
properties: new Map(),
parent: node,
parent: parent,
fullPath: {
identifier: node.fullPath.identifier,
path: node.fullPath.path.concat([{property, optional: false}]),
identifier: parent.fullPath.identifier,
path: parent.fullPath.path.concat(entry),
},
};
node.properties.set(property, child);
parent.properties.set(entry.property, child);
}
return child;
}

getPropertyLoadNode(n: ReactiveScopeDependency): PropertyLoadNode {
getOrCreateProperty(n: ReactiveScopeDependency): PropertyPathNode {
/**
* We add ReactiveScopeDependencies according to instruction ordering,
* so all subpaths of a PropertyLoad should already exist
* (e.g. a.b is added before a.b.c),
*/
let currNode = this.getOrCreateRoot(n.identifier);
let currNode = this.getOrCreateIdentifier(n.identifier);
if (n.path.length === 0) {
return currNode;
}
for (let i = 0; i < n.path.length - 1; i++) {
currNode = assertNonNull(currNode.properties.get(n.path[i].property));
currNode = PropertyPathRegistry.getOrCreatePropertyEntry(
currNode,
n.path[i],
);
}

return Tree.#getOrCreateProperty(currNode, n.path.at(-1)!.property);
return PropertyPathRegistry.getOrCreatePropertyEntry(
currNode,
n.path.at(-1)!,
);
}
}

function pushPropertyLoadNode(
node: PropertyLoadNode,
function addNonNullPropertyPath(
node: PropertyPathNode,
instrId: InstructionId,
knownImmutableIdentifiers: Set<IdentifierId>,
result: Set<PropertyLoadNode>,
result: Set<PropertyPathNode>,
): void {
const object = node.fullPath.identifier;
/**
Expand All @@ -194,19 +209,15 @@ function pushPropertyLoadNode(
!isMutableAtInstr ||
knownImmutableIdentifiers.has(node.fullPath.identifier.id)
) {
let curr: PropertyLoadNode | null = node;
while (curr != null) {
result.add(curr);
curr = curr.parent;
}
result.add(node);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Trust that all previous property loads were added correctly. If a property load chain ever spans multiple blocks, this is technically the correct behavior

}
}

function collectNonNullsInBlocks(
fn: HIRFunction,
temporaries: ReadonlyMap<IdentifierId, ReactiveScopeDependency>,
registry: PropertyPathRegistry,
): ReadonlyMap<BlockId, BlockInfo> {
const tree = new Tree();
/**
* Due to current limitations of mutable range inference, there are edge cases in
* which we infer known-immutable values (e.g. props or hook params) to have a
Expand All @@ -227,19 +238,19 @@ function collectNonNullsInBlocks(
* Known non-null objects such as functional component props can be safely
* read from any block.
*/
const knownNonNullIdentifiers = new Set<PropertyLoadNode>();
const knownNonNullIdentifiers = new Set<PropertyPathNode>();
if (
fn.env.config.enablePropagateDepsInHIR === 'enabled_with_optimizations' &&
fn.fnType === 'Component' &&
fn.params.length > 0 &&
fn.params[0].kind === 'Identifier'
) {
const identifier = fn.params[0].identifier;
knownNonNullIdentifiers.add(tree.getOrCreateRoot(identifier));
knownNonNullIdentifiers.add(registry.getOrCreateIdentifier(identifier));
}
const nodes = new Map<BlockId, BlockInfo>();
for (const [_, block] of fn.body.blocks) {
const assumedNonNullObjects = new Set<PropertyLoadNode>(
const assumedNonNullObjects = new Set<PropertyPathNode>(
knownNonNullIdentifiers,
);
for (const instr of block.instructions) {
Expand All @@ -248,8 +259,8 @@ function collectNonNullsInBlocks(
identifier: instr.value.object.identifier,
path: [],
};
const propertyNode = tree.getPropertyLoadNode(source);
pushPropertyLoadNode(
const propertyNode = registry.getOrCreateProperty(source);
addNonNullPropertyPath(
propertyNode,
instr.id,
knownImmutableIdentifiers,
Expand All @@ -262,8 +273,8 @@ function collectNonNullsInBlocks(
const source = instr.value.value.identifier.id;
const sourceNode = temporaries.get(source);
if (sourceNode != null) {
pushPropertyLoadNode(
tree.getPropertyLoadNode(sourceNode),
addNonNullPropertyPath(
registry.getOrCreateProperty(sourceNode),
instr.id,
knownImmutableIdentifiers,
assumedNonNullObjects,
Expand All @@ -276,8 +287,8 @@ function collectNonNullsInBlocks(
const source = instr.value.object.identifier.id;
const sourceNode = temporaries.get(source);
if (sourceNode != null) {
pushPropertyLoadNode(
tree.getPropertyLoadNode(sourceNode),
addNonNullPropertyPath(
registry.getOrCreateProperty(sourceNode),
instr.id,
knownImmutableIdentifiers,
assumedNonNullObjects,
Expand Down Expand Up @@ -319,7 +330,6 @@ function propagateNonNull(
nodeId: BlockId,
direction: 'forward' | 'backward',
traversalState: Map<BlockId, 'active' | 'done'>,
nonNullObjectsByBlock: Map<BlockId, ReadonlySet<PropertyLoadNode>>,
): boolean {
/**
* Avoid re-visiting computed or currently active nodes, which can
Expand Down Expand Up @@ -350,7 +360,6 @@ function propagateNonNull(
pred,
direction,
traversalState,
nonNullObjectsByBlock,
);
changed ||= neighborChanged;
}
Expand Down Expand Up @@ -379,38 +388,36 @@ function propagateNonNull(
const neighborAccesses = Set_intersect(
Array.from(neighbors)
.filter(n => traversalState.get(n) === 'done')
.map(n => assertNonNull(nonNullObjectsByBlock.get(n))),
.map(n => assertNonNull(nodes.get(n)).assumedNonNullObjects),
);

const prevObjects = assertNonNull(nonNullObjectsByBlock.get(nodeId));
const newObjects = Set_union(prevObjects, neighborAccesses);
const prevObjects = assertNonNull(nodes.get(nodeId)).assumedNonNullObjects;
const mergedObjects = Set_union(prevObjects, neighborAccesses);

nonNullObjectsByBlock.set(nodeId, newObjects);
assertNonNull(nodes.get(nodeId)).assumedNonNullObjects = mergedObjects;
traversalState.set(nodeId, 'done');
changed ||= prevObjects.size !== newObjects.size;
changed ||= prevObjects.size !== mergedObjects.size;
return changed;
}
const fromEntry = new Map<BlockId, ReadonlySet<PropertyLoadNode>>();
const fromExit = new Map<BlockId, ReadonlySet<PropertyLoadNode>>();
for (const [blockId, blockInfo] of nodes) {
fromEntry.set(blockId, blockInfo.assumedNonNullObjects);
fromExit.set(blockId, blockInfo.assumedNonNullObjects);
}
const traversalState = new Map<BlockId, 'done' | 'active'>();
const reversedBlocks = [...fn.body.blocks];
reversedBlocks.reverse();

let i = 0;
let changed;
let i = 0;
do {
i++;
CompilerError.invariant(i++ < 100, {
reason:
'[CollectHoistablePropertyLoads] fixed point iteration did not terminate after 100 loops',
loc: GeneratedSource,
});

changed = false;
for (const [blockId] of fn.body.blocks) {
const forwardChanged = recursivelyPropagateNonNull(
blockId,
'forward',
traversalState,
fromEntry,
);
changed ||= forwardChanged;
}
Expand All @@ -420,43 +427,14 @@ function propagateNonNull(
blockId,
'backward',
traversalState,
fromExit,
);
changed ||= backwardChanged;
}
traversalState.clear();
} while (changed);

/**
* TODO: validate against meta internal code, then remove in future PR.
* Currently cannot come up with a case that requires fixed-point iteration.
*/
CompilerError.invariant(i <= 2, {
reason: 'require fixed-point iteration',
description: `#iterations = ${i}`,
loc: GeneratedSource,
});

CompilerError.invariant(
fromEntry.size === fromExit.size && fromEntry.size === nodes.size,
{
reason:
'bad sizes after calculating fromEntry + fromExit ' +
`${fromEntry.size} ${fromExit.size} ${nodes.size}`,
loc: GeneratedSource,
},
);

for (const [id, node] of nodes) {
const assumedNonNullObjects = Set_union(
assertNonNull(fromEntry.get(id)),
assertNonNull(fromExit.get(id)),
);
node.assumedNonNullObjects = assumedNonNullObjects;
}
}

function assertNonNull<T extends NonNullable<U>, U>(
export function assertNonNull<T extends NonNullable<U>, U>(
value: T | null | undefined,
source?: string,
): T {
Expand Down
25 changes: 16 additions & 9 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,17 @@ const ENABLE_DEBUG_INVARIANTS = true;
export class ReactiveScopeDependencyTreeHIR {
#roots: Map<Identifier, DependencyNode> = new Map();

#getOrCreateRoot(identifier: Identifier, isNonNull: boolean): DependencyNode {
#getOrCreateRoot(
identifier: Identifier,
accessType: PropertyAccessType,
): DependencyNode {
// roots can always be accessed unconditionally in JS
let rootNode = this.#roots.get(identifier);

if (rootNode === undefined) {
rootNode = {
properties: new Map(),
accessType: isNonNull
? PropertyAccessType.NonNullAccess
: PropertyAccessType.Access,
accessType,
};
this.#roots.set(identifier, rootNode);
}
Expand All @@ -37,16 +38,19 @@ export class ReactiveScopeDependencyTreeHIR {

addDependency(dep: ReactiveScopePropertyDependency): void {
const {path} = dep;
let currNode = this.#getOrCreateRoot(dep.identifier, false);
let currNode = this.#getOrCreateRoot(dep.identifier, MIN_ACCESS_TYPE);

const accessType = PropertyAccessType.Access;

currNode.accessType = merge(currNode.accessType, accessType);

for (const property of path) {
// all properties read 'on the way' to a dependency are marked as 'access'
let currChild = getOrMakeProperty(currNode, property.property);
currChild.accessType = merge(currChild.accessType, accessType);
let currChild = makeOrMergeProperty(
currNode,
property.property,
accessType,
);
currNode = currChild;
}

Expand Down Expand Up @@ -251,17 +255,20 @@ function printSubtree(
return results;
}

function getOrMakeProperty(
function makeOrMergeProperty(
node: DependencyNode,
property: string,
accessType: PropertyAccessType,
): DependencyNode {
let child = node.properties.get(property);
if (child == null) {
child = {
properties: new Map(),
accessType: MIN_ACCESS_TYPE,
accessType,
};
node.properties.set(property, child);
} else {
child.accessType = merge(child.accessType, accessType);
}
return child;
}
Loading