refactor(core): switch signals to a refcounting algorithm (#51226)

This commit switches the signals library from a bidirectional symmetric dependency graph using weak references, to a bidirectional _asymmetric_ graph which uses strong references. This is made possible with a reference counting algorithm which only tracks producer -> consumer references for effect-like "live" consumers, preventing memory leaks. The new algorithm should be simpler and faster than the previous implementation as weak references are fairly slow to create and traverse. A tradeoff is that non-live consumers must now poll their producers when read, as they cannot rely on dirty notifications. As part of this refactoring, the `ReactiveNode` class is replaced with an interface instead, and methods are moved to standalone functions. This is paired with instantiating individual signals/computeds via `Object.create` against a prototype node which contains static or initial values. This technique, in conjunction with the rest, greatly improves the performance of node creation. PR Close #51226
angular · Sep 1, 2023 · f56b655 · f56b655
1 parent 7bb4f91
commit f56b655
Show file tree

Hide file tree

Showing 26 changed files with 1,019 additions and 700 deletions.
diff --git a/goldens/size-tracking/integration-payloads.json b/goldens/size-tracking/integration-payloads.json
@@ -16,7 +16,7 @@
   "cli-hello-world-lazy": {
     "uncompressed": {
       "runtime": 2734,
-      "main": 232018,
+      "main": 238106,
       "polyfills": 33810,
       "src_app_lazy_lazy_routes_ts": 487
     }
@@ -38,7 +38,7 @@
   "platform-server/standalone/browser": {
     "uncompressed": {
       "runtime": 2694,
-      "main": 232544,
+      "main": 238273,
       "polyfills": 33782
     }
   },

diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts
@@ -17,7 +17,7 @@ import {DoCheck, OnChanges, OnInit} from '../../interface/lifecycle_hooks';
 import {SchemaMetadata} from '../../metadata/schema';
 import {ViewEncapsulation} from '../../metadata/view';
 import {validateAgainstEventAttributes, validateAgainstEventProperties} from '../../sanitization/sanitization';
-import {setActiveConsumer} from '../../signals';
+import {consumerAfterComputation, consumerBeforeComputation, setActiveConsumer} from '../../signals';
 import {assertDefined, assertEqual, assertGreaterThan, assertGreaterThanOrEqual, assertIndexInRange, assertNotEqual, assertNotSame, assertSame, assertString} from '../../util/assert';
 import {escapeCommentText} from '../../util/dom';
 import {normalizeDebugBindingName, normalizeDebugBindingValue} from '../../util/ng_reflect';
@@ -78,8 +78,14 @@ export function processHostBindingOpCodes(tView: TView, lView: LView): void {
         const bindingRootIndx = hostBindingOpCodes[++i] as number;
         const hostBindingFn = hostBindingOpCodes[++i] as HostBindingsFunction<any>;
         setBindingRootForHostBindings(bindingRootIndx, directiveIdx);
-        const context = lView[directiveIdx];
-        consumer.runInContext(hostBindingFn, RenderFlags.Update, context);
+        consumer.dirty = false;
+        const prevConsumer = consumerBeforeComputation(consumer);
+        try {
+          const context = lView[directiveIdx];
+          hostBindingFn(RenderFlags.Update, context);
+        } finally {
+          consumerAfterComputation(consumer, prevConsumer);
+        }
       }
     }
   } finally {
@@ -262,15 +268,15 @@ export function executeTemplate<T>(
     const preHookType =
         isUpdatePhase ? ProfilerEvent.TemplateUpdateStart : ProfilerEvent.TemplateCreateStart;
     profiler(preHookType, context as unknown as {});
-    if (isUpdatePhase) {
-      consumer.runInContext(templateFn, rf, context);
-    } else {
-      const prevConsumer = setActiveConsumer(null);
-      try {
-        templateFn(rf, context);
-      } finally {
-        setActiveConsumer(prevConsumer);
+    const effectiveConsumer = isUpdatePhase ? consumer : null;
+    const prevConsumer = consumerBeforeComputation(effectiveConsumer);
+    try {
+      if (effectiveConsumer !== null) {
+        effectiveConsumer.dirty = false;
       }
+      templateFn(rf, context);
+    } finally {
+      consumerAfterComputation(effectiveConsumer, prevConsumer);
     }
   } finally {
     if (isUpdatePhase && lView[REACTIVE_TEMPLATE_CONSUMER] === null) {

diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts
@@ -9,6 +9,7 @@
 import {hasInSkipHydrationBlockFlag} from '../hydration/skip_hydration';
 import {ViewEncapsulation} from '../metadata/view';
 import {RendererStyleFlags2} from '../render/api_flags';
+import {consumerDestroy} from '../signals';
 import {addToArray, removeFromArray} from '../util/array_utils';
 import {assertDefined, assertEqual, assertFunction, assertNumber, assertString} from '../util/assert';
 import {escapeCommentText} from '../util/dom';
@@ -376,8 +377,8 @@ export function destroyLView(tView: TView, lView: LView) {
   if (!(lView[FLAGS] & LViewFlags.Destroyed)) {
     const renderer = lView[RENDERER];
 
-    lView[REACTIVE_TEMPLATE_CONSUMER]?.destroy();
-    lView[REACTIVE_HOST_BINDING_CONSUMER]?.destroy();
+    lView[REACTIVE_TEMPLATE_CONSUMER] && consumerDestroy(lView[REACTIVE_TEMPLATE_CONSUMER]);
+    lView[REACTIVE_HOST_BINDING_CONSUMER] && consumerDestroy(lView[REACTIVE_HOST_BINDING_CONSUMER]);
 
     if (renderer.destroyNode) {
       applyView(tView, lView, renderer, WalkTNodeTreeAction.Destroy, null, null);

diff --git a/packages/core/src/render3/reactive_lview_consumer.ts b/packages/core/src/render3/reactive_lview_consumer.ts
@@ -6,63 +6,21 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {ReactiveNode, setActiveConsumer} from '../signals';
+import {REACTIVE_NODE, ReactiveNode} from '../signals';
 import {assertDefined, assertEqual} from '../util/assert';
 
 import {markViewDirty} from './instructions/mark_view_dirty';
-import {ComponentTemplate, HostBindingsFunction, RenderFlags} from './interfaces/definition';
 import {LView, REACTIVE_HOST_BINDING_CONSUMER, REACTIVE_TEMPLATE_CONSUMER} from './interfaces/view';
 
-export class ReactiveLViewConsumer extends ReactiveNode {
-  protected override consumerAllowSignalWrites = false;
-  private _lView: LView|null = null;
-
-  set lView(lView: LView) {
-    (typeof ngDevMode === 'undefined' || ngDevMode) &&
-        assertEqual(this._lView, null, 'Consumer already associated with a view.');
-    this._lView = lView;
-  }
-
-  protected override onConsumerDependencyMayHaveChanged() {
-    (typeof ngDevMode === 'undefined' || ngDevMode) &&
-        assertDefined(
-            this._lView,
-            'Updating a signal during template or host binding execution is not allowed.');
-    markViewDirty(this._lView!);
-  }
-
-  protected override onProducerUpdateValueVersion(): void {
-    // This type doesn't implement the producer side of a `ReactiveNode`.
-  }
-
-  get hasReadASignal(): boolean {
-    return this.hasProducers;
-  }
-
-  runInContext(
-      fn: HostBindingsFunction<unknown>|ComponentTemplate<unknown>, rf: RenderFlags,
-      ctx: unknown): void {
-    const prevConsumer = setActiveConsumer(this);
-    this.trackingVersion++;
-    try {
-      fn(rf, ctx);
-    } finally {
-      setActiveConsumer(prevConsumer);
-    }
-  }
-
-  destroy(): void {
-    // Incrementing the version means that every producer which tries to update this consumer will
-    // consider its record stale, and not notify.
-    this.trackingVersion++;
-  }
-}
-
 let currentConsumer: ReactiveLViewConsumer|null = null;
+export interface ReactiveLViewConsumer extends ReactiveNode {
+  lView: LView|null;
+}
 
-function getOrCreateCurrentLViewConsumer() {
-  currentConsumer ??= new ReactiveLViewConsumer();
-  return currentConsumer;
+export function setLViewForConsumer(node: ReactiveLViewConsumer, lView: LView): void {
+  (typeof ngDevMode === 'undefined' || ngDevMode) &&
+      assertEqual(node.lView, null, 'Consumer already associated with a view.');
+  node.lView = lView;
 }
 
 /**
@@ -90,11 +48,33 @@ export function commitLViewConsumerIfHasProducers(
     lView: LView,
     slot: typeof REACTIVE_TEMPLATE_CONSUMER|typeof REACTIVE_HOST_BINDING_CONSUMER): void {
   const consumer = getOrCreateCurrentLViewConsumer();
-  if (!consumer.hasReadASignal) {
+  if (!consumer.producerNode?.length) {
     return;
   }
 
   lView[slot] = currentConsumer;
   consumer.lView = lView;
-  currentConsumer = new ReactiveLViewConsumer();
+  currentConsumer = createLViewConsumer();
+}
+
+const REACTIVE_LVIEW_CONSUMER_NODE = {
+  ...REACTIVE_NODE,
+  consumerIsAlwaysLive: true,
+  consumerMarkedDirty: (node: ReactiveLViewConsumer) => {
+    (typeof ngDevMode === 'undefined' || ngDevMode) &&
+        assertDefined(
+            node.lView,
+            'Updating a signal during template or host binding execution is not allowed.');
+    markViewDirty(node.lView!);
+  },
+  lView: null,
+};
+
+function createLViewConsumer(): ReactiveLViewConsumer {
+  return Object.create(REACTIVE_LVIEW_CONSUMER_NODE);
+}
+
+function getOrCreateCurrentLViewConsumer() {
+  currentConsumer ??= createLViewConsumer();
+  return currentConsumer;
 }
diff --git a/packages/core/src/render3/reactivity/effect.ts b/packages/core/src/render3/reactivity/effect.ts
@@ -11,7 +11,7 @@ import {Injector} from '../../di/injector';
 import {inject} from '../../di/injector_compatibility';
 import {ɵɵdefineInjectable} from '../../di/interface/defs';
 import {DestroyRef} from '../../linker/destroy_ref';
-import {Watch} from '../../signals';
+import {Watch, watch} from '../../signals';
 
 /**
  * An effect can, optionally, register a cleanup function. If registered, the cleanup is executed
@@ -38,26 +38,26 @@ export class EffectManager {
       effectFn: (onCleanup: (cleanupFn: EffectCleanupFn) => void) => void,
       destroyRef: DestroyRef|null, allowSignalWrites: boolean): EffectRef {
     const zone = (typeof Zone === 'undefined') ? null : Zone.current;
-    const watch = new Watch(effectFn, (watch) => {
+    const w = watch(effectFn, (watch) => {
       if (!this.all.has(watch)) {
         return;
       }
 
       this.queue.set(watch, zone);
     }, allowSignalWrites);
 
-    this.all.add(watch);
+    this.all.add(w);
 
     // Effects start dirty.
-    watch.notify();
+    w.notify();
 
     let unregisterOnDestroy: (() => void)|undefined;
 
     const destroy = () => {
-      watch.cleanup();
+      w.cleanup();
       unregisterOnDestroy?.();
-      this.all.delete(watch);
-      this.queue.delete(watch);
+      this.all.delete(w);
+      this.queue.delete(w);
     };
 
     unregisterOnDestroy = destroyRef?.onDestroy(destroy);