From 2b47cac5fdf3e96f2e69d9081f5a03db4a2c97a2 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Tue, 14 Feb 2023 14:09:53 -0800 Subject: [PATCH] Create a separate pass to prune non-reactive dependencies The fact that InferReactiveIdentifiers is integrated directly into PropagateScopeDependencies has made the latter pretty tricky to debug at times. If a dependency is missing, we have to introspect and figure out if that's because it was somehow inferred as non-reactive. This PR creates a new PruneNonReactiveDependencies pass to separate out these phases. --- compiler/forget/src/CompilerPipeline.ts | 8 ++++ .../PropagateScopeDependencies.ts | 16 +------- .../PruneNonReactiveDependencies.ts | 41 +++++++++++++++++++ 3 files changed, 51 insertions(+), 14 deletions(-) create mode 100644 compiler/forget/src/ReactiveScopes/PruneNonReactiveDependencies.ts diff --git a/compiler/forget/src/CompilerPipeline.ts b/compiler/forget/src/CompilerPipeline.ts index cbd44e26b7..0422d38496 100644 --- a/compiler/forget/src/CompilerPipeline.ts +++ b/compiler/forget/src/CompilerPipeline.ts @@ -35,6 +35,7 @@ import { renameVariables, } from "./ReactiveScopes"; import { flattenScopesWithHooks } from "./ReactiveScopes/FlattenScopesWithHooks"; +import { pruneNonReactiveDependencies } from "./ReactiveScopes/PruneNonReactiveDependencies"; import { eliminateRedundantPhi, enterSSA, leaveSSA } from "./SSA"; import { inferTypes } from "./TypeInference"; import { logHIRFunction, logReactiveFunction } from "./Utils/logger"; @@ -137,6 +138,13 @@ export function* run( value: reactiveFunction, }); + pruneNonReactiveDependencies(reactiveFunction); + yield log({ + kind: "reactive", + name: "PruneNonReactiveDependencies", + value: reactiveFunction, + }); + pruneUnusedScopes(reactiveFunction); yield log({ kind: "reactive", diff --git a/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts b/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts index 4a08286b9d..7a95f7e1c9 100644 --- a/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts +++ b/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts @@ -22,7 +22,6 @@ import { } from "../HIR/HIR"; import { eachInstructionValueOperand } from "../HIR/visitors"; import { assertExhaustive } from "../Utils/utils"; -import { inferReactiveIdentifiers } from "./InferReactiveIdentifiers"; import { eachReactiveValueOperand } from "./visitors"; /** @@ -32,7 +31,7 @@ import { eachReactiveValueOperand } from "./visitors"; * their direct dependencies and those of their child scopes. */ export function propagateScopeDependencies(fn: ReactiveFunction): void { - const context = new Context(inferReactiveIdentifiers(fn)); + const context = new Context(); if (fn.id !== null) { context.declare(fn.id, { kind: DeclKind.Const, @@ -67,16 +66,12 @@ type Scopes = Array; class Context { #declarations: DeclMap = new Map(); #dependencies: Set = new Set(); - #reactiveIdentifiers: Set; // Produces a de-duplicated mapping of Id -> ReactiveScopeDependency // This helps with.. temporaries that are created only for property loads // but can be generalized to all non-allocating temporaries #properties: Map = new Map(); #scopes: Scopes = []; - constructor(reactiveIdentifiers: Set) { - this.#reactiveIdentifiers = reactiveIdentifiers; - } enter(scope: ReactiveScope, fn: () => void): Set { const previousDependencies = this.#dependencies; const scopedDependencies = new Set(); @@ -120,10 +115,6 @@ class Context { return this.#scopes.at(-1) ?? null; } - isReactive(id: Identifier): boolean { - return this.#reactiveIdentifiers.has(id); - } - visitOperand(place: Place): void { this.visitDependency({ place, path: null }); } @@ -383,11 +374,8 @@ function visitInstruction(context: Context, instr: ReactiveInstruction): void { if (lvalue.kind === InstructionKind.Reassign) { context.visitReassignment(lvalue); } else { - const kind = context.isReactive(lvalue.place.identifier) - ? DeclKind.Dynamic - : DeclKind.Const; context.declare(lvalue.place.identifier, { - kind, + kind: DeclKind.Dynamic, id: lvalue.place.identifier.mutableRange.start, scope: context.currentScope, }); diff --git a/compiler/forget/src/ReactiveScopes/PruneNonReactiveDependencies.ts b/compiler/forget/src/ReactiveScopes/PruneNonReactiveDependencies.ts new file mode 100644 index 0000000000..c3c1449b10 --- /dev/null +++ b/compiler/forget/src/ReactiveScopes/PruneNonReactiveDependencies.ts @@ -0,0 +1,41 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import { Identifier, ReactiveFunction, ReactiveScopeBlock } from "../HIR"; +import { inferReactiveIdentifiers } from "./InferReactiveIdentifiers"; +import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors"; + +/** + * PropagateScopeDependencies infers dependencies without considering whether dependencies + * are actually reactive or not (ie, whether their value can change over time). + * + * This pass prunes dependencies that are guaranteed to be non-reactive. + */ +export function pruneNonReactiveDependencies(fn: ReactiveFunction): void { + const state = inferReactiveIdentifiers(fn); + visitReactiveFunction(fn, new Visitor(), state); +} + +type State = Set; + +class Visitor extends ReactiveFunctionVisitor { + override visitScope(scope: ReactiveScopeBlock, state: State): void { + this.traverseScope(scope, state); + for (const dep of scope.scope.dependencies) { + const isReactive = state.has(dep.place.identifier); + if (!isReactive) { + scope.scope.dependencies.delete(dep); + } + } + // If a scope now has no dependencies, then its declarations are all non-reactive + if (scope.scope.dependencies.size === 0) { + for (const [, declaration] of scope.scope.declarations) { + state.delete(declaration); + } + } + } +}