@samitouri / QOS-React-2 / commits / 58a1cd872e

Be explicit about lvalues (to distinguish their memo level later)

This is prep for #1345, which distinguishes different memoization levels per lvalue

Joe Savona committed Mar 9, 2023 at 12:44 UTC 58a1cd872ebe86cc077816528f92a82cc141836c
1 file changed +54 -27
compiler/forget/src/ReactiveScopes/PruneNonEscapingScopes.ts
+54 -27
@@ -22,7 +22,7 @@ import {
22 ReactiveValue,
23 ScopeId,
24 } from "../HIR";
25 -import { eachInstructionLValue } from "../HIR/visitors";
25 +import { eachPatternOperand } from "../HIR/visitors";
26 import { log } from "../Utils/logger";
27 import { assertExhaustive } from "../Utils/utils";
28 import { getPlaceScope } from "./BuildReactiveBlocks";
@@ -302,6 +302,11 @@ function computeMemoizedIdentifiers(state: State): Set<IdentifierId> {
302 return memoized;
303 }
304
305 +type LValueMemoization = {
306 + place: Place;
307 + level: MemoizationLevel;
308 +};
309 +
310 /**
311 * Given a value, returns a description of how it should be memoized:
312 * - lvalues: optional extra places that are lvalue-like in the sense of
@@ -309,20 +314,23 @@ function computeMemoizedIdentifiers(state: State): Set<IdentifierId> {
314 * - rvalues: places that are aliased by the instruction's lvalues.
315 * - level: the level of memoization to apply to this value
316 */
312 -function computeMemoizationInputs(value: ReactiveValue): {
317 +function computeMemoizationInputs(
318 + value: ReactiveValue,
319 + lvalue: Place | null
320 +): {
321 // can optionally return a custom set of lvalues per instruction
314 - lvalues: Array<Place> | null;
322 + lvalues: Array<Place>;
323 rvalues: Array<Place>;
324 level: MemoizationLevel;
325 } {
326 switch (value.kind) {
327 case "ConditionalExpression": {
328 return {
321 - lvalues: null,
329 + lvalues: lvalue !== null ? [lvalue] : [],
330 rvalues: [
331 // Conditionals do not alias their test value.
324 - ...computeMemoizationInputs(value.consequent).rvalues,
325 - ...computeMemoizationInputs(value.alternate).rvalues,
332 + ...computeMemoizationInputs(value.consequent, null).rvalues,
333 + ...computeMemoizationInputs(value.alternate, null).rvalues,
334 ],
335 // Only need to memoize if the rvalues are memoized
336 level: MemoizationLevel.Conditional,
@@ -330,10 +338,10 @@ function computeMemoizationInputs(value: ReactiveValue): {
338 }
339 case "LogicalExpression": {
340 return {
333 - lvalues: null,
341 + lvalues: lvalue !== null ? [lvalue] : [],
342 rvalues: [
335 - ...computeMemoizationInputs(value.left).rvalues,
336 - ...computeMemoizationInputs(value.right).rvalues,
343 + ...computeMemoizationInputs(value.left, null).rvalues,
344 + ...computeMemoizationInputs(value.right, null).rvalues,
345 ],
346 // Only need to memoize if the rvalues are memoized
347 level: MemoizationLevel.Conditional,
@@ -341,11 +349,11 @@ function computeMemoizationInputs(value: ReactiveValue): {
349 }
350 case "SequenceExpression": {
351 return {
344 - lvalues: null,
352 + lvalues: lvalue !== null ? [lvalue] : [],
353 // Only the final value of the sequence is a true rvalue:
354 // values from the sequence's instructions are evaluated
355 // as separate nodes
348 - rvalues: computeMemoizationInputs(value.value).rvalues,
356 + rvalues: computeMemoizationInputs(value.value, null).rvalues,
357 // Only memoize if the final value was memoized
358 level: MemoizationLevel.Conditional,
359 };
@@ -366,7 +374,7 @@ function computeMemoizationInputs(value: ReactiveValue): {
374 }
375 }
376 return {
369 - lvalues: null,
377 + lvalues: lvalue !== null ? [lvalue] : [],
378 rvalues: operands,
379 // JSX elements themselves are not memoized unless forced to
380 // avoid breaking downstream memoization
@@ -375,7 +383,7 @@ function computeMemoizationInputs(value: ReactiveValue): {
383 }
384 case "JsxFragment": {
385 return {
378 - lvalues: null,
386 + lvalues: lvalue !== null ? [lvalue] : [],
387 rvalues: value.children,
388 // JSX elements themselves are not memoized unless forced to
389 // avoid breaking downstream memoization
@@ -391,7 +399,7 @@ function computeMemoizationInputs(value: ReactiveValue): {
399 case "BinaryExpression":
400 case "UnaryExpression": {
401 return {
394 - lvalues: null,
402 + lvalues: lvalue !== null ? [lvalue] : [],
403 rvalues: [],
404 // All of these instructions return a primitive value and never need to be memoized
405 level: MemoizationLevel.Never,
@@ -399,7 +407,7 @@ function computeMemoizationInputs(value: ReactiveValue): {
407 }
408 case "TypeCastExpression": {
409 return {
402 - lvalues: null,
410 + lvalues: lvalue !== null ? [lvalue] : [],
411 // Indirection for the inner value, memoized if the value is
412 rvalues: [value.value],
413 level: MemoizationLevel.Conditional,
@@ -407,16 +415,29 @@ function computeMemoizationInputs(value: ReactiveValue): {
415 }
416 case "LoadLocal": {
417 return {
410 - lvalues: null,
418 + lvalues: lvalue !== null ? [lvalue] : [],
419 // Indirection for the inner value, memoized if the value is
420 rvalues: [value.place],
421 level: MemoizationLevel.Conditional,
422 };
423 }
416 - case "Destructure":
424 case "StoreLocal": {
425 return {
419 - lvalues: null,
426 + lvalues:
427 + lvalue !== null ? [lvalue, value.lvalue.place] : [value.lvalue.place],
428 + // Indirection for the inner value, memoized if the value is
429 + rvalues: [value.value],
430 + level: MemoizationLevel.Conditional,
431 + };
432 + }
433 + case "Destructure": {
434 + const lvalues = [];
435 + if (lvalue !== null) {
436 + lvalues.push(lvalue);
437 + }
438 + lvalues.push(...eachPatternOperand(value.lvalue.pattern));
439 + return {
440 + lvalues: lvalues,
441 // Indirection for the inner value, memoized if the value is
442 rvalues: [value.value],
443 level: MemoizationLevel.Conditional,
@@ -425,7 +446,8 @@ function computeMemoizationInputs(value: ReactiveValue): {
446 case "ComputedLoad":
447 case "PropertyLoad": {
448 return {
428 - lvalues: null,
449 + lvalues: lvalue !== null ? [lvalue] : [],
450 + // Indirection for the inner value, memoized if the value is
451 // Only the object is aliased to the result, and the result only needs to be
452 // memoized if the object is
453 rvalues: [value.object],
@@ -436,7 +458,7 @@ function computeMemoizationInputs(value: ReactiveValue): {
458 // The object being stored to acts as an lvalue (it aliases the value), but
459 // the computed key is not aliased
460 return {
439 - lvalues: [value.object],
461 + lvalues: lvalue !== null ? [lvalue, value.object] : [value.object],
462 rvalues: [value.value],
463 level: MemoizationLevel.Conditional,
464 };
@@ -453,8 +475,14 @@ function computeMemoizationInputs(value: ReactiveValue): {
475 // All of these instructions may produce new values which must be memoized if
476 // reachable from a return value. Any mutable rvalue may alias any other rvalue
477 const operands = [...eachReactiveValueOperand(value)];
478 + const lvalues = operands.filter((operand) =>
479 + isMutableEffect(operand.effect)
480 + );
481 + if (lvalue !== null) {
482 + lvalues.push(lvalue);
483 + }
484 return {
457 - lvalues: operands.filter((operand) => isMutableEffect(operand.effect)),
485 + lvalues,
486 rvalues: operands,
487 level: MemoizationLevel.Memoized,
488 };
@@ -480,7 +508,10 @@ class CollectDependenciesVisitor extends ReactiveFunctionVisitor<State> {
508 this.traverseInstruction(instruction, state);
509
510 // Determe the level of memoization for this value and the lvalues/rvalues
483 - const aliasing = computeMemoizationInputs(instruction.value);
511 + const aliasing = computeMemoizationInputs(
512 + instruction.value,
513 + instruction.lvalue
514 + );
515
516 // Associate all the rvalues with the instruction's scope if it has one
517 for (const operand of aliasing.rvalues) {
@@ -490,11 +521,7 @@ class CollectDependenciesVisitor extends ReactiveFunctionVisitor<State> {
521 }
522
523 // Add the operands as dependencies of all lvalues.
493 - const lvalues =
494 - aliasing.lvalues !== null
495 - ? [...eachInstructionLValue(instruction), ...aliasing.lvalues]
496 - : [...eachInstructionLValue(instruction)];
497 - for (const lvalue of lvalues) {
524 + for (const lvalue of aliasing.lvalues) {
525 const lvalueId =
526 state.definitions.get(lvalue.identifier.id) ?? lvalue.identifier.id;
527 let node = state.identifiers.get(lvalueId);