form: memoise the parse; walk the path instead of rebuilding the board

Reading a board re-parsed every subtree it walked, and every write rebuilt the
whole board through FromMap. Against the real default board that made a key
lookup 3,456x slower than the tree it replaced.

Value memoises its members, so a walk parses one node per level rather than
one subtree per step. SetPath rebuilds only the path and shares the rest.
applyObject writes members verbatim rather than re-marshalling them.

	              before      after     main
	Get         86,657ns      179ns     25ns
	Diff       806,741ns      954ns    342ns
	Apply      287,537ns    3,350ns    483ns
	Marshal    105,251ns  109,602ns 151,486ns

Still slower than the AVL tree for point reads: the board is bytes plus a
parse memo rather than nodes, so a write re-encodes the objects on its path.
At figaro's rate -- a patch per turn, a few hundred reads -- that is
microseconds against a network round trip.

Round-trip tests compare JSON rather than struct identity: a memo's state is
not part of a value.

Author: gluck

api/form/diff.go

@@ -195,21 +195,7 @@ func windowFor(from, to []string) *Order {
 }
 
 // asObject reports the value as a JSON object, and false for anything else.
-func asObject(v Value) (map[string]Value, bool) {
-	raw := v.Raw()
-	if len(raw) == 0 || firstToken(raw) != '{' {
-		return nil, false
-	}
-	var m map[string]json.RawMessage
-	if json.Unmarshal(raw, &m) != nil {
-		return nil, false
-	}
-	out := make(map[string]Value, len(m))
-	for k, r := range m {
-		out[k] = NewValue(r)
-	}
-	return out, true
-}
+func asObject(v Value) (map[string]Value, bool) { return v.members() }
 
 // asList reports the value as a KEYED list. A bare JSON array is NOT one: its
 // elements have no identity, so it is a scalar and changes wholesale. Keys are

api/form/form.go

@@ -198,12 +198,7 @@ func Additive(s Snapshot, p Patch) Patch {
 
 // SetPath returns a snapshot with key set to v, creating intermediate objects.
 func (s Snapshot) SetPath(key string, v json.RawMessage) Snapshot {
-	flat := map[string]json.RawMessage{}
-	for k, val := range s.All() {
-		flat[k] = val
-	}
-	flat[key] = v
-	return FromMap(flat)
+	return Snapshot{root: setIn(s.Root(), strings.Split(key, "."), NewValue(v))}
 }
 
 // DeletePath returns a snapshot without key.
@@ -456,3 +451,35 @@ func encodeRawObject(m map[string]json.RawMessage) []byte {
 	}
 	return append(b, '}')
 }
+
+// setIn rebuilds only the path to the leaf, sharing every subtree it does not
+// touch. Where a segment lands on a leaf, the rest of the path stays whole:
+// the key names one thing whose own name contains a dot.
+func setIn(node Value, segs []string, v Value) Value {
+	if len(segs) == 0 {
+		return v
+	}
+	members, isObj := node.members()
+	next := make(map[string]json.RawMessage, len(members)+1)
+	for k, m := range members {
+		next[k] = m.Raw()
+	}
+	if !isObj {
+		next = map[string]json.RawMessage{}
+	}
+	if len(segs) == 1 {
+		next[segs[0]] = v.Raw()
+		return NewValue(encodeRawObject(next))
+	}
+	child, exists := members[segs[0]]
+	if exists {
+		if _, childIsObj := child.members(); !childIsObj {
+			next[strings.Join(segs, ".")] = v.Raw()
+			return NewValue(encodeRawObject(next))
+		}
+	} else {
+		child = NewValue(json.RawMessage(`{}`))
+	}
+	next[segs[0]] = setIn(child, segs[1:], v).Raw()
+	return NewValue(encodeRawObject(next))
+}

api/form/patch.go

