Skip to content

Commit

Permalink
refactor(core): global epoch to optimize non-live signal reads (angul…
Browse files Browse the repository at this point in the history
…ar#52420)

This commit adds a global epoch to the reactive graph, which can optimize
non-live reads.

When a non-live read occurs, a computed must poll its dependencies to check
if they've changed, and this operation is transitive and not cacheable.
Since non-live computeds don't receive dirty notifications, they're forced
to assume potential dirtiness on each and every read.

Using a global epoch, we can add an important optimization: if *no* signals
have been set globally since the last time it polled its dependencies, then
we *can* assume a clean state. This significantly improves performance of
large unwatched graphs when repeatedly reading values.

PR Close angular#52420
  • Loading branch information
alxhub committed Oct 31, 2023
1 parent b2d1e5c commit 62161a6
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 2 deletions.
1 change: 1 addition & 0 deletions goldens/public-api/core/primitives/signals/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export interface ReactiveNode {
consumerMarkedDirty(node: unknown): void;
consumerOnSignalRead(node: unknown): void;
dirty: boolean;
lastCleanEpoch: Version;
liveConsumerIndexOfThis: number[] | undefined;
liveConsumerNode: ReactiveNode[] | undefined;
nextProducerIndex: number;
Expand Down
32 changes: 32 additions & 0 deletions packages/core/primitives/signals/src/graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ let inNotificationPhase = false;

type Version = number&{__brand: 'Version'};

/**
* Global epoch counter. Incremented whenever a source signal is set.
*/
let epoch: Version = 1 as Version;

/**
* Symbol used to tell `Signal`s apart from other functions.
*
Expand Down Expand Up @@ -52,6 +57,7 @@ export function isReactive(value: unknown): value is Reactive {

export const REACTIVE_NODE: ReactiveNode = {
version: 0 as Version,
lastCleanEpoch: 0 as Version,
dirty: false,
producerNode: undefined,
producerLastReadVersion: undefined,
Expand Down Expand Up @@ -88,6 +94,14 @@ export interface ReactiveNode {
*/
version: Version;

/**
* Epoch at which this node is verified to be clean.
*
* This allows skipping of some polling operations in the case where no signals have been set
* since this node was last read.
*/
lastCleanEpoch: Version;

/**
* Whether this node (in its consumer capacity) is dirty.
*
Expand Down Expand Up @@ -231,6 +245,15 @@ export function producerAccessed(node: ReactiveNode): void {
activeConsumer.producerLastReadVersion[idx] = node.version;
}

/**
* Increment the global epoch counter.
*
* Called by source producers (that is, not computeds) whenever their values change.
*/
export function producerIncrementEpoch(): void {
epoch++;
}

/**
* Ensure this producer's `version` is up-to-date.
*/
Expand All @@ -241,17 +264,26 @@ export function producerUpdateValueVersion(node: ReactiveNode): void {
return;
}

if (!node.dirty && node.lastCleanEpoch === epoch) {
// Even non-live consumers can skip polling if they previously found themselves to be clean at
// the current epoch, since their dependencies could not possibly have changed (such a change
// would've increased the epoch).
return;
}

if (!node.producerMustRecompute(node) && !consumerPollProducersForChange(node)) {
// None of our producers report a change since the last time they were read, so no
// recomputation of our value is necessary, and we can consider ourselves clean.
node.dirty = false;
node.lastCleanEpoch = epoch;
return;
}

node.producerRecomputeValue(node);

// After recomputing the value, we're no longer dirty.
node.dirty = false;
node.lastCleanEpoch = epoch;
}

/**
Expand Down
3 changes: 2 additions & 1 deletion packages/core/primitives/signals/src/signal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import {defaultEquals, ValueEqualityFn} from './equality';
import {throwInvalidWriteToSignalError} from './errors';
import {producerAccessed, producerNotifyConsumers, producerUpdatesAllowed, REACTIVE_NODE, ReactiveNode, SIGNAL} from './graph';
import {producerAccessed, producerIncrementEpoch, producerNotifyConsumers, producerUpdatesAllowed, REACTIVE_NODE, ReactiveNode, SIGNAL} from './graph';

/**
* If set, called after `WritableSignal`s are updated.
Expand Down Expand Up @@ -98,6 +98,7 @@ const SIGNAL_NODE: object = /* @__PURE__ */ (() => {

function signalValueChanged<T>(node: SignalNode<T>): void {
node.version++;
producerIncrementEpoch();
producerNotifyConsumers(node);
postSignalSetFn?.();
}
35 changes: 34 additions & 1 deletion packages/core/test/signals/signal_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import {computed, signal} from '@angular/core';
import {setPostSignalSetFn} from '@angular/core/primitives/signals';
import {ReactiveNode, setPostSignalSetFn, SIGNAL} from '@angular/core/primitives/signals';

describe('signals', () => {
it('should be a getter which reflects the set value', () => {
Expand Down Expand Up @@ -104,6 +104,39 @@ describe('signals', () => {
expect(double()).toBe(4);
});

describe('optimizations', () => {
it('should not repeatedly poll status of a non-live node if no signals have changed', () => {
const unrelated = signal(0);
const source = signal(1);
let computations = 0;
const derived = computed(() => {
computations++;
return source() * 2;
});

expect(derived()).toBe(2);
expect(computations).toBe(1);

const sourceNode = source[SIGNAL] as ReactiveNode;
// Forcibly increment the version of the source signal. This will cause a mismatch during
// polling, and will force the derived signal to recompute if polled (which we should observe
// in this test).
sourceNode.version++;

// Read the derived signal again. This should not recompute (even with the forced version
// update) as no signals have been set since the last read.
expect(derived()).toBe(2);
expect(computations).toBe(1);

// Set the `unrelated` signal, which now means that `derived` should poll if read again.
// Because of the forced version, that poll will cause a recomputation which we will observe.
unrelated.set(1);

expect(derived()).toBe(2);
expect(computations).toBe(2);
});
});

describe('post-signal-set functions', () => {
let prevPostSignalSetFn: (() => void)|null = null;
let log: number;
Expand Down

0 comments on commit 62161a6

Please sign in to comment.