Skip to content

Commit

Permalink
use iterative approach in recompute dependents
Browse files Browse the repository at this point in the history
  • Loading branch information
dmaskasky committed Nov 15, 2024
1 parent d9091a1 commit baa1bc5
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 14 deletions.
44 changes: 30 additions & 14 deletions src/vanilla/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -454,24 +454,40 @@ const buildStore = (
epochNumber: number,
])[] = []
const markedAtoms = new Set<AnyAtom>()
const visit = (a: AnyAtom, aState: AtomState) => {
if (markedAtoms.has(a)) {
return
// 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: Array<{ a: AnyAtom; aState: AtomState; visited: boolean }> = [
{ a: atom, aState: atomState, visited: false },
]
while (stack.length > 0) {
const current = stack[stack.length - 1]!
const { a, aState } = current

if (current.visited) {
// All dependents have been processed, now process this atom
stack.pop()
// 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])
continue
}
markedAtoms.add(a)
for (const [d, s] of getDependents(pending, a, aState)) {
if (a !== d) {
visit(d, s)

if (!markedAtoms.has(a)) {
markedAtoms.add(a)
current.visited = true

// Push unvisited dependents onto the stack
for (const [d, s] of getDependents(pending, a, aState)) {
if (a !== d && !markedAtoms.has(d)) {
stack.push({ a: d, aState: s, visited: false })
}
}
} else {
// Atom has been visited but not yet processed
current.visited = true
}
// 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)
// 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
40 changes: 40 additions & 0 deletions tests/vanilla/store.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -876,3 +876,43 @@ describe('should mount and trigger listeners even when an error is thrown', () =
expect(listener).toHaveBeenCalledOnce()
})
})

it('processes deep atom a graph beyond maxDepth', () => {
function getMaxDepth() {
let depth = 0
function d() {

Check failure on line 883 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (5.0.4)

'd' implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions.

Check failure on line 883 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.9.5)

'd' implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions.

Check failure on line 883 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.8.4)

'd' implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions.

Check failure on line 883 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.7.4)

'd' implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions.

Check failure on line 883 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.6.4)

'd' implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions.

Check failure on line 883 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.5.5)

'd' implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions.

Check failure on line 883 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.4.4)

'd' implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions.

Check failure on line 883 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.3.5)

'd' implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions.
++depth

Check failure on line 884 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.2.3)

'd' implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions.

Check failure on line 884 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.1.5)

'd' implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions.

Check failure on line 884 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (4.0.5)

'd' implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions.

Check failure on line 884 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (3.9.7)

'd' implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions.

Check failure on line 884 in tests/vanilla/store.test.tsx

View workflow job for this annotation

GitHub Actions / test_matrix (3.8.3)

'd' implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions.
try {
return d()
} catch (error) {
return depth
}
}
return d()
}
const maxDepth = getMaxDepth() + 1
const store = createStore()
const baseAtom = atom(0)
const atoms = [
atom(
(get) => get(baseAtom),
(_, set) => set(baseAtom, (v) => ++v),
),
]
Array.from({ length: maxDepth }, (_, i) => {
const prevAtom = atoms[i]!
const a = atom(
(get) => get(prevAtom),
(get, set) => (get(prevAtom), set(prevAtom)),
)
a.onMount = () => () => {}
atoms.push(a)
store.sub(a, () => {})
})
const lastAtom = atoms[maxDepth]!
expect(() => store.get(lastAtom)).not.toThrow()
expect(() => store.sub(lastAtom, () => {})).not.toThrow()
expect(() => store.set(baseAtom, 1)).not.toThrow()
expect(() => store.get(lastAtom)).not.toThrow()
// store.set(lastAtom) // FIXME: This is causing a stack overflow
})

0 comments on commit baa1bc5

Please sign in to comment.