Skip to content

Commit

Permalink
Adds RemoveInput to MapN (#26)
Browse files Browse the repository at this point in the history

The goal here is to allow, in addition to "Adding" inputs dynamically, "Removing" inputs dynamically from MapN nodes.

The key implications of removing nodes:

    We need to make sure that we remove the input as a parent, and the map node as a child, from the removed node
    We need to make sure the map node is marked as stale and is recomputed
    We need to make sure that the removed node is detected as "unnecessary" and removed from the graph if the map node was its only child

Things we don't need to do in practice:

    Because inputs are "parents", we don't need to invalidate or propagate invalidity for the node; this extends from the fact that if a map node is "observed" it will continue to be observed regardless of what its children are.
    We do not need to update the map node's height, even if logically it would have changed, because it will never get lower in practice (and staying the same, even if strictly higher than it should be, is ok)
  • Loading branch information
wcharczuk authored Oct 27, 2024
1 parent 64cf7b3 commit 5ed6154
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 8 deletions.
8 changes: 5 additions & 3 deletions list_util.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package incr

func remove[A INode](nodes []A, id Identifier) []A {
output := make([]A, 0, len(nodes))
func remove[A INode](nodes []A, id Identifier) (output []A, removed A) {
output = make([]A, 0, len(nodes))
for _, n := range nodes {
if n.Node().id != id {
output = append(output, n)
} else {
removed = n
}
}
return output
return
}
4 changes: 3 additions & 1 deletion list_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ func Test_remove(t *testing.T) {
nodes := []INode{
n0, n1, n2,
}
nodes = remove(nodes, n1.Node().id)
var removed INode
nodes, removed = remove(nodes, n1.Node().id)

testutil.Equal(t, 2, len(nodes))
testutil.NotNil(t, removed)
}
14 changes: 14 additions & 0 deletions map_n.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type MapNContextFunc[A, B any] func(context.Context, ...A) (B, error)
type MapNIncr[A, B any] interface {
Incr[B]
AddInput(Incr[A]) error
RemoveInput(Identifier) error
}

var (
Expand Down Expand Up @@ -68,6 +69,19 @@ func (mn *mapNIncr[A, B]) AddInput(i Incr[A]) error {
return nil
}

func (mn *mapNIncr[A, B]) RemoveInput(id Identifier) error {
var removed Incr[A]
mn.inputs, removed = remove(mn.inputs, id)
if removed != nil {
mn.Node().removeParent(id)
removed.Node().removeChild(mn.n.id)
GraphForNode(mn).SetStale(mn)
GraphForNode(mn).checkIfUnnecessary(removed)
return nil
}
return nil
}

func (mn *mapNIncr[A, B]) Node() *Node { return mn.n }

func (mn *mapNIncr[A, B]) Value() B { return mn.val }
Expand Down
87 changes: 87 additions & 0 deletions map_n_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,90 @@ func sum[A ~int | ~float64](values ...A) (out A) {
}
return
}

func Test_MapN_RemoveInput(t *testing.T) {
ctx := testContext()
g := New()

r0 := Return(g, 1)
r1 := Return(g, 2)
mn := MapN(g, sum, r0, r1)
om := MustObserve(g, mn)

r2 := Return(g, 3)
err := mn.AddInput(r2)
testutil.NoError(t, err)

err = g.Stabilize(ctx)
testutil.NoError(t, err)
testutil.Equal(t, 6, om.Value())

err = mn.RemoveInput(r1.Node().ID())
testutil.NoError(t, err)

testutil.Equal(t, 2, len(mn.Node().parents))

err = g.Stabilize(ctx)
testutil.NoError(t, err)
testutil.Equal(t, 4, om.Value())

hasR1 := g.Has(r1)
testutil.Equal(t, false, hasR1)
}

func Test_MapN_RemoveInput_onlyInput(t *testing.T) {
ctx := testContext()
g := New()

mn := MapN[int](g, sum)
om := MustObserve(g, mn)

r2 := Return(g, 3)
err := mn.AddInput(r2)
testutil.NoError(t, err)

err = g.Stabilize(ctx)
testutil.NoError(t, err)
testutil.Equal(t, 3, om.Value())

err = mn.RemoveInput(r2.Node().ID())
testutil.NoError(t, err)

testutil.Equal(t, 0, len(mn.Node().parents))

err = g.Stabilize(ctx)
testutil.NoError(t, err)
testutil.Equal(t, 0, om.Value())

hasR2 := g.Has(r2)
testutil.Equal(t, false, hasR2)
}

func Test_MapN_RemoveInput_heightUpdates(t *testing.T) {
ctx := testContext()
g := New()

r0 := Return(g, 2)
m0 := Map[int](g, r0, ident)

r1 := Return(g, 1)

mn := MapN[int](g, sum, r1)
om := MustObserve(g, mn)

err := mn.AddInput(m0)
testutil.NoError(t, err)

err = g.Stabilize(ctx)
testutil.NoError(t, err)
testutil.Equal(t, 3, om.Value())
testutil.Equal(t, 2, mn.Node().height)

err = mn.RemoveInput(m0.Node().ID())
testutil.NoError(t, err)

err = g.Stabilize(ctx)
testutil.NoError(t, err)
testutil.Equal(t, 1, om.Value())
testutil.Equal(t, 2, mn.Node().height, "the height should stay the same as strictly it shouldn't get smaller, but staying higher is fine")
}
8 changes: 4 additions & 4 deletions node.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,19 +218,19 @@ func (n *Node) addSentinels(sentinels ...ISentinel) {
}

func (n *Node) removeChild(id Identifier) {
n.children = remove(n.children, id)
n.children, _ = remove(n.children, id)
}

func (n *Node) removeParent(id Identifier) {
n.parents = remove(n.parents, id)
n.parents, _ = remove(n.parents, id)
}

func (n *Node) removeObserver(id Identifier) {
n.observers = remove(n.observers, id)
n.observers, _ = remove(n.observers, id)
}

func (n *Node) removeSentinel(id Identifier) {
n.sentinels = remove(n.sentinels, id)
n.sentinels, _ = remove(n.sentinels, id)
}

// maybeCutoff calls the cutoff delegate if it's set, otherwise
Expand Down

0 comments on commit 5ed6154

Please sign in to comment.