-
Notifications
You must be signed in to change notification settings - Fork 158
feat(cache): negative caching, goroutine arenas, global key prefix, cache op errors #1435
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
Merged
jensneuse
merged 3 commits into
feat/add-caching-support
from
jensneuse/verify-caching-impl
Mar 6, 2026
Merged
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
108 changes: 108 additions & 0 deletions
108
v2/pkg/engine/resolve/arena_thread_safety_bench_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| package resolve | ||
|
|
||
| import ( | ||
| "sync" | ||
| "testing" | ||
|
|
||
| "github.com/wundergraph/astjson" | ||
| "github.com/wundergraph/go-arena" | ||
| ) | ||
|
|
||
| // cacheLoadAllocs simulates the allocation pattern of tryL2CacheLoad: | ||
| // parse cached JSON bytes, create wrapper objects, allocate slices. | ||
| func cacheLoadAllocs(a arena.Arena) { | ||
| // 1. extractCacheKeysStrings: allocate slice + string bytes | ||
| keys := arena.AllocateSlice[string](a, 0, 4) | ||
| for range 4 { | ||
| buf := arena.AllocateSlice[byte](a, 0, 64) | ||
| buf = arena.SliceAppend(a, buf, []byte("cache:entity:Product:id:prod-1234")...) | ||
| keys = arena.SliceAppend(a, keys, string(buf)) | ||
| } | ||
| _ = keys | ||
|
|
||
| // 2. populateFromCache: parse JSON bytes | ||
| v, _ := astjson.ParseBytesWithArena(a, []byte(`{"__typename":"Product","id":"prod-1234","name":"Test Product","price":29.99}`)) | ||
|
|
||
| // 3. EntityMergePath wrapping: create wrapper objects | ||
| obj := astjson.ObjectValue(a) | ||
| obj.Set(a, "product", v) | ||
| outer := astjson.ObjectValue(a) | ||
| outer.Set(a, "data", obj) | ||
|
|
||
| // 4. denormalizeFromCache: create new object tree | ||
| result := astjson.ObjectValue(a) | ||
| result.Set(a, "productName", v.Get("name")) | ||
| result.Set(a, "productPrice", v.Get("price")) | ||
| } | ||
|
|
||
| // BenchmarkConcurrentArena measures Option A: single arena wrapped with NewConcurrentArena. | ||
| // All goroutines allocate from the same mutex-protected arena. | ||
| func BenchmarkConcurrentArena(b *testing.B) { | ||
| for _, goroutines := range []int{1, 4, 8, 16} { | ||
| b.Run(goroutineName(goroutines), func(b *testing.B) { | ||
| a := arena.NewConcurrentArena(arena.NewMonotonicArena(arena.WithMinBufferSize(64 * 1024))) | ||
| b.ResetTimer() | ||
| for b.Loop() { | ||
| var wg sync.WaitGroup | ||
| for range goroutines { | ||
| wg.Go(func() { | ||
| cacheLoadAllocs(a) | ||
| }) | ||
| } | ||
| wg.Wait() | ||
| a.Reset() | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // BenchmarkPerGoroutineArena measures Option B: each goroutine gets its own arena from sync.Pool. | ||
| // Zero lock contention on allocations. | ||
| func BenchmarkPerGoroutineArena(b *testing.B) { | ||
| pool := sync.Pool{ | ||
| New: func() any { | ||
| return arena.NewMonotonicArena(arena.WithMinBufferSize(4096)) | ||
| }, | ||
| } | ||
|
|
||
| for _, goroutines := range []int{1, 4, 8, 16} { | ||
| b.Run(goroutineName(goroutines), func(b *testing.B) { | ||
| b.ResetTimer() | ||
| for b.Loop() { | ||
| arenas := make([]arena.Arena, goroutines) | ||
| var wg sync.WaitGroup | ||
| for i := range goroutines { | ||
| ga := pool.Get().(arena.Arena) | ||
| arenas[i] = ga | ||
| wg.Go(func() { | ||
| cacheLoadAllocs(ga) | ||
| }) | ||
| } | ||
| wg.Wait() | ||
| for _, ga := range arenas { | ||
| ga.Reset() | ||
| pool.Put(ga) | ||
| } | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func goroutineName(n int) string { | ||
| return "goroutines=" + stringFromInt(n) | ||
| } | ||
|
|
||
| func stringFromInt(n int) string { | ||
| switch n { | ||
| case 1: | ||
| return "1" | ||
| case 4: | ||
| return "4" | ||
| case 8: | ||
| return "8" | ||
| case 16: | ||
| return "16" | ||
| default: | ||
| return "?" | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,178 @@ | ||
| package resolve | ||
|
|
||
| import ( | ||
| "runtime" | ||
| "runtime/debug" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/wundergraph/astjson" | ||
| "github.com/wundergraph/go-arena" | ||
| ) | ||
|
|
||
| // TestCrossArenaMergeValuesCreatesShallowReferences proves that MergeValues | ||
| // links *Value pointers from the source arena into the target arena's tree | ||
| // without deep-copying. Resetting the source arena makes the merged values stale. | ||
| // | ||
| // This is the foundational invariant for AC-THREAD-04: goroutine arenas that | ||
| // hold FromCache values must NOT be released before the response is fully rendered. | ||
| func TestCrossArenaMergeValuesCreatesShallowReferences(t *testing.T) { | ||
| old := debug.SetGCPercent(1) | ||
| defer debug.SetGCPercent(old) | ||
|
|
||
| mainArena := arena.NewMonotonicArena(arena.WithMinBufferSize(4096)) | ||
| goroutineArena := arena.NewMonotonicArena(arena.WithMinBufferSize(4096)) | ||
|
|
||
| // Parse entity data on the "goroutine" arena (simulates populateFromCache) | ||
| fromCache, err := astjson.ParseBytesWithArena(goroutineArena, []byte(`{"id":"prod-1","name":"Widget"}`)) | ||
| require.NoError(t, err) | ||
|
|
||
| // Parse the target item on the main arena (simulates the response tree) | ||
| item, err := astjson.ParseBytesWithArena(mainArena, []byte(`{"id":"prod-1"}`)) | ||
| require.NoError(t, err) | ||
|
|
||
| // Merge: this splices FromCache nodes into item's object tree | ||
| merged, _, err := astjson.MergeValues(mainArena, item, fromCache) | ||
| require.NoError(t, err) | ||
|
|
||
| // Verify merged result contains data from both arenas | ||
| mergedJSON := string(merged.MarshalTo(nil)) | ||
| assert.Contains(t, mergedJSON, `"name":"Widget"`) | ||
| assert.Contains(t, mergedJSON, `"id":"prod-1"`) | ||
|
|
||
| // Force GC to stress-test pointer validity — goroutine arena is still alive | ||
| runtime.GC() | ||
| runtime.GC() | ||
|
|
||
| // Values should still be valid since goroutine arena hasn't been reset | ||
| postGCJSON := string(merged.MarshalTo(nil)) | ||
| assert.Equal(t, mergedJSON, postGCJSON, | ||
| "merged values should survive GC when goroutine arena is still alive") | ||
|
|
||
| // Now reset the goroutine arena — simulates premature release | ||
| goroutineArena.Reset() | ||
|
|
||
| // Overwrite the freed memory with different data | ||
| _, _ = astjson.ParseBytesWithArena(goroutineArena, []byte(`{"id":"STALE","name":"CORRUPTED"}`)) | ||
|
|
||
| // The merged tree still holds pointers into the (now overwritten) goroutine arena. | ||
| // This proves MergeValues is shallow — accessing the stale data may panic or | ||
| // return corrupted values. | ||
| staleOrPanicked := func() (result string, panicked bool) { | ||
| defer func() { | ||
| if r := recover(); r != nil { | ||
| panicked = true | ||
| } | ||
| }() | ||
| return string(merged.MarshalTo(nil)), false | ||
| } | ||
| staleJSON, panicked := staleOrPanicked() | ||
| assert.True(t, panicked || staleJSON != mergedJSON, | ||
| "merged values should be stale or inaccessible after goroutine arena reset — "+ | ||
| "this proves MergeValues creates cross-arena shallow references") | ||
|
|
||
| runtime.KeepAlive(mainArena) | ||
| runtime.KeepAlive(goroutineArena) | ||
| } | ||
|
|
||
| // TestGoroutineArenaLifetimeWithDeferredRelease verifies the correct pattern: | ||
| // goroutine arenas survive through the full resolve lifecycle and are only | ||
| // released in Free(). This matches the Loader.goroutineArenas design. | ||
| func TestGoroutineArenaLifetimeWithDeferredRelease(t *testing.T) { | ||
| old := debug.SetGCPercent(1) | ||
| defer debug.SetGCPercent(old) | ||
|
|
||
| mainArena := arena.NewMonotonicArena(arena.WithMinBufferSize(4096)) | ||
|
|
||
| // Simulate multiple goroutines, each with their own arena | ||
| const numGoroutines = 4 | ||
| goroutineArenas := make([]arena.Arena, numGoroutines) | ||
| fromCacheValues := make([]*astjson.Value, numGoroutines) | ||
|
|
||
| for i := range numGoroutines { | ||
| goroutineArenas[i] = arena.NewMonotonicArena(arena.WithMinBufferSize(4096)) | ||
| var err error | ||
| fromCacheValues[i], err = astjson.ParseBytesWithArena( | ||
| goroutineArenas[i], | ||
| []byte(`{"id":"prod-`+stringFromInt(i+1)+`","name":"Product `+stringFromInt(i+1)+`"}`), | ||
| ) | ||
| require.NoError(t, err) | ||
| } | ||
|
|
||
| // Phase 4: merge all FromCache values into main arena tree | ||
| items := make([]*astjson.Value, numGoroutines) | ||
| for i := range numGoroutines { | ||
| items[i], _ = astjson.ParseBytesWithArena(mainArena, []byte(`{"id":"prod-`+stringFromInt(i+1)+`"}`)) | ||
| merged, _, err := astjson.MergeValues(mainArena, items[i], fromCacheValues[i]) | ||
| require.NoError(t, err) | ||
| items[i] = merged | ||
| } | ||
|
|
||
| // GC pressure — all arenas still alive | ||
| runtime.GC() | ||
| runtime.GC() | ||
|
|
||
| // Verify all merged values are still valid (simulates response rendering) | ||
| for i := range numGoroutines { | ||
| json := string(items[i].MarshalTo(nil)) | ||
| assert.Contains(t, json, `"name":"Product `+stringFromInt(i+1)+`"`, | ||
| "merged value %d should be readable with goroutine arenas alive", i) | ||
| } | ||
|
|
||
| // Now release goroutine arenas (simulates Loader.Free()) | ||
| for _, a := range goroutineArenas { | ||
| a.Reset() | ||
| } | ||
|
|
||
| runtime.KeepAlive(mainArena) | ||
| runtime.KeepAlive(goroutineArenas) | ||
| } | ||
|
|
||
| // Benchmark_CrossArenaGCSafety exercises the goroutine arena pattern under GC | ||
| // pressure. Each iteration creates goroutine arenas, merges values, renders the | ||
| // result, then releases. runtime.GC() between iterations maximizes pressure on | ||
| // any dangling pointers. | ||
| func Benchmark_CrossArenaGCSafety(b *testing.B) { | ||
| old := debug.SetGCPercent(1) | ||
| defer debug.SetGCPercent(old) | ||
|
|
||
| entityJSON := []byte(`{"__typename":"Product","id":"prod-1","name":"Widget","price":9.99}`) | ||
| itemJSON := []byte(`{"__typename":"Product","id":"prod-1"}`) | ||
|
|
||
| b.ResetTimer() | ||
| for b.Loop() { | ||
| mainArena := arena.NewMonotonicArena(arena.WithMinBufferSize(4096)) | ||
| goroutineArena := arena.NewMonotonicArena(arena.WithMinBufferSize(4096)) | ||
|
|
||
| // Simulate goroutine: parse cached entity | ||
| fromCache, err := astjson.ParseBytesWithArena(goroutineArena, entityJSON) | ||
| if err != nil { | ||
| b.Fatal(err) | ||
| } | ||
|
|
||
| // Simulate Phase 4: merge into response tree | ||
| item, err := astjson.ParseBytesWithArena(mainArena, itemJSON) | ||
| if err != nil { | ||
| b.Fatal(err) | ||
| } | ||
| merged, _, err := astjson.MergeValues(mainArena, item, fromCache) | ||
| if err != nil { | ||
| b.Fatal(err) | ||
| } | ||
|
|
||
| // Simulate response rendering | ||
| buf := merged.MarshalTo(nil) | ||
| if len(buf) == 0 { | ||
| b.Fatal("empty output") | ||
| } | ||
|
|
||
| // Release (correct order: goroutine arena after rendering) | ||
| goroutineArena.Reset() | ||
| mainArena.Reset() | ||
|
|
||
| // GC pressure between iterations | ||
| runtime.GC() | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.