[perf] faster signia capture (#3471)

This PR uses an additional ArraySet to make capturing parent
relationships faster for computeds with more than a handful of parents.
Seems to result in an overall ~20% speedup of the `maybeCaptureParent`
function in normal usage.

### Change Type

- [x] `sdk` — Changes the tldraw SDK
- [x] `improvement` — Improving existing features

### Test Plan

1. Add a step-by-step description of how to test your PR here.
2.

- [ ] Unit Tests
- [ ] End to end tests

### Release Notes

- Slight performance improvement to reactivity bookkeeping.
This commit is contained in:
David Sheldrick 2024-04-16 09:21:27 +01:00 committed by GitHub
parent 273ba62e0e
commit 8a5741c283
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 76 additions and 42 deletions

View file

@ -33,6 +33,8 @@ export interface Computed<Value, Diff = unknown> extends Signal<Value, Diff> {
readonly parentEpochs: number[]; readonly parentEpochs: number[];
// @internal (undocumented) // @internal (undocumented)
readonly parents: Signal<any, any>[]; readonly parents: Signal<any, any>[];
// @internal (undocumented)
readonly parentSet: ArraySet<Signal<any, any>>;
} }
// @public // @public

View file

@ -144,4 +144,29 @@ export class ArraySet<T> {
throw new Error('no set or array') throw new Error('no set or array')
} }
has(elem: T) {
if (this.array) {
return this.array.indexOf(elem) !== -1
} else {
return this.set!.has(elem)
}
}
clear() {
if (this.set) {
this.set.clear()
} else {
this.arraySize = 0
this.array?.fill(undefined)
}
}
size() {
if (this.set) {
return this.set.size
} else {
return this.arraySize
}
}
} }

View file

