Skip to content

Commit

Permalink
Update (base update)
Browse files Browse the repository at this point in the history
[ghstack-poisoned]
  • Loading branch information
mofeiZ committed Sep 26, 2024
1 parent 5b19dc0 commit 4526deb
Show file tree
Hide file tree
Showing 48 changed files with 2,241 additions and 280 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ function* runWithEnvironment(
});
assertTerminalSuccessorsExist(hir);
assertTerminalPredsExist(hir);
if (env.config.enablePropagateDepsInHIR) {
if (env.config.enablePropagateDepsInHIR !== 'disabled') {
propagateScopeDependenciesHIR(hir);
yield log({
kind: 'hir',
Expand Down Expand Up @@ -378,7 +378,7 @@ function* runWithEnvironment(
});
assertScopeInstructionsWithinScopes(reactiveFunction);

if (!env.config.enablePropagateDepsInHIR) {
if (env.config.enablePropagateDepsInHIR === 'disabled') {
propagateScopeDependencies(reactiveFunction);
yield log({
kind: 'reactive',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
HIRFunction,
Identifier,
IdentifierId,
Place,
InstructionId,
ReactiveScopeDependency,
ScopeId,
} from './HIR';
Expand Down Expand Up @@ -66,7 +66,7 @@ export function collectHoistablePropertyLoads(
fn: HIRFunction,
temporaries: ReadonlyMap<IdentifierId, ReactiveScopeDependency>,
): ReadonlyMap<ScopeId, BlockInfo> {
const nodes = collectPropertyLoadsInBlocks(fn, temporaries);
const nodes = collectNonNullsInBlocks(fn, temporaries);
propagateNonNull(fn, nodes);

const nodesKeyedByScopeId = new Map<ScopeId, BlockInfo>();
Expand All @@ -87,61 +87,6 @@ export type BlockInfo = {
assumedNonNullObjects: ReadonlySet<PropertyLoadNode>;
};

export function getProperty(
object: Place,
propertyName: string,
temporaries: ReadonlyMap<IdentifierId, ReactiveScopeDependency>,
): ReactiveScopeDependency {
/*
* (1) Get the base object either from the temporary sidemap (e.g. a LoadLocal)
* or a deep copy of an existing property dependency.
* Example 1:
* $0 = LoadLocal x
* $1 = PropertyLoad $0.y
* getProperty($0, ...) -> resolvedObject = x, resolvedDependency = null
*
* Example 2:
* $0 = LoadLocal x
* $1 = PropertyLoad $0.y
* $2 = PropertyLoad $1.z
* getProperty($1, ...) -> resolvedObject = null, resolvedDependency = x.y
*
* Example 3:
* $0 = Call(...)
* $1 = PropertyLoad $0.y
* getProperty($0, ...) -> resolvedObject = null, resolvedDependency = null
*/
const resolvedDependency = temporaries.get(object.identifier.id);

/**
* (2) Push the last PropertyLoad
* TODO(mofeiZ): understand optional chaining
*/
let property: ReactiveScopeDependency;
if (resolvedDependency == null) {
property = {
identifier: object.identifier,
path: [{property: propertyName, optional: false}],
};
} else {
property = {
identifier: resolvedDependency.identifier,
path: [
...resolvedDependency.path,
{property: propertyName, optional: false},
],
};
}
return property;
}

export function resolveTemporary(
place: Place,
temporaries: ReadonlyMap<IdentifierId, Identifier>,
): Identifier {
return temporaries.get(place.identifier.id) ?? place.identifier;
}

/**
* Tree data structure to dedupe property loads (e.g. a.b.c)
* and make computing sets intersections simpler.
Expand All @@ -151,7 +96,7 @@ type RootNode = {
parent: null;
// Recorded to make later computations simpler
fullPath: ReactiveScopeDependency;
root: Identifier;
root: IdentifierId;
};

type PropertyLoadNode =
Expand All @@ -163,26 +108,26 @@ type PropertyLoadNode =
| RootNode;

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

#getOrCreateRoot(identifier: Identifier): PropertyLoadNode {
getOrCreateRoot(identifier: Identifier): PropertyLoadNode {
/**
* Reads from a statically scoped variable are always safe in JS,
* with the exception of TDZ (not addressed by this pass).
*/
let rootNode = this.roots.get(identifier);
let rootNode = this.roots.get(identifier.id);

if (rootNode === undefined) {
rootNode = {
root: identifier,
root: identifier.id,
properties: new Map(),
fullPath: {
identifier,
path: [],
},
parent: null,
};
this.roots.set(identifier, rootNode);
this.roots.set(identifier.id, rootNode);
}
return rootNode;
}
Expand All @@ -207,17 +152,15 @@ class Tree {
}

getPropertyLoadNode(n: ReactiveScopeDependency): PropertyLoadNode {
CompilerError.invariant(n.path.length > 0, {
reason:
'[CollectHoistablePropertyLoads] Expected property node, found root node',
loc: GeneratedSource,
});
/**
* 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.getOrCreateRoot(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));
}
Expand All @@ -226,10 +169,44 @@ class Tree {
}
}

function collectPropertyLoadsInBlocks(
function pushPropertyLoadNode(
node: PropertyLoadNode,
instrId: InstructionId,
knownImmutableIdentifiers: Set<IdentifierId>,
result: Set<PropertyLoadNode>,
): void {
const object = node.fullPath.identifier;
/**
* Since this runs *after* buildReactiveScopeTerminals, identifier mutable ranges
* are not valid with respect to current instruction id numbering.
* We use attached reactive scope ranges as a proxy for mutable range, but this
* is an overestimate as (1) scope ranges merge and align to form valid program
* blocks and (2) passes like MemoizeFbtAndMacroOperands may assign scopes to
* non-mutable identifiers.
*
* See comment at top of function for why we track known immutable identifiers.
*/
const isMutableAtInstr =
object.mutableRange.end > object.mutableRange.start + 1 &&
object.scope != null &&
inRange({id: instrId}, object.scope.range);
if (
!isMutableAtInstr ||
knownImmutableIdentifiers.has(node.fullPath.identifier.id)
) {
let curr: PropertyLoadNode | null = node;
while (curr != null) {
result.add(curr);
curr = curr.parent;
}
}
}

function collectNonNullsInBlocks(
fn: HIRFunction,
temporaries: ReadonlyMap<IdentifierId, ReactiveScopeDependency>,
): 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 @@ -238,53 +215,75 @@ function collectPropertyLoadsInBlocks(
* We track known immutable identifiers to reduce regressions (as PropagateScopeDeps
* is being rewritten to HIR).
*/
const knownImmutableIdentifiers = new Set<Identifier>();
const knownImmutableIdentifiers = new Set<IdentifierId>();
if (fn.fnType === 'Component' || fn.fnType === 'Hook') {
for (const p of fn.params) {
if (p.kind === 'Identifier') {
knownImmutableIdentifiers.add(p.identifier);
knownImmutableIdentifiers.add(p.identifier.id);
}
}
}
const tree = new Tree();
/**
* Known non-null objects such as functional component props can be safely
* read from any block.
*/
const knownNonNullIdentifiers = new Set<PropertyLoadNode>();
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));
}
const nodes = new Map<BlockId, BlockInfo>();
for (const [_, block] of fn.body.blocks) {
const assumedNonNullObjects = new Set<PropertyLoadNode>();
const assumedNonNullObjects = new Set<PropertyLoadNode>(
knownNonNullIdentifiers,
);
for (const instr of block.instructions) {
if (instr.value.kind === 'PropertyLoad') {
const property = getProperty(
instr.value.object,
instr.value.property,
temporaries,
const source = temporaries.get(instr.value.object.identifier.id) ?? {
identifier: instr.value.object.identifier,
path: [],
};
const propertyNode = tree.getPropertyLoadNode(source);
pushPropertyLoadNode(
propertyNode,
instr.id,
knownImmutableIdentifiers,
assumedNonNullObjects,
);
const propertyNode = tree.getPropertyLoadNode(property);
const object = instr.value.object.identifier;
/**
* Since this runs *after* buildReactiveScopeTerminals, identifier mutable ranges
* are not valid with respect to current instruction id numbering.
* We use attached reactive scope ranges as a proxy for mutable range, but this
* is an overestimate as (1) scope ranges merge and align to form valid program
* blocks and (2) passes like MemoizeFbtAndMacroOperands may assign scopes to
* non-mutable identifiers.
*
* See comment at top of function for why we track known immutable identifiers.
*/
const isMutableAtInstr =
object.mutableRange.end > object.mutableRange.start + 1 &&
object.scope != null &&
inRange(instr, object.scope.range);
if (
!isMutableAtInstr ||
knownImmutableIdentifiers.has(propertyNode.fullPath.identifier)
) {
let curr = propertyNode.parent;
while (curr != null) {
assumedNonNullObjects.add(curr);
curr = curr.parent;
}
} else if (
instr.value.kind === 'Destructure' &&
fn.env.config.enablePropagateDepsInHIR === 'enabled_with_optimizations'
) {
const source = instr.value.value.identifier.id;
const sourceNode = temporaries.get(source);
if (sourceNode != null) {
pushPropertyLoadNode(
tree.getPropertyLoadNode(sourceNode),
instr.id,
knownImmutableIdentifiers,
assumedNonNullObjects,
);
}
} else if (
instr.value.kind === 'ComputedLoad' &&
fn.env.config.enablePropagateDepsInHIR === 'enabled_with_optimizations'
) {
const source = instr.value.object.identifier.id;
const sourceNode = temporaries.get(source);
if (sourceNode != null) {
pushPropertyLoadNode(
tree.getPropertyLoadNode(sourceNode),
instr.id,
knownImmutableIdentifiers,
assumedNonNullObjects,
);
}
}
// TODO handle destructuring
}

nodes.set(block.id, {
Expand Down Expand Up @@ -449,10 +448,11 @@ function propagateNonNull(
);

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,9 @@ const EnvironmentConfigSchema = z.object({
*/
enableUseTypeAnnotations: z.boolean().default(false),

enablePropagateDepsInHIR: z.boolean().default(false),
enablePropagateDepsInHIR: z
.enum(['disabled', 'enabled_baseline', 'enabled_with_optimizations'])
.default('disabled'),

/**
* Enables inference of optional dependency chains. Without this flag
Expand Down
Loading

0 comments on commit 4526deb

Please sign in to comment.