variablelab/theatre
4
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Massive perf-gain in @theatre/react

Browse source 
Remove cold derivation reads

Prior to this commit, the first render of every `useDerivation()` resulted in a cold read of its inner derivation. Cold reads are predictably slow. The reason we'd run cold reads was to comply with react's rule of not running side-effects during render. (Turning a derivation hot is _technically_ a side-effect).

However, now that users are animating scenes with hundreds of objects in the same sequence, the lag started to be noticable.

This commit changes `useDerivation()` so that it turns its derivation hot before rendering them.

Freshen derivations before render

Previously in order to avoid the zombie child problem (https://kaihao.dev/posts/stale-props-and-zombie-children-in-redux) we deferred freshening the derivations to the render phase of components. This meant that if a derivation's dependencies changed, `useDerivation()` would schedule a re-render, regardless of whether that change actually affected the derivation's value. Here is a contrived example:

```
const num = new Box(1)
const isPositiveD = prism(() => num.derivation.getValue() >= 0)

const Comp = () => {
  return <div>{useDerivation(isPositiveD)}</div>
}

num.set(2) // would cause Comp to re-render- even though 1 is still a positive number
```

We now avoid this problem by freshening the derivation (i.e. calling `der.getValue()`) inside `runQueue()`, and then only causing a re-render if the derivation's value is actually changed.

This still avoids the zombie-child problem because `runQueue` reads the derivations in-order of their position in  the mounting tree.

On the off-chance that one of them still turns out to be a zombile child, `runQueue` will defer that particular `useDerivation()` to be read inside a normal react render phase.
This commit is contained in:
Aria Minaei 2022-07-09 00:52:47 +02:00 committed by Aria
parent cbfc8861b5
commit 667f9d4fa2
1 changed files with 201 additions and 75 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

276
packages/react/src/index.ts
View file

@ -22,15 +22,16 @@ import {unstable_batchedUpdates} from 'react-dom'
type $IntentionalAny = any
type VoidFn = () => void
const logger = {log: console.log}
/**
* Enables a few traces and debug points to help identify performance glitches in `@theatre/react`.
* Look up references to this value to see how to make use of those traces.
*/
const TRACE: boolean = false && process.env.NODE_ENV !== 'production'
function useForceUpdate(debugLabel?: string) {
const [, setTick] = useState(0)
const update = useCallback(() => {
if (process.env.NODE_ENV !== 'production' && debugLabel)
logger.log(debugLabel, 'forceUpdate', {trace: new Error()})
setTick((tick) => tick + 1)
}, [])
@ -94,13 +95,72 @@ let lastOrder = 0
const queue: QueueItem[] = []
const setOfQueuedItems = new Set<QueueItem>()
type QueueItem = {
type QueueItem<T = unknown> = {
order: number
/**
* runUpdate() is the equivalent of a forceUpdate() call.
* runUpdate() is the equivalent of a forceUpdate() call. It would only be called
* if the value of the inner derivation has _actually_ changed.
*/
runUpdate: VoidFn
debugLabel?: string
/**
* Some debugging info that are only present if {@link TRACE} is true
*/
debug?: {
/**
* The `debugLabel` given to `usePrism()/useDerivation()`
*/
label?: string
/**
* A trace of the first time the component got rendered
*/
traceOfFirstTimeRender: Error
/**
* An array of the operations done on/about this useDerivation. This is helpful to trace
* why a useDerivation's update was added to the queue and why it re-rendered
*/
history: Array<
/**
* Item reached its turn in the queue
*/
| `queue reached`
/**
* Item reached its turn in the queue, and errored (likely something in `prism()` threw an error)
*/
| `queue: der.getValue() errored`
/**
* The item was added to the queue (may be called multiple times, but will only queue once)
*/
| `queueUpdate()`
/**
* `cb` in `item.der.changesWithoutValues(cb)` was called
*/
| `changesWithoutValues(cb)`
/**
* Item was rendered
*/
| `rendered`
>
}
/**
* A reference to the derivation
*/
der: IDerivation<T>
/**
* The last value of this derivation.
*/
lastValue: T
/**
* Would be set to true if the element hosting the `useDerivation()` was unmounted
*/
unmounted: boolean
/**
* Adds the `useDerivation` to the update queue
*/
queueUpdate: () => void
/**
* Untaps from `this.der.changesWithoutValues()`
*/
untap: () => void
}
let microtaskIsQueued = false
@ -132,6 +192,9 @@ const _pushToQueue = (item: QueueItem) => {
}
}
/**
* Plucks items from the queue
*/
const removeFromQueue = (item: QueueItem) => {
if (!setOfQueuedItems.has(item)) return
setOfQueuedItems.delete(item)
@ -141,31 +204,51 @@ const removeFromQueue = (item: QueueItem) => {
}
function queueIfNeeded() {
if (!microtaskIsQueued) {
microtaskIsQueued = true
queueMicrotask(() => {
let i = 0
while (queue.length > 0) {
i++
if (i === 4) {
// react might be skipping updates, perhaps in concurrent mode.
//we can recheck the queue later
setTimeout(queueIfNeeded, 1)
break
}
unstable_batchedUpdates(() => {
for (const item of queue) {
item.runUpdate()
}
}, 1)
}
microtaskIsQueued = false
})
}
}
if (microtaskIsQueued) return
microtaskIsQueued = true
queueMicrotask(() => {
unstable_batchedUpdates(function runQueue() {
while (queue.length > 0) {
const item = queue.shift()!
setOfQueuedItems.delete(item)
let newValue
if (TRACE) {
item.debug?.history.push(`queue reached`)
}
try {
newValue = item.der.getValue()
} catch (error) {
if (TRACE) {
item.debug?.history.push(`queue: der.getValue() errored`)
}
console.error(
'A `der.getValue()` in `useDerivation(der)` threw an error. ' +
"This may be a zombie child issue, so we're gonna try to get its value again in a normal react render phase." +
'If you see the same error again, then you either have an error in your prism code, or the deps array in `usePrism(fn, deps)` is missing ' +
'a dependency and causing the prism to read stale values.',
)
console.error(error)
item.runUpdate()
continue
}
if (newValue !== item.lastValue) {
item.lastValue = newValue
item.runUpdate()
}
}
}, 1)
microtaskIsQueued = false
})
}
/**
* A React hook that returns the value of the derivation that it received as the first argument. It works like an implementation of Dataverse's Ticker, except that it runs the side effects in an order where a component's derivation is guaranteed to run before any of its descendents' derivations.
* A React hook that returns the value of the derivation that it received as the first argument.
* It works like an implementation of Dataverse's Ticker, except that it runs the side effects in
* an order where a component's derivation is guaranteed to run before any of its descendents' derivations.
*
* @param der - The derivation
* @param debugLabel - The label used by the debugger
@ -178,73 +261,116 @@ function queueIfNeeded() {
*
*
* I'm happy with how little bookkeeping we ended up doing here.
*
* ---
*
* Notes on the latest implementation:
*
* # Remove cold derivation reads
*
* Prior to the latest change, the first render of every `useDerivation()` resulted in a cold read of its inner derivation.
* Cold reads are predictably slow. The reason we'd run cold reads was to comply with react's rule of not running side-effects
* during render. (Turning a derivation hot is _technically_ a side-effect).
*
* However, now that users are animating scenes with hundreds of objects in the same sequence, the lag started to be noticable.
*
* This commit changes `useDerivation()` so that it turns its derivation hot before rendering them.
*
* # Freshen derivations before render
*
* Previously in order to avoid the zombie child problem (https://kaihao.dev/posts/stale-props-and-zombie-children-in-redux)
* we deferred freshening the derivations to the render phase of components. This meant that if a derivation's dependencies
* changed, `useDerivation()` would schedule a re-render, regardless of whether that change actually affected the derivation's
* value. Here is a contrived example:
*
* ```ts
* const num = new Box(1)
* const isPositiveD = prism(() => num.derivation.getValue() >= 0)
*
* const Comp = () => {
* return <div>{useDerivation(isPositiveD)}</div>
* }
*
* num.set(2) // would cause Comp to re-render- even though 1 is still a positive number
* ```
*
* We now avoid this problem by freshening the derivation (i.e. calling `der.getValue()`) inside `runQueue()`,
* and then only causing a re-render if the derivation's value is actually changed.
*
* This still avoids the zombie-child problem because `runQueue` reads the derivations in-order of their position in
* the mounting tree.
*
* On the off-chance that one of them still turns out to be a zombile child, `runQueue` will defer that particular
* `useDerivation()` to be read inside a normal react render phase.
*/
export function useDerivation<T>(der: IDerivation<T>, debugLabel?: string): T {
const _forceUpdate = useForceUpdate(debugLabel)
const refs = useRef<{queueItem: QueueItem; unmounted: boolean}>(
undefined as $IntentionalAny,
)
const ref = useRef<QueueItem<T>>(undefined as $IntentionalAny)
if (!refs.current) {
if (!ref.current) {
lastOrder++
refs.current = {
queueItem: {
debugLabel,
order: lastOrder,
runUpdate: () => {
if (!refs.current.unmounted) {
_forceUpdate()
}
},
ref.current = {
order: lastOrder,
runUpdate: () => {
if (!ref.current.unmounted) {
_forceUpdate()
}
},
der,
lastValue: undefined as $IntentionalAny,
unmounted: false,
queueUpdate: () => {
if (TRACE) {
ref.current.debug?.history.push(`queueUpdate()`)
}
pushToQueue(ref.current)
},
untap: der.changesWithoutValues().tap(() => {
if (TRACE) {
ref.current.debug!.history.push(`changesWithoutValues(cb)`)
}
ref.current!.queueUpdate()
}),
}
if (TRACE) {
ref.current.debug = {
label: debugLabel,
traceOfFirstTimeRender: new Error(),
history: [],
}
}
}
const queueUpdate = useCallback(() => {
pushToQueue(refs.current.queueItem)
}, [])
useLayoutEffect(() => {
const untap = der.changesWithoutValues().tap(() => {
queueUpdate()
})
if (lastValueRef.current !== der.getValue()) {
queueUpdate()
if (process.env.NODE_ENV !== 'production') {
if (der !== ref.current.der) {
console.error(
'Argument `der` in `useDerivation(der)` should not change between renders.',
)
}
return untap
}, [der])
}
useLayoutEffect(() => {
return function onUnmount() {
refs.current.unmounted = true
removeFromQueue(refs.current.queueItem)
ref.current.unmounted = true
ref.current.untap()
removeFromQueue(ref.current)
}
}, [])
const lastValueRef = useRef<T>(undefined as $IntentionalAny as T)
const queueItem = refs.current.queueItem
// if we're queued but are rendering before our turn, remove us from the queue
removeFromQueue(ref.current)
// we defer refreshing our derivation if:
const mustDefer =
// we are actually queued to refresh
setOfQueuedItems.has(queueItem) &&
// but it's not our turn yet
queue[0] !== refs.current.queueItem
const newValue = ref.current.der.getValue()
ref.current.lastValue = newValue
if (!mustDefer) {
removeFromQueue(queueItem)
lastValueRef.current = der.getValue()
} else {
// if it's not our turn, we return the last cached value,
// which react will actually drop, because the microtask
// queue will make sure to forceUpdate() us before react
// flushes to DOM.
if (TRACE) {
ref.current.debug?.history.push(`rendered`)
}
return lastValueRef.current
return newValue
}
/**