Skip to content

Commit

Permalink
Don't recompute when dependent signals come back to previous values u…
Browse files Browse the repository at this point in the history
…sed for last computation
  • Loading branch information
divdavem committed Sep 10, 2024
1 parent c6aa9ba commit 49e85fe
Show file tree
Hide file tree
Showing 5 changed files with 141 additions and 13 deletions.
26 changes: 26 additions & 0 deletions src/computed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
REACTIVE_NODE,
ReactiveNode,
SIGNAL,
Version,
} from './graph.js';

/**
Expand All @@ -41,6 +42,7 @@ export interface ComputedNode<T> extends ReactiveNode {
computation: () => T;

equal: ValueEqualityFn<T>;
equalCache: Record<number, boolean> | null;
}

export type ComputedGetter<T> = (() => T) & {
Expand Down Expand Up @@ -103,13 +105,37 @@ const COMPUTED_NODE = /* @__PURE__ */ (() => {
dirty: true,
error: null,
equal: defaultEquals,
equalCache: null,

producerMustRecompute(node: ComputedNode<unknown>): boolean {
// Force a recomputation if there's no current value, or if the current value is in the
// process of being calculated (which should throw an error).
return node.value === UNSET || node.value === COMPUTING;
},

producerEquals(node: ComputedNode<unknown>, value: unknown, valueVersion: Version) {
if (
valueVersion + 1 === node.version || // equal is called before the version is incremented
value === ERRORED ||
node.value === ERRORED ||
value === COMPUTING ||
node.value === COMPUTING ||
value === UNSET ||
node.value === UNSET
) {
return false;
}
let res = node.equalCache?.[valueVersion];
if (res == null) {
res = !!node.equal.call(node.wrapper, value, node.value);
if (!node.equalCache) {
node.equalCache = {};
}
node.equalCache[valueVersion] = res;
}
return res;
},

producerRecomputeValue(node: ComputedNode<unknown>): void {
if (node.value === COMPUTING) {
// Our computation somehow led to a cyclic read of itself.
Expand Down
43 changes: 30 additions & 13 deletions src/graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ declare const ngDevMode: boolean | undefined;
let activeConsumer: ReactiveNode | null = null;
let inNotificationPhase = false;

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

/**
* Global epoch counter. Incremented whenever a source signal is set.
Expand Down Expand Up @@ -55,11 +55,13 @@ export function isReactive(value: unknown): value is Reactive {
}

export const REACTIVE_NODE: ReactiveNode = {
value: undefined,
version: 0 as Version,
lastCleanEpoch: 0 as Version,
dirty: false,
producerNode: undefined,
producerLastReadVersion: undefined,
producerLastReadValue: undefined,
producerIndexOfThis: undefined,
nextProducerIndex: 0,
liveConsumerNode: undefined,
Expand All @@ -68,6 +70,7 @@ export const REACTIVE_NODE: ReactiveNode = {
consumerIsAlwaysLive: false,
producerMustRecompute: () => false,
producerRecomputeValue: () => {},
producerEquals: () => false,
consumerMarkedDirty: () => {},
consumerOnSignalRead: () => {},
};
Expand All @@ -85,6 +88,8 @@ export const REACTIVE_NODE: ReactiveNode = {
* A `ReactiveNode` may be both a producer and consumer.
*/
export interface ReactiveNode {
value: unknown;

/**
* Version of the value that this node produces.
*
Expand Down Expand Up @@ -123,6 +128,13 @@ export interface ReactiveNode {
*/
producerLastReadVersion: Version[] | undefined;

/**
* Value last read by a given producer.
*
* Uses the same indices as the `producerNode`, `producerLastReadVersion` and `producerIndexOfThis` arrays.
*/
producerLastReadValue: unknown[] | undefined;

/**
* Index of `this` (consumer) in each producer's `liveConsumers` array.
*
Expand Down Expand Up @@ -173,6 +185,7 @@ export interface ReactiveNode {
*/
producerMustRecompute(node: unknown): boolean;
producerRecomputeValue(node: unknown): void;
producerEquals(node: unknown, value: unknown, valueVersion: Version): boolean;
consumerMarkedDirty(this: unknown): void;

/**
Expand Down Expand Up @@ -201,6 +214,7 @@ interface ConsumerNode extends ReactiveNode {
producerNode: NonNullable<ReactiveNode['producerNode']>;
producerIndexOfThis: NonNullable<ReactiveNode['producerIndexOfThis']>;
producerLastReadVersion: NonNullable<ReactiveNode['producerLastReadVersion']>;
producerLastReadValue: NonNullable<ReactiveNode['producerLastReadValue']>;
}

interface ProducerNode extends ReactiveNode {
Expand Down Expand Up @@ -260,6 +274,7 @@ export function producerAccessed(node: ReactiveNode): void {
: 0;
}
activeConsumer.producerLastReadVersion[idx] = node.version;
activeConsumer.producerLastReadValue[idx] = node.value;
}

/**
Expand Down Expand Up @@ -366,7 +381,8 @@ export function consumerAfterComputation(
!node ||
node.producerNode === undefined ||
node.producerIndexOfThis === undefined ||
node.producerLastReadVersion === undefined
node.producerLastReadVersion === undefined ||
node.producerLastReadValue === undefined
) {
return;
}
Expand All @@ -385,6 +401,7 @@ export function consumerAfterComputation(
while (node.producerNode.length > node.nextProducerIndex) {
node.producerNode.pop();
node.producerLastReadVersion.pop();
node.producerLastReadValue.pop();
node.producerIndexOfThis.pop();
}
}
Expand All @@ -400,21 +417,19 @@ export function consumerPollProducersForChange(node: ReactiveNode): boolean {
for (let i = 0; i < node.producerNode.length; i++) {
const producer = node.producerNode[i];
const seenVersion = node.producerLastReadVersion[i];
const seenValue = node.producerLastReadValue[i];

// First check the versions. A mismatch means that the producer's value is known to have
// changed since the last time we read it.
if (seenVersion !== producer.version) {
return true;
}

// The producer's version is the same as the last time we read it, but it might itself be
// stale. Force the producer to recompute its version (calculating a new value if necessary).
// Force the producer to recompute its version (calculating a new value if necessary).
producerUpdateValueVersion(producer);

// Now when we do this check, `producer.version` is guaranteed to be up to date, so if the
// versions still match then it has not changed since the last time we read it.
// If the producer's version has changed since we last read it, then we need to recompute.
if (seenVersion !== producer.version) {
return true;
if (producer.producerEquals(producer, seenValue, seenVersion)) {
node.producerLastReadVersion[i] = producer.version;
node.producerLastReadValue[i] = producer.value;
} else {
return true;
}
}
}

Expand All @@ -436,6 +451,7 @@ export function consumerDestroy(node: ReactiveNode): void {
// Truncate all the arrays to drop all connection from this node to the graph.
node.producerNode.length =
node.producerLastReadVersion.length =
node.producerLastReadValue.length =
node.producerIndexOfThis.length =
0;
if (node.liveConsumerNode) {
Expand Down Expand Up @@ -518,6 +534,7 @@ export function assertConsumerNode(node: ReactiveNode): asserts node is Consumer
node.producerNode ??= [];
node.producerIndexOfThis ??= [];
node.producerLastReadVersion ??= [];
node.producerLastReadValue ??= [];
}

export function assertProducerNode(node: ReactiveNode): asserts node is ProducerNode {
Expand Down
17 changes: 17 additions & 0 deletions src/signal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ let postSignalSetFn: (() => void) | null = null;
export interface SignalNode<T> extends ReactiveNode {
value: T;
equal: ValueEqualityFn<T>;
equalCache: Record<number, boolean> | null;
}

export type SignalBaseGetter<T> = (() => T) & {readonly [SIGNAL]: unknown};
Expand Down Expand Up @@ -94,12 +95,28 @@ export const SIGNAL_NODE: SignalNode<unknown> = /* @__PURE__ */ (() => {
return {
...REACTIVE_NODE,
equal: defaultEquals,
equalCache: null,
value: undefined,
producerEquals(node: SignalNode<unknown>, value, valueVersion) {
if (valueVersion + 1 === node.version) {
return false; // equal is called before the version is incremented
}
let res = node.equalCache?.[valueVersion];
if (res == null) {
res = !!node.equal.call(node.wrapper, value, node.value);
if (!node.equalCache) {
node.equalCache = {};
}
node.equalCache[valueVersion] = res;
}
return res;
},
};
})();

function signalValueChanged<T>(node: SignalNode<T>): void {
node.version++;
node.equalCache = null;
producerIncrementEpoch();
producerNotifyConsumers(node);
postSignalSetFn?.();
Expand Down
31 changes: 31 additions & 0 deletions tests/Signal/computed.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,35 @@ describe('Computed', () => {
expect(calls).toBe(2);
});
});

it('should not recompute when the dependent values go back to the ones used for last computation', () => {
const s = new Signal.State(0);
let n = 0;
const c = new Signal.Computed(() => (n++, s.get()));
expect(n).toBe(0);
expect(c.get()).toBe(0);
expect(n).toBe(1);
s.set(1);
expect(n).toBe(1);
s.set(0);
expect(n).toBe(1);
expect(c.get()).toBe(0); // the last time c was computed was with s = 0, no need to recompute
expect(n).toBe(1);
});

it('should not recompute when the dependent values go back to the ones used for last computation (with extra computed)', () => {
const s = new Signal.State(0);
let n = 0;
const extra = new Signal.Computed(() => s.get());
const c = new Signal.Computed(() => (n++, extra.get()));
expect(n).toBe(0);
expect(c.get()).toBe(0);
expect(n).toBe(1);
s.set(1);
expect(n).toBe(1);
s.set(0);
expect(n).toBe(1);
expect(c.get()).toBe(0); // the last time c was computed was with s = 0, no need to recompute
expect(n).toBe(1);
});
});
37 changes: 37 additions & 0 deletions tests/behaviors/custom-equality.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,41 @@ describe('Custom equality', () => {
expect(outerFn).toBeCalledTimes(2);
expect(cutoff).toBeCalledTimes(2);
});

it('should not call equal multiple times for the same comparison', () => {
let equalCalls: [number, number][] = [];
const equals = (a: number, b: number) => {
equalCalls.push([a, b]);
return a === b;
};
const s = new Signal.State<number>(0, {equals});
let n1 = 0;
let n2 = 0;
const c1 = new Signal.Computed(() => (n1++, s.get()));
const c2 = new Signal.Computed(() => (n2++, s.get()));
expect(equalCalls).toEqual([]);
expect(n1).toBe(0);
expect(c1.get()).toBe(0);
expect(n1).toBe(1);
expect(n2).toBe(0);
expect(c2.get()).toBe(0);
expect(n2).toBe(1);
s.set(1);
expect(equalCalls).toEqual([[0, 1]]);
equalCalls = [];
expect(n1).toBe(1);
expect(n2).toBe(1);
s.set(0);
expect(equalCalls).toEqual([[1, 0]]);
equalCalls = [];
expect(n1).toBe(1);
expect(n2).toBe(1);
expect(c1.get()).toBe(0); // the last time c1 was computed was with s = 0, no need to recompute
expect(equalCalls).toEqual([[0, 0]]); // equal should have been called
equalCalls = [];
expect(c2.get()).toBe(0); // the last time c2 was computed was with s = 0, no need to recompute
expect(equalCalls).toEqual([]); // equal should not have been called again
expect(n1).toBe(1);
expect(n2).toBe(1);
});
});

0 comments on commit 49e85fe

Please sign in to comment.