@ -123,6 +123,8 @@ export interface Computed<Value, Diff = unknown> extends Signal<Value, Diff> {
*/ */
readonly isActivelyListening: boolean readonly isActivelyListening: boolean
/** @internal */
readonly parentSet: ArraySet<Signal<any, any>>
/** @internal */ /** @internal */
readonly parents: Signal<any, any>[] readonly parents: Signal<any, any>[]
/** @internal */ /** @internal */
@ -141,6 +143,7 @@ class __UNSAFE__Computed<Value, Diff = unknown> implements Computed<Value, Diff>
*/ */
private lastCheckedEpoch = GLOBAL_START_EPOCH private lastCheckedEpoch = GLOBAL_START_EPOCH
parentSet = new ArraySet<Signal<any, any>>()
parents: Signal<any, any>[] = [] parents: Signal<any, any>[] = []
parentEpochs: number[] = [] parentEpochs: number[] = []

View file

@ -1,3 +1,4 @@
import { ArraySet } from './ArraySet'
import { startCapturingParents, stopCapturingParents } from './capture' import { startCapturingParents, stopCapturingParents } from './capture'
import { GLOBAL_START_EPOCH } from './constants' import { GLOBAL_START_EPOCH } from './constants'
import { attach, detach, haveParentsChanged, singleton } from './helpers' import { attach, detach, haveParentsChanged, singleton } from './helpers'
@ -63,9 +64,11 @@ class __EffectScheduler__<Result> {
} }
/** @internal */ /** @internal */
parentEpochs: number[] = [] readonly parentSet = new ArraySet<Signal<any, any>>()
/** @internal */ /** @internal */
parents: Signal<any, any>[] = [] readonly parentEpochs: number[] = []
/** @internal */
readonly parents: Signal<any, any>[] = []
private readonly _scheduleEffect?: (execute: () => void) => void private readonly _scheduleEffect?: (execute: () => void) => void
constructor( constructor(
public readonly name: string, public readonly name: string,

View file

@ -94,12 +94,16 @@ function runTest(seed: number) {
for (let i = 0; i < 1000; i++) { for (let i = 0; i < 1000; i++) {
const num = nums[Math.floor(r() * nums.length)] const num = nums[Math.floor(r() * nums.length)]
if (r() > 0.5) { const choice = r()
if (choice < 0.45) {
as.add(num) as.add(num)
s.add(num) s.add(num)
} else { } else if (choice < 0.9) {
as.remove(num) as.remove(num)
s.delete(num) s.delete(num)
} else {
as.clear()
s.clear()
} }
try { try {

View file

@ -1,3 +1,4 @@
import { ArraySet } from '../ArraySet'
import { atom } from '../Atom' import { atom } from '../Atom'
import { computed } from '../Computed' import { computed } from '../Computed'
import { react } from '../EffectScheduler' import { react } from '../EffectScheduler'
@ -14,6 +15,7 @@ const emptyChild = (props: Partial<Child> = {}) =>
({ ({
parentEpochs: [], parentEpochs: [],
parents: [], parents: [],
parentSet: new ArraySet(),
isActivelyListening: false, isActivelyListening: false,
lastTraversedEpoch: 0, lastTraversedEpoch: 0,
...props, ...props,

View file

@ -3,7 +3,6 @@ import type { Child, Signal } from './types'
class CaptureStackFrame { class CaptureStackFrame {
offset = 0 offset = 0
numNewParents = 0
maybeRemoved?: Signal<any>[] maybeRemoved?: Signal<any>[]
@ -50,33 +49,29 @@ export function unsafe__withoutCapture<T>(fn: () => T): T {
export function startCapturingParents(child: Child) { export function startCapturingParents(child: Child) {
inst.stack = new CaptureStackFrame(inst.stack, child) inst.stack = new CaptureStackFrame(inst.stack, child)
child.parentSet.clear()
} }
export function stopCapturingParents() { export function stopCapturingParents() {
const frame = inst.stack! const frame = inst.stack!
inst.stack = frame.below inst.stack = frame.below
const didParentsChange = frame.numNewParents > 0 || frame.offset !== frame.child.parents.length if (frame.offset < frame.child.parents.length) {
for (let i = frame.offset; i < frame.child.parents.length; i++) {
if (!didParentsChange) { const maybeRemovedParent = frame.child.parents[i]
return if (!frame.child.parentSet.has(maybeRemovedParent)) {
} detach(maybeRemovedParent, frame.child)
}
for (let i = frame.offset; i < frame.child.parents.length; i++) {
const p = frame.child.parents[i]
const parentWasRemoved = frame.child.parents.indexOf(p) >= frame.offset
if (parentWasRemoved) {
detach(p, frame.child)
} }
}
frame.child.parents.length = frame.offset frame.child.parents.length = frame.offset
frame.child.parentEpochs.length = frame.offset frame.child.parentEpochs.length = frame.offset
}
if (inst.stack?.maybeRemoved) { if (inst.stack?.maybeRemoved) {
for (let i = 0; i < inst.stack.maybeRemoved.length; i++) { for (let i = 0; i < inst.stack.maybeRemoved.length; i++) {
const maybeRemovedParent = inst.stack.maybeRemoved[i] const maybeRemovedParent = inst.stack.maybeRemoved[i]
if (frame.child.parents.indexOf(maybeRemovedParent) === -1) { if (!inst.stack.child.parentSet.has(maybeRemovedParent)) {
detach(maybeRemovedParent, frame.child) detach(maybeRemovedParent, frame.child)
} }
} }
@ -86,34 +81,33 @@ export function stopCapturingParents() {
// this must be called after the parent is up to date // this must be called after the parent is up to date
export function maybeCaptureParent(p: Signal<any, any>) { export function maybeCaptureParent(p: Signal<any, any>) {
if (inst.stack) { if (inst.stack) {
const idx = inst.stack.child.parents.indexOf(p) const wasCapturedAlready = inst.stack.child.parentSet.has(p)
// if the child didn't deref this parent last time it executed, then idx will be -1 // if the child didn't deref this parent last time it executed, then idx will be -1
// if the child did deref this parent last time but in a different order relative to other parents, then idx will be greater than stack.offset // if the child did deref this parent last time but in a different order relative to other parents, then idx will be greater than stack.offset
// if the child did deref this parent last time in the same order, then idx will be the same as stack.offset // if the child did deref this parent last time in the same order, then idx will be the same as stack.offset
// if the child did deref this parent already during this capture session then 0 <= idx < stack.offset // if the child did deref this parent already during this capture session then 0 <= idx < stack.offset
if (idx < 0) { if (wasCapturedAlready) {
inst.stack.numNewParents++ return
if (inst.stack.child.isActivelyListening) { }
attach(p, inst.stack.child)
inst.stack.child.parentSet.add(p)
if (inst.stack.child.isActivelyListening) {
attach(p, inst.stack.child)
}
if (inst.stack.offset < inst.stack.child.parents.length) {
const maybeRemovedParent = inst.stack.child.parents[inst.stack.offset]
if (!inst.stack.maybeRemoved) {
inst.stack.maybeRemoved = [maybeRemovedParent]
} else {
inst.stack.maybeRemoved.push(maybeRemovedParent)
} }
} }
if (idx < 0 || idx >= inst.stack.offset) { inst.stack.child.parents[inst.stack.offset] = p
if (idx !== inst.stack.offset && idx > 0) { inst.stack.child.parentEpochs[inst.stack.offset] = p.lastChangedEpoch
const maybeRemovedParent = inst.stack.child.parents[inst.stack.offset] inst.stack.offset++
if (!inst.stack.maybeRemoved) {
inst.stack.maybeRemoved = [maybeRemovedParent]
} else if (inst.stack.maybeRemoved.indexOf(maybeRemovedParent) === -1) {
inst.stack.maybeRemoved.push(maybeRemovedParent)
}
}
inst.stack.child.parents[inst.stack.offset] = p
inst.stack.child.parentEpochs[inst.stack.offset] = p.lastChangedEpoch
inst.stack.offset++
}
} }
} }

View file

@ -51,8 +51,9 @@ export interface Signal<Value, Diff = unknown> {
/** @internal */ /** @internal */
export type Child = { export type Child = {
lastTraversedEpoch: number lastTraversedEpoch: number
parents: Signal<any, any>[] readonly parentSet: ArraySet<Signal<any, any>>
parentEpochs: number[] readonly parents: Signal<any, any>[]
readonly parentEpochs: number[]
isActivelyListening: boolean isActivelyListening: boolean
} }