🚨 [experiment] Attempt a monotonic revision based signals polyfill

[lifeart]

Prior to this change, we used the current signals polyfill which had some performance troubles.

This change implements the signals API but uses Pzurek's monotonic revision to handle tracking and reactivity.

This is an attempt to see if it shows performance improvements.

Paired-with: NullVoxPopulli

re-wind of #145

[lifeart]

@sukima looks like some error in implementation, Benchmark is unable to start:
check on

http://localhost:5173/benchmark

seems reactive tests is breaking

http://localhost:5173/tests.html?filter=reactive

[sukima]

I don't have access to push to this branch. I ran through the main API tests and here is a patch to update the polyfill as an experiment to see if the benchmark builds again.

From 6a79c84d1652736e79f95d82c314031aa242b06d Mon Sep 17 00:00:00 2001
From: Devin Weaver <[email protected]>
Date: Mon, 20 May 2024 20:46:51 -0400
Subject: [PATCH] WIP check implementation with benchmark [skip ci]

---
 src/signal-polyfill.ts | 61 +++++++++++++++++++++++++++---------------
 1 file changed, 39 insertions(+), 22 deletions(-)

diff --git a/src/signal-polyfill.ts b/src/signal-polyfill.ts
index f309b0a..03535f4 100644
--- a/src/signal-polyfill.ts
+++ b/src/signal-polyfill.ts
@@ -2,6 +2,7 @@ type Revision = number;
 
 interface Signal<T> {
   get(): T;
+  isDirty: boolean;
 }
 
 const $WATCHED = Symbol('watched');
@@ -11,24 +12,24 @@ const $WATCHER_NOTIFY = Symbol('watcher notify');
 
 const WATCHERS = new Set<Watcher>();
 
