From 90e06a9c1759d95517cddaedb92b00f114948bc8 Mon Sep 17 00:00:00 2001 From: Delicious233 <101502465+DeliciousBuding@users.noreply.github.com> Date: Sun, 6 Sep 2026 15:41:57 +0800 Subject: [PATCH 1/2] test(store): measure sparse and dense scoped list reads Refs #2337. Add real Store facade controls before choosing allocation changes. Co-authored-by: Codex --- .../internal/store/store_query_bench_test.go | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 edge-server/internal/store/store_query_bench_test.go diff --git a/edge-server/internal/store/store_query_bench_test.go b/edge-server/internal/store/store_query_bench_test.go new file mode 100644 index 000000000..0ee338d94 --- /dev/null +++ b/edge-server/internal/store/store_query_bench_test.go @@ -0,0 +1,102 @@ +package store + +import ( + "fmt" + "testing" +) + +// BenchmarkStoreScopedLists measures actual memory-store facades, not SQL or +// JSON encoding. The sparse scope has ten records; dense has the rest. Missing +// and unfiltered controls expose allocation tradeoffs hidden by sparse-only +// tests. No runtime request rate, RSS or response-retention claim is implied. +func BenchmarkStoreScopedLists(b *testing.B) { + for _, total := range []int{100, 1000} { + b.Run(fmt.Sprintf("resident=%d", total), func(b *testing.B) { + s := scopedListBenchmarkStore(b, total) + for _, scope := range []struct { + name, threadID, runID string + first, count int + }{ + {"missing", "missing", "missing", 0, 0}, + {"sparse", "sparse", "run-000000", 0, 10}, + {"dense", "dense", "run-000010", 10, total - 10}, + {"all", "", "", 0, total}, + } { + b.Run("runs/"+scope.name, func(b *testing.B) { + id := func(run Run) string { return run.ID } + got := s.ListRuns(scope.threadID) + checkScopedListBenchmarkIDs(b, got, scope.first, scope.count, "run", id) + b.ReportAllocs() + for b.Loop() { + got = s.ListRuns(scope.threadID) + if len(got) != scope.count { + b.Fatalf("run count = %d, want %d", len(got), scope.count) + } + } + checkScopedListBenchmarkIDs(b, got, scope.first, scope.count, "run", id) + b.ReportMetric(float64(len(got)), "results/op") + b.ReportMetric(float64(cap(got)), "result-cap") + }) + b.Run("artifacts/"+scope.name, func(b *testing.B) { + id := func(artifact Artifact) string { return artifact.ID } + got := s.ListArtifacts(scope.runID) + checkScopedListBenchmarkIDs(b, got, scope.first, scope.count, "artifact", id) + b.ReportAllocs() + for b.Loop() { + got = s.ListArtifacts(scope.runID) + if len(got) != scope.count { + b.Fatalf("artifact count = %d, want %d", len(got), scope.count) + } + } + checkScopedListBenchmarkIDs(b, got, scope.first, scope.count, "artifact", id) + b.ReportMetric(float64(len(got)), "results/op") + b.ReportMetric(float64(cap(got)), "result-cap") + }) + } + }) + } +} + +// All content sources are generated metadata; no path is read from disk. The +// Store APIs validate project/thread/run references before the timed workload. +func scopedListBenchmarkStore(b *testing.B, total int) *Store { + b.Helper() + s := New() + if _, err := s.CreateProject("project", "Scoped lists", ""); err != nil { + b.Fatal(err) + } + for _, threadID := range []string{"sparse", "dense"} { + if _, err := s.CreateThread(threadID, "project", "Scoped lists", "", "", ""); err != nil { + b.Fatal(err) + } + } + for i := 0; i < total; i++ { + threadID, artifactRunID := "dense", "run-000010" + if i < 10 { + threadID, artifactRunID = "sparse", "run-000000" + } + if _, err := s.CreateRun(fmt.Sprintf("run-%06d", i), "project", threadID); err != nil { + b.Fatal(err) + } + name := fmt.Sprintf("output-%06d.txt", i) + if _, err := s.UpsertArtifact(Artifact{ + ID: fmt.Sprintf("artifact-%06d", i), RunID: artifactRunID, Kind: "file", Path: name, + ContentSource: &ArtifactContentSource{Kind: ArtifactContentSourceBasename, Path: name, Readable: false}, + }); err != nil { + b.Fatal(err) + } + } + return s +} + +func checkScopedListBenchmarkIDs[T any](b *testing.B, got []T, first, count int, prefix string, id func(T) string) { + b.Helper() + if got == nil || len(got) != count { + b.Fatalf("%s result nil=%v len=%d, want non-nil len=%d", prefix, got == nil, len(got), count) + } + for i, row := range got { + if want := fmt.Sprintf("%s-%06d", prefix, first+i); id(row) != want { + b.Fatalf("%s result[%d] = %q, want %q", prefix, i, id(row), want) + } + } +} From 5ab5fca23881e77031d093ea9bff5a35cc1202ec Mon Sep 17 00:00:00 2001 From: Delicious233 <101502465+DeliciousBuding@users.noreply.github.com> Date: Sun, 6 Sep 2026 16:38:17 +0800 Subject: [PATCH 2/2] perf(store): bound sparse query allocations and clone matched artifacts Refs #2337. Preserve insertion order, single-pass predicates, non-nil empty results and detached artifact metadata. Measure dense/all controls alongside sparse wins. Co-authored-by: Codex --- edge-server/internal/store/store_query.go | 40 +++++-- .../internal/store/store_query_test.go | 102 +++++++++++++++--- 2 files changed, 118 insertions(+), 24 deletions(-) diff --git a/edge-server/internal/store/store_query.go b/edge-server/internal/store/store_query.go index 6b1db682c..a5f34dd5a 100644 --- a/edge-server/internal/store/store_query.go +++ b/edge-server/internal/store/store_query.go @@ -29,23 +29,43 @@ func collectOrdered[T any](order []string, items map[string]T) []T { } func filterOrdered[T any](order []string, items map[string]T, keep func(T) bool) []T { - out := make([]T, 0, len(order)) - for _, id := range order { + // Stage small results on the stack instead of allocating for every resident + // row. Dense results still allocate once, without repeated slice growth or + // calling keep again for rows already visited. + var small [16]T + count := 0 + for index, id := range order { item := items[id] - if keep(item) { - out = append(out, item) + if !keep(item) { + continue + } + if count < len(small) { + small[count] = item + count++ + continue + } + out := make([]T, count, len(order)) + copy(out, small[:]) + out = append(out, item) + for _, remainingID := range order[index+1:] { + remaining := items[remainingID] + if keep(remaining) { + out = append(out, remaining) + } } + return out } + out := make([]T, count) + copy(out, small[:count]) return out } func listClonedArtifacts(order []string, items map[string]Artifact, runID string) []Artifact { - out := make([]Artifact, 0, len(order)) - for _, id := range order { - artifact := cloneArtifact(items[id]) - if scopeEquals(runID, artifact.RunID) { - out = append(out, artifact) - } + out := filterOrdered(order, items, func(artifact Artifact) bool { + return scopeEquals(runID, artifact.RunID) + }) + for i := range out { + out[i] = cloneArtifact(out[i]) } return out } diff --git a/edge-server/internal/store/store_query_test.go b/edge-server/internal/store/store_query_test.go index 796e4752f..14effa319 100644 --- a/edge-server/internal/store/store_query_test.go +++ b/edge-server/internal/store/store_query_test.go @@ -1,6 +1,7 @@ package store import ( + "fmt" "reflect" "testing" ) @@ -56,6 +57,44 @@ func TestCollectAndFilterOrdered(t *testing.T) { } } +func TestFilterOrderedSinglePass(t *testing.T) { + t.Parallel() + if got := filterOrdered[Run](nil, nil, func(Run) bool { + t.Fatal("empty input must not call the predicate") + return false + }); got == nil || len(got) != 0 { + t.Fatalf("empty input = %#v, want non-nil empty result", got) + } + for _, matches := range []int{0, 1, 16, 17, 40} { + t.Run(fmt.Sprintf("matches=%d", matches), func(t *testing.T) { + items := make(map[string]Run) + order := make([]string, 0) + want := make([]Run, 0) + for i := 0; i < 2*max(matches, 20)+1; i++ { + id := fmt.Sprintf("run-%03d", 100-i) + run := Run{ID: id, ThreadID: "other"} + if i%2 == 1 && i < 2*matches { + run.ThreadID = "selected" + want = append(want, run) + } + items[id] = run + order = append(order, id) + } + var visited []string + got := filterOrdered(order, items, func(run Run) bool { + visited = append(visited, run.ID) + return run.ThreadID == "selected" + }) + if !reflect.DeepEqual(visited, order) { + t.Fatalf("predicate visits = %v, want one ordered visit per row: %v", visited, order) + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("filterOrdered = %#v, want %#v", got, want) + } + }) + } +} + func TestSelectCurrentUserProfile(t *testing.T) { t.Parallel() order := []string{"u1", "u2", "u3"} @@ -109,20 +148,55 @@ func TestSortItemsAndPins(t *testing.T) { func TestListClonedArtifacts(t *testing.T) { t.Parallel() - src := &ArtifactContentSource{Kind: ArtifactContentSourceBasename, Path: "a.txt", Readable: false} - items := map[string]Artifact{ - "a1": {ID: "a1", RunID: "r1", ContentSource: src}, - "a2": {ID: "a2", RunID: "r2"}, - } - order := []string{"a1", "a2"} - got := listClonedArtifacts(order, items, "r1") - if len(got) != 1 || got[0].ID != "a1" { - t.Fatalf("listClonedArtifacts = %#v", got) - } - // Mutating returned content source must not affect map. - got[0].ContentSource.Path = "mutated" - if items["a1"].ContentSource.Path != "a.txt" { - t.Fatal("clone did not isolate ContentSource") + items := make(map[string]Artifact) + order := make([]string, 0) + idsByRun := make(map[string][]string) + for i := 0; i < 40; i++ { + id, runID := fmt.Sprintf("artifact-%02d", 40-i), "dense" + if i%4 == 1 { + runID = "small" + } + artifact := Artifact{ID: id, RunID: runID} + if i%3 != 0 { + artifact.ContentSource = &ArtifactContentSource{ + Kind: ArtifactContentSourceBasename, Path: id + ".txt", Readable: true, + } + } + items[id] = artifact + order = append(order, id) + idsByRun[runID] = append(idsByRun[runID], id) + } + for _, scope := range []struct { + name, runID string + want []string + }{ + {"missing", "missing", []string{}}, + {"small", "small", idsByRun["small"]}, + {"dense", "dense", idsByRun["dense"]}, + {"all", "", order}, + } { + t.Run(scope.name, func(t *testing.T) { + got := listClonedArtifacts(order, items, scope.runID) + if got == nil || len(got) != len(scope.want) { + t.Fatalf("listClonedArtifacts = %#v, want non-nil with %d results", got, len(scope.want)) + } + for i, id := range scope.want { + if got[i].ID != id { + t.Fatalf("result[%d].ID = %q, want %q", i, got[i].ID, id) + } + source := items[id].ContentSource + if !reflect.DeepEqual(got[i].ContentSource, source) { + t.Fatalf("result[%d] content source = %#v, want %#v", i, got[i].ContentSource, source) + } + if source != nil { + got[i].ContentSource.Path = "mutated" + got[i].ContentSource.Readable = false + if source.Path != id+".txt" || !source.Readable { + t.Fatalf("result[%d] content source aliases the store", i) + } + } + } + }) } }