@samitouri / QOS-React-2 / commits / 5e95967c7c

[be][dependencies] Remove control flow info from PropertyLoad sidemap

--- We don't need to store whether a `PropertyLoad` happens within a conditional (within its reactive scope). In fact, the PropertyLoad producing a rval often is in a different ReactiveScope from where the rval is used. We only need to add `#inConditionalWithinScope` when we actually visit a reactive dependency.

Mofei Zhang committed Mar 24, 2023 at 17:53 UTC 5e95967c7c4788519a02486feea48455079b0fcd
2 files changed +17 -27
compiler/forget/src/ReactiveScopes/DeriveMinimalDependencies.ts
+3 -7
@@ -3,10 +3,6 @@ import { Identifier, ReactiveScopeDependency } from "../HIR";
3 import { printIdentifier } from "../HIR/PrintHIR";
4 import { assertExhaustive } from "../Utils/utils";
5
6 -export type ReactiveScopeDependencyInfo = ReactiveScopeDependency & {
7 - cond: boolean;
8 -};
9 -
6 /**
7 * Finalizes a set of ReactiveScopeDependencies to produce a set of minimal unconditional
8 * dependencies, preserving granular accesses when possible.
@@ -46,14 +42,14 @@ export class ReactiveScopeDependencyTree {
42 return rootNode;
43 }
44
49 - add(dep: ReactiveScopeDependencyInfo): void {
45 + add(dep: ReactiveScopeDependency, inConditional: boolean): void {
46 const path = dep.path ?? [];
47 let currNode = this.#getOrCreateRoot(dep.identifier);
48
53 - const accessType = dep.cond
49 + const accessType = inConditional
50 ? PropertyAccessType.ConditionalAccess
51 : PropertyAccessType.UnconditionalAccess;
56 - const depType = dep.cond
52 + const depType = inConditional
53 ? PropertyAccessType.ConditionalDependency
54 : PropertyAccessType.UnconditionalDependency;
55
compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts
+14 -20
@@ -24,10 +24,7 @@ import {
24 eachPatternOperand,
25 } from "../HIR/visitors";
26 import { assertExhaustive } from "../Utils/utils";
27 -import {
28 - ReactiveScopeDependencyInfo,
29 - ReactiveScopeDependencyTree,
30 -} from "./DeriveMinimalDependencies";
27 +import { ReactiveScopeDependencyTree } from "./DeriveMinimalDependencies";
28
29 /**
30 * Infers the dependencies of each scope to include variables whose values
@@ -66,7 +63,12 @@ class Context {
63 // Reactive dependencies used in the current reactive scope.
64 #dependencies: ReactiveScopeDependencyTree =
65 new ReactiveScopeDependencyTree();
69 - #properties: Map<Identifier, ReactiveScopeDependencyInfo> = new Map();
66 + // We keep a sidemap for temporaries created by PropertyLoads, and do
67 + // not store any control flow (i.e. #inConditionalWithinScope) here.
68 + // - a ReactiveScope (A) containing a PropertyLoad may differ from the
69 + // ReactiveScope (B) that uses the produced temporary.
70 + // - codegen will inline these PropertyLoads back into scope (B)
71 + #properties: Map<Identifier, ReactiveScopeDependency> = new Map();
72 #temporaries: Map<Identifier, Place> = new Map();
73 #inConditionalWithinScope: boolean = false;
74 // Reactive dependencies used unconditionally in the current conditional.
@@ -187,18 +189,16 @@ class Context {
189 declareProperty(lvalue: Place, object: Place, property: string): void {
190 const resolvedObject = this.#temporaries.get(object.identifier) ?? object;
191 const objectDependency = this.#properties.get(resolvedObject.identifier);
190 - let nextDependency: ReactiveScopeDependencyInfo;
192 + let nextDependency: ReactiveScopeDependency;
193 if (objectDependency === undefined) {
194 nextDependency = {
195 identifier: resolvedObject.identifier,
196 path: [property],
195 - cond: this.#inConditionalWithinScope,
197 };
198 } else {
199 nextDependency = {
200 identifier: objectDependency.identifier,
201 path: [...(objectDependency.path ?? []), property],
201 - cond: this.#inConditionalWithinScope,
202 };
203 }
204 this.#properties.set(lvalue.identifier, nextDependency);
@@ -234,32 +234,29 @@ class Context {
234 this.visitDependency({
235 identifier: resolved.identifier,
236 path: null,
237 - cond: this.#inConditionalWithinScope,
237 });
238 }
239
240 visitProperty(object: Place, property: string): void {
241 const resolvedObject = this.#temporaries.get(object.identifier) ?? object;
242 const objectDependency = this.#properties.get(resolvedObject.identifier);
244 - let nextDependency: ReactiveScopeDependencyInfo;
243 + let nextDependency: ReactiveScopeDependency;
244 if (objectDependency === undefined) {
245 nextDependency = {
246 identifier: resolvedObject.identifier,
247 path: [property],
249 - cond: this.#inConditionalWithinScope,
248 };
249 } else {
250 nextDependency = {
251 identifier: objectDependency.identifier,
252 path: [...(objectDependency.path ?? []), property],
255 - cond: this.#inConditionalWithinScope,
253 };
254 }
255 this.visitDependency(nextDependency);
256 }
257
261 - visitDependency(dependency: ReactiveScopeDependencyInfo): void {
262 - let maybeDependency: ReactiveScopeDependencyInfo;
258 + visitDependency(dependency: ReactiveScopeDependency): void {
259 + let maybeDependency: ReactiveScopeDependency;
260 if (dependency.path !== null) {
261 // Operands may have memberPaths when propagating depenencies of an inner scope upward
262 // In this case we use the dependency as-is
@@ -269,7 +266,7 @@ class Context {
266 // the expanded Place. Fall back to using the operand as-is.
267 let propDep = this.#properties.get(dependency.identifier);
268 if (dependency.identifier.name === null && propDep !== undefined) {
272 - maybeDependency = { ...propDep, cond: dependency.cond };
269 + maybeDependency = { ...propDep };
270 } else {
271 maybeDependency = dependency;
272 }
@@ -297,13 +294,10 @@ class Context {
294 }
295
296 if (this.#checkValidDependencyId(maybeDependency.identifier)) {
300 - this.#depsInCurrentConditional.add({
301 - ...maybeDependency,
302 - cond: true,
303 - });
297 + this.#depsInCurrentConditional.add(maybeDependency, true);
298 // Add info about this dependency to the existing tree
299 // We do not try to join/reduce dependencies here due to missing info
306 - this.#dependencies.add(maybeDependency);
300 + this.#dependencies.add(maybeDependency, this.#inConditionalWithinScope);
301 }
302 }
303