-let CONSUME_TAGS: boolean = true;
-let CURRENT_REVISION: Revision = 0;
-let CURRENT_COMPUTATION: Set<Tag> | null = null;
-let CURRENT_COMPUTED: Computed | null = null;
+let consumeTags: boolean = true;
+let currentRevision: Revision = 0;
+let currentComputation: Set<Tag> | null = null;
+// let currentComputed: Computed | null = null;
 
 class Tag {
-  [$REVISION]: Revision = CURRENT_REVISION;
+  [$REVISION]: Revision = currentRevision;
 }
 
 function dirtyTag(tag: Tag): void {
-  if (CURRENT_COMPUTATION?.has(tag))
+  if (currentComputation?.has(tag))
     throw new Error('cannot dirty tag that has been used during a computation');
-  tag[$REVISION] = ++CURRENT_REVISION;
+  tag[$REVISION] = ++currentRevision;
   notifyWatchers();
 }
 
 function consumeTag(tag: Tag): void {
-  if (CONSUME_TAGS) CURRENT_COMPUTATION?.add(tag);
+  if (consumeTags) currentComputation?.add(tag);
 }
 
 function notifyWatchers(): void {
@@ -41,11 +42,17 @@ function getMax(tags: Tag[]): Revision {
 
 class State<T> implements Signal<T> {
   private tag = new Tag();
+  private lastRevision: Revision;
   private equals = (a: T, b: T): boolean => a === b;
   private [$WATCHED] = (): void => {};
   private [$UNWATCHED] = (): void => {};
 
+  get isDirty() {
+    return this.lastRevision < this.tag[$REVISION];
+  }
+
   constructor(private value: T, options: SignalOptions<T> = {}) {
+    this.lastRevision = this.tag[$REVISION];
     this.equals = options.equals ?? this.equals;
     this[$WATCHED] = options[$WATCHED] ?? this[$WATCHED];
     this[$UNWATCHED] = options[$UNWATCHED] ?? this[$UNWATCHED];
@@ -53,6 +60,7 @@ class State<T> implements Signal<T> {
 
   get(): T {
     consumeTag(this.tag);
+    this.lastRevision = this.tag[$REVISION];
     return this.value;
   }
 
@@ -71,6 +79,10 @@ class Computed<T = unknown> implements Signal<T> {
   private [$WATCHED] = (): void => {};
   private [$UNWATCHED] = (): void => {};
 
+  get isDirty() {
+    return !(this.lastTags && getMax(this.lastTags) === this.lastRevision);
+  }
+
   constructor(private cb: (this: Computed<T>) => T, options: SignalOptions<T> = {}) {
     this.equals = options.equals ?? this.equals;
     this[$WATCHED] = options[$WATCHED] ?? this[$WATCHED];
@@ -78,26 +90,27 @@ class Computed<T = unknown> implements Signal<T> {
   }
 
   get(): T {
-    if (this.lastTags && getMax(this.lastTags) === this.lastRevision) {
-      if (CURRENT_COMPUTATION && this.lastTags.length > 0)
-        for (let tag of this.lastTags) CURRENT_COMPUTATION.add(tag);
+    if (this.lastTags && !this.isDirty) {
+      if (currentComputation && this.lastTags.length > 0)
+        for (let tag of this.lastTags) currentComputation.add(tag);
       return this.lastValue;
     }
 
-    let previousComputation = CURRENT_COMPUTATION;
+    let previousComputation = currentComputation;
+    currentComputation = new Set<Tag>();
 
     try {
       this.lastValue = this.cb.call(this);
     } finally {
-      let tags = Array.from(CURRENT_COMPUTATION ?? []);
+      let tags = Array.from(currentComputation ?? []);
       this.lastTags = tags;
       this.lastRevision = getMax(tags);
 
       if (previousComputation && tags.length > 0)
         for (let tag of tags) previousComputation.add(tag);
 
-      CURRENT_COMPUTATION = previousComputation;
-      CURRENT_COMPUTED = null;
+      currentComputation = previousComputation;
+      // currentComputed = null;
     }
 
     return this.lastValue;
@@ -109,17 +122,17 @@ class Computed<T = unknown> implements Signal<T> {
 // Analogous to `crypto.subtle`
 function untrack<T>(cb: () => T): T {
   try {
-    CONSUME_TAGS = false;
+    consumeTags = false;
     return cb();
   } finally {
-    CONSUME_TAGS = true;
+    consumeTags = true;
   }
 }
 
 // Get the current computed signal which is tracking any signal reads, if any
-function currentComputed(): Computed | null {
-  return CURRENT_COMPUTED;
-}
+// function currentComputed(): Computed | null {
+//   return currentComputed;
+// }
 
 // Returns ordered list of all signals which this one referenced
 // during the last time it was evaluated.
@@ -170,7 +183,11 @@ class Watcher {
   // Returns the set of sources in the Watcher's set which are still dirty, or is a computed signal
   // with a source which is dirty or pending and hasn't yet been re-evaluated
   getPending(): Signal<unknown>[] {
-    return Array.from(this.signals);
+    return Array.from(this.pending());
+  }
+
+  *pending(): Generator<Signal<unknown>> {
+    for (let signal of this.signals) if (signal.isDirty) yield signal;
   }
 
   [$WATCHER_NOTIFY](): void {
@@ -187,7 +204,7 @@ export const Signal = {
   Computed,
   subtle: {
     Watcher,
-    currentComputed,
+    // currentComputed,
     untrack,
     watched,
     unwatched,
-- 
2.44.0.270.g2953d95d40

[lifeart]

@sukima thanks! Going to update PR. GH says your invite is still pending:

[github-actions]

duration phase no difference [-90ms to 1ms]
renderEnd phase no difference [0ms to 0ms]
render1000Items1End phase estimated regression +2ms [1ms to 13ms] OR +1.25% [0.37% to 6.69%]
clearItems1End phase no difference [-2ms to 0ms]
render1000Items2End phase no difference [-4ms to 4ms]
clearItems2End phase no difference [0ms to 0ms]
render5000Items1End phase estimated improvement -27ms [-33ms to -18ms] OR -3.28% [-4.01% to -2.21%]
clearManyItems1End phase no difference [-11ms to 0ms]
render5000Items2End phase estimated regression +28ms [17ms to 33ms] OR +3.84% [2.35% to 4.54%]
clearManyItems2End phase estimated improvement -17ms [-17ms to -16ms] OR -14.31% [-14.36% to -14.13%]
render1000Items3End phase no difference [0ms to 1ms]
append1000Items1End phase no difference [-2ms to 20ms]
append1000Items2End phase no difference [-4ms to 115ms]
updateEvery10thItem1End phase estimated regression +75ms [1ms to 97ms] OR +2090.45% [18.46% to 2694.3%]
updateEvery10thItem2End phase no difference [-70ms to 1ms]
selectFirstRow1End phase estimated improvement -22ms [-46ms to -20ms] OR -53.48% [-110.43% to -48.94%]
selectSecondRow1End phase estimated improvement -20ms [-22ms to -18ms] OR -58.75% [-64.91% to -55.25%]
removeFirstRow1End phase estimated regression +7ms [3ms to 8ms] OR +13.34% [6.66% to 16.3%]
removeSecondRow1End phase no difference [0ms to 0ms]
swapRows1End phase no difference [0ms to 0ms]
swapRows2End phase no difference [0ms to 0ms]
clearItems4End phase estimated improvement -8ms [-9ms to -7ms] OR -13.02% [-14.48% to -11.58%]
paint phase estimated regression +3ms [3ms to 3ms] OR +263.43% [249.59% to 278.61%]

[05:26:47] Generating Benchmark Reports [started]
[05:26:52] Generating Benchmark Reports [completed]

Benchmark Reports    

JSON: /home/runner/work/glimmer-next/glimmer-next/tracerbench-results/compare.json

PDF: /home/runner/work/glimmer-next/glimmer-next/tracerbench-results/artifact-1.pdf

HTML: /home/runner/work/glimmer-next/glimmer-next/tracerbench-results/artifact-1.html

[lifeart]

artifact-1.pdf

[lifeart]

@sukima seems this signals implementation is way better at least in unwatch performance.
Some floating in numbers may be caused by increased memory usage (GC).

[sukima]

The github app seems to hide invites? I'll look into the web site version later for the invite.

I'm pleased to know this is working. @NullVoxPopuli want to pair again to game plan next steps? Should we add more introspection at the cost of performance or not and optimize the current code?

[NullVoxPopuli]

it would have been in your email 🙃

Should we add more introspection at the cost of performance or not and optimize the current code?

I think it'd be great to see you post over on the signals proposal with these findings -- maybe comment on the original issue: tc39/proposal-signals#215 <3

This way, we have more public record of progress, so we don't forget where we're starting from, and ensure that we don't regress (or rather, we know where we regress if we do)