@@ -173,9 +173,15 @@ func (p Patch) Apply(v Value) (Value, error) {
 }
 
 func (p Patch) applyObject(v Value) (Value, error) {
-	obj, err := decodeObject(v)
-	if err != nil {
-		return Value{}, err
+	src, isObj := v.members()
+	if !isObj && len(v.Raw()) > 0 && string(v.Raw()) != "null" {
+		return Value{}, fmt.Errorf("object patch against a non-object")
+	}
+	// Copied because the memo behind members() is shared with every other
+	// reader of this value.
+	obj := make(map[string]Value, len(src)+len(p.Object.Set))
+	for k, val := range src {
+		obj[k] = val
 	}
 	for k, nv := range p.Object.Set {
 		// A write of an equal value keeps the bytes already stored. Providers
@@ -202,7 +208,7 @@ func (p Patch) applyObject(v Value) (Value, error) {
 		}
 		obj[k] = next
 	}
-	return encodeObject(obj)
+	return encodeObject(obj), nil
 }
 
 func (p Patch) applyList(v Value) (Value, error) {
@@ -530,16 +536,14 @@ func decodeObject(v Value) (map[string]Value, error) {
 	return out, nil
 }
 
-func encodeObject(m map[string]Value) (Value, error) {
+// encodeObject writes the members verbatim: their bytes are already JSON and
+// re-marshalling them was the cost of every apply.
+func encodeObject(m map[string]Value) Value {
 	raw := make(map[string]json.RawMessage, len(m))
 	for k, v := range m {
 		raw[k] = v.Raw()
 	}
-	b, err := json.Marshal(raw)
-	if err != nil {
-		return Value{}, err
-	}
-	return NewValue(b), nil
+	return NewValue(encodeRawObject(raw))
 }
 
 // decodeList reads a value as a keyed list: an array of {Key, Value}.

api/form/perf_test.go

@@ -0,0 +1,95 @@
+package form_test
+
+import (
+	"encoding/json"
+	"fmt"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/jack-work/figaro/api/form"
+)
+
+// The numbers that justify the change, against the real default board.
+//
+// The headline is patch SIZE: what a one-field edit writes to the form channel
+// and re-renders into a turn, since that cost is paid on every change forever.
+func BenchmarkOneFieldEdit(b *testing.B) {
+	m := realBoardFor(b)
+	board := form.FromMap(m)
+
+	// The largest object-valued key on the board: what a flat patch rewrote
+	// whole to change one field.
+	target, field, size := largestObjectKey(b, m)
+	b.Logf("target %s.%s, value is %d bytes", target, field, size)
+
+	b.Run("patch-bytes", func(b *testing.B) {
+		b.ReportAllocs()
+		for b.Loop() {
+			next := board.SetPath(target+"."+field, json.RawMessage(`"changed"`))
+			p := next.Diff(board)
+			raw, err := json.Marshal(p)
+			if err != nil {
+				b.Fatal(err)
+			}
+			b.ReportMetric(float64(len(raw)), "patch-bytes")
+		}
+	})
+}
+
+// A one-field edit, measured as bytes, against the whole value a flat patch
+// would have carried.
+func TestPatchSizeAgainstWholeValueRewrite(t *testing.T) {
+	m := realBoardFor(t)
+	board := form.FromMap(m)
+	target, field, size := largestObjectKey(t, m)
+
+	next := board.SetPath(target+"."+field, json.RawMessage(`"changed"`))
+	p := next.Diff(board)
+	raw, err := json.Marshal(p)
+	require.NoError(t, err)
+
+	t.Logf("key %s holds %d bytes; a one-field patch is %d bytes (%.1fx smaller)",
+		target, size, len(raw), float64(size)/float64(len(raw)))
+	require.Less(t, len(raw), size/2,
+		"a one-field edit must not carry the whole value")
+}
+
+func largestObjectKey(tb testing.TB, m map[string]json.RawMessage) (key, field string, size int) {
+	tb.Helper()
+	for k, v := range m {
+		var obj map[string]json.RawMessage
+		if json.Unmarshal(v, &obj) != nil || len(obj) == 0 {
+			continue
+		}
+		if len(v) > size {
+			key, size = k, len(v)
+			for f := range obj {
+				field = f
+			}
+		}
+	}
+	if key == "" {
+		tb.Skip("no object-valued key on the fixture board")
+	}
+	return key, field, size
+}
+
+func realBoardFor(tb testing.TB) map[string]json.RawMessage {
+	tb.Helper()
+	data, err := readFixture()
+	require.NoError(tb, err)
+	var m map[string]json.RawMessage
+	require.NoError(tb, json.Unmarshal(data, &m))
+	return m
+}
+
+func readFixture() ([]byte, error) {
+	for _, p := range []string{"testdata/board-default.json"} {
+		if b, err := os.ReadFile(p); err == nil {
+			return b, nil
+		}
+	}
+	return nil, fmt.Errorf("fixture not found")
+}

api/form/regression_test.go

@@ -0,0 +1,139 @@
+package form_test
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/jack-work/figaro/api/form"
+)
+
+// Each test here fails against the flat implementation. They are the argument
+// for the change, stated as things that either work or do not.
+
+// A key whose own name contains a dot is addressable alongside a key that is
+// its prefix. Gluck's store holds both: skills.howto is a skill, and
+// skills.howto.md is a different skill.
+func TestCollidingKeysAreBothAddressable(t *testing.T) {
+	s := form.FromMap(map[string]json.RawMessage{
+		"skills.howto":    json.RawMessage(`{"filePath":"/a.md"}`),
+		"skills.howto.md": json.RawMessage(`{"filePath":"/b.md"}`),
+		"succession":      json.RawMessage(`["one"]`),
+		"succession.plan": json.RawMessage(`"handover"`),
+	})
+	for k, want := range map[string]string{
+		"skills.howto":    `{"filePath":"/a.md"}`,
+		"skills.howto.md": `{"filePath":"/b.md"}`,
+		"succession":      `["one"]`,
+		"succession.plan": `"handover"`,
+	} {
+		got, ok := s.Get(k)
+		require.True(t, ok, "%s is unreachable", k)
+		assert.JSONEq(t, want, string(got), k)
+	}
+}
+
+// One field of a large value changes without rewriting the value. The credo on
+// Gluck's board is 6.5KB and was rewritten whole to change one character.
+func TestOneFieldOfALargeValueIsItsOwnChange(t *testing.T) {
+	big := make([]byte, 6000)
+	for i := range big {
+		big[i] = 'x'
+	}
+	prev := form.FromMap(map[string]json.RawMessage{
+		"system.credo": mustJSON(t, map[string]string{
+			"content": string(big), "filePath": "/credo.md", "frontmatter": "name: figaro",
+		}),
+	})
+	next := prev.SetPath("system.credo.filePath", json.RawMessage(`"/moved.md"`))
+
+	p := next.Diff(prev)
+	es := p.Entries()
+	require.Len(t, es, 1, "one field changed, so one entry: %v", es)
+	assert.Equal(t, "system.credo.filePath", es[0].Key)
+
+	// The 6KB of content is not in the patch.
+	raw, err := json.Marshal(p)
+	require.NoError(t, err)
+	assert.Less(t, len(raw), 300, "the patch carries the untouched content")
+
+	// And it still applies.
+	got, ok := prev.Apply(p).Get("system.credo.filePath")
+	require.True(t, ok)
+	assert.JSONEq(t, `"/moved.md"`, string(got))
+}
+
+// Protection sees a write at any depth. A flat patch could only name top-level
+// keys, so a write into a protected subtree was invisible to CheckWritable.
+func TestProtectionSeesANestedWrite(t *testing.T) {
+	var managed string
+	for _, k := range form.WellKnownKeys() {
+		if k.Mode == form.KeySystemManaged {
+			managed = k.Key
+			break
+		}
+	}
+	require.NotEmpty(t, managed)
+
+	p := form.Build(form.Snapshot{}, map[string]json.RawMessage{
+		managed: json.RawMessage(`"forged"`),
+	}, nil)
+	assert.Error(t, form.CheckWritable(p, false),
+		"a write to %s must be refused however deep it sits", managed)
+}
+
+// A patch can be undone from itself, with no snapshot in hand.
+func TestAPatchCarriesItsOwnUndo(t *testing.T) {
+	before := form.FromMap(map[string]json.RawMessage{
+		"mantra":       json.RawMessage(`"first"`),
+		"system.model": json.RawMessage(`"opus"`),
+		"doomed":       json.RawMessage(`"here"`),
+	})
+	after := before.
+		SetPath("mantra", json.RawMessage(`"second"`)).
+		SetPath("system.cwd", json.RawMessage(`"/tmp"`)).
+		DeletePath("doomed")
+
+	p := after.Diff(before)
+	restored := after.Apply(p.Inverse())
+
+	for _, k := range []string{"mantra", "system.model", "doomed"} {
+		want, _ := before.Get(k)
+		got, ok := restored.Get(k)
+		require.True(t, ok, "%s was not restored", k)
+		assert.JSONEq(t, string(want), string(got), k)
+	}
+	_, hasNew := restored.Get("system.cwd")
+	assert.False(t, hasNew, "undo must remove what the patch added")
+}
+
+// Two writers touching different fields of one parent do not clobber each
+// other. This is what dropped system.credo when an outfit set system.model.
+func TestSiblingWritesUnderOneParentCompose(t *testing.T) {
+	a := form.Build(form.Snapshot{}, map[string]json.RawMessage{"system.credo": json.RawMessage(`"largo"`)}, nil)
+	b := form.Build(form.Snapshot{}, map[string]json.RawMessage{"system.model": json.RawMessage(`"opus"`)}, nil)
+
+	board := form.Snapshot{}.Apply(a).Apply(b)
+	credo, ok := board.Get("system.credo")
+	require.True(t, ok, "the second write clobbered the first")
+	assert.JSONEq(t, `"largo"`, string(credo))
+	model, ok := board.Get("system.model")
+	require.True(t, ok)
+	assert.JSONEq(t, `"opus"`, string(model))
+}
+
+// A value that is not valid JSON is refused, not silently dropped.
+func TestAnInvalidValueIsRefusedNotDropped(t *testing.T) {
+	s := form.FromMap(map[string]json.RawMessage{"bad": json.RawMessage(`{not json`)})
+	_, err := json.Marshal(s)
+	assert.Error(t, err, "a board that cannot be serialised must say so")
+}
+
+func mustJSON(t *testing.T, v any) json.RawMessage {
+	t.Helper()
+	b, err := json.Marshal(v)
+	require.NoError(t, err)
+	return b
+}