Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(core): Use iterative approach in recompute dependents #2821

Merged
merged 1 commit into from
Nov 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 49 additions & 28 deletions src/vanilla/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -439,41 +439,62 @@ const buildStore = (
return dependents
}

// This is a topological sort via depth-first search, slightly modified from
// what's described here for simplicity and performance reasons:
// https://en.wikipedia.org/wiki/Topological_sorting#Depth-first_search
function getSortedDependents(
pending: Pending,
rootAtom: AnyAtom,
rootAtomState: AtomState,
): [[AnyAtom, AtomState, number][], Set<AnyAtom>] {
const sorted: [atom: AnyAtom, atomState: AtomState, epochNumber: number][] =
[]
const visiting = new Set<AnyAtom>()
const visited = new Set<AnyAtom>()
// Visit the root atom. This is the only atom in the dependency graph
// without incoming edges, which is one reason we can simplify the algorithm
const stack: [a: AnyAtom, aState: AtomState][] = [[rootAtom, rootAtomState]]
while (stack.length > 0) {
const [a, aState] = stack[stack.length - 1]!
if (visited.has(a)) {
// All dependents have been processed, now process this atom
stack.pop()
continue
}
if (visiting.has(a)) {
// The algorithm calls for pushing onto the front of the list. For
// performance, we will simply push onto the end, and then will iterate in
// reverse order later.
sorted.push([a, aState, aState.n])
// Atom has been visited but not yet processed
visited.add(a)
stack.pop()
continue
}
visiting.add(a)
// Push unvisited dependents onto the stack
for (const [d, s] of getDependents(pending, a, aState)) {
if (a !== d && !visiting.has(d)) {
stack.push([d, s])
}
}
}
return [sorted, visited]
}

const recomputeDependents = <Value>(
pending: Pending,
atom: Atom<Value>,
atomState: AtomState<Value>,
) => {
// This is a topological sort via depth-first search, slightly modified from
// what's described here for simplicity and performance reasons:
// https://en.wikipedia.org/wiki/Topological_sorting#Depth-first_search

// Step 1: traverse the dependency graph to build the topsorted atom list
// We don't bother to check for cycles, which simplifies the algorithm.
const topsortedAtoms: (readonly [
atom: AnyAtom,
atomState: AtomState,
epochNumber: number,
])[] = []
const markedAtoms = new Set<AnyAtom>()
const visit = (a: AnyAtom, aState: AtomState) => {
if (markedAtoms.has(a)) {
return
}
markedAtoms.add(a)
for (const [d, s] of getDependents(pending, a, aState)) {
if (a !== d) {
visit(d, s)
}
}
// The algorithm calls for pushing onto the front of the list. For
// performance, we will simply push onto the end, and then will iterate in
// reverse order later.
topsortedAtoms.push([a, aState, aState.n])
}
// Visit the root atom. This is the only atom in the dependency graph
// without incoming edges, which is one reason we can simplify the algorithm
visit(atom, atomState)
const [topsortedAtoms, markedAtoms] = getSortedDependents(
pending,
atom,
atomState,
)

// Step 2: use the topsorted atom list to recompute all affected atoms
// Track what's changed, so that we can short circuit when possible
const changedAtoms = new Set<AnyAtom>([atom])
Expand Down
32 changes: 31 additions & 1 deletion tests/vanilla/store.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { waitFor } from '@testing-library/react'
import { assert, describe, expect, it, vi } from 'vitest'
import { atom, createStore } from 'jotai/vanilla'
import type { Getter } from 'jotai/vanilla'
import type { Atom, Getter, PrimitiveAtom } from 'jotai/vanilla'

it('should not fire on subscribe', async () => {
const store = createStore()
Expand Down Expand Up @@ -933,3 +933,33 @@ it('should call subscribers after setAtom updates atom value on mount but not on
expect(unmount).toHaveBeenCalledTimes(1)
expect(listener).toHaveBeenCalledTimes(0)
})

it('processes deep atom a graph beyond maxDepth', () => {
function getMaxDepth() {
let depth = 0
function d(): number {
++depth
try {
return d()
} catch (error) {
return depth
}
}
return d()
}
const maxDepth = getMaxDepth()
dmaskasky marked this conversation as resolved.
Show resolved Hide resolved
const store = createStore()
const baseAtom = atom(0)
const atoms: [PrimitiveAtom<number>, ...Atom<number>[]] = [baseAtom]
Array.from({ length: maxDepth }, (_, i) => {
const prevAtom = atoms[i]!
const a = atom((get) => get(prevAtom))
atoms.push(a)
})
const lastAtom = atoms[maxDepth]!
// store.get(lastAtom) // FIXME: This is causing a stack overflow
expect(() => store.sub(lastAtom, () => {})).not.toThrow()
// store.get(lastAtom) // FIXME: This is causing a stack overflow
expect(() => store.set(baseAtom, 1)).not.toThrow()
// store.set(lastAtom) // FIXME: This is causing a stack overflow
})