Skip to content

Commit 718046c

Browse files
perf: add inventory cache for stateless server patterns
Add CachedInventory to build tool/resource/prompt definitions once at startup rather than per-request. This is particularly useful for the remote server pattern where a new server instance is created per request. Key features: - InitInventoryCache(t) initializes the cache once at startup - CachedInventoryBuilder() returns a builder with pre-cached definitions - Per-request configuration (read-only, toolsets, feature flags) still works - Thread-safe via sync.Once - Backward compatible: NewInventory(t) still works without caching This addresses the performance concern raised in go-sdk PR #685 at a higher level by caching the entire []ServerTool slice rather than individual schemas. Related: modelcontextprotocol/go-sdk#685
1 parent 97feb5c commit 718046c

File tree

2 files changed

+264
-0
lines changed

2 files changed

+264
-0
lines changed

pkg/github/inventory_cache.go

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
package github
2+
3+
import (
4+
"sync"
5+
6+
"github.com/github/github-mcp-server/pkg/inventory"
7+
"github.com/github/github-mcp-server/pkg/translations"
8+
)
9+
10+
// CachedInventory provides a cached inventory builder that builds tool definitions
11+
// only once, regardless of how many times NewInventoryBuilder is called.
12+
//
13+
// This is particularly useful for stateless server patterns (like the remote server)
14+
// where a new server instance is created per request. Without caching, every request
15+
// would rebuild all ~130 tool definitions including JSON schema generation, causing
16+
// significant performance overhead.
17+
//
18+
// Usage:
19+
//
20+
// // Option 1: Initialize once at startup with your translator
21+
// github.InitInventoryCache(myTranslator)
22+
//
23+
// // Then get pre-built inventory on each request
24+
// inv := github.CachedInventoryBuilder().
25+
// WithReadOnly(cfg.ReadOnly).
26+
// WithToolsets(cfg.Toolsets).
27+
// Build()
28+
//
29+
// // Option 2: Use NewInventory which doesn't use the cache (legacy behavior)
30+
// inv := github.NewInventory(myTranslator).Build()
31+
//
32+
// The cache stores the built []ServerTool, []ServerResourceTemplate, and []ServerPrompt.
33+
// Per-request configuration (read-only, toolsets, feature flags, filters) is still
34+
// applied when building the Inventory from the cached data.
35+
type CachedInventory struct {
36+
once sync.Once
37+
tools []inventory.ServerTool
38+
resources []inventory.ServerResourceTemplate
39+
prompts []inventory.ServerPrompt
40+
}
41+
42+
// global singleton for caching
43+
var globalInventoryCache = &CachedInventory{}
44+
45+
// InitInventoryCache initializes the global inventory cache with the given translator.
46+
// This should be called once at startup before any requests are processed.
47+
// It's safe to call multiple times - only the first call has any effect.
48+
//
49+
// For the local server, this is typically called with the configured translator.
50+
// For the remote server, use translations.NullTranslationHelper since translations
51+
// aren't needed per-request.
52+
//
53+
// Example:
54+
//
55+
// func main() {
56+
// t, _ := translations.TranslationHelper()
57+
// github.InitInventoryCache(t)
58+
// // ... start server
59+
// }
60+
func InitInventoryCache(t translations.TranslationHelperFunc) {
61+
globalInventoryCache.init(t)
62+
}
63+
64+
// init initializes the cache with the given translator (sync.Once protected).
65+
func (c *CachedInventory) init(t translations.TranslationHelperFunc) {
66+
c.once.Do(func() {
67+
c.tools = AllTools(t)
68+
c.resources = AllResources(t)
69+
c.prompts = AllPrompts(t)
70+
})
71+
}
72+
73+
// CachedInventoryBuilder returns an inventory.Builder pre-populated with cached
74+
// tool/resource/prompt definitions.
75+
//
76+
// The cache must be initialized via InitInventoryCache before calling this function.
77+
// If the cache is not initialized, this will initialize it with NullTranslationHelper.
78+
//
79+
// Per-request configuration can still be applied via the builder methods:
80+
// - WithReadOnly(bool) - filter to read-only tools
81+
// - WithToolsets([]string) - enable specific toolsets
82+
// - WithTools([]string) - enable specific tools
83+
// - WithFeatureChecker(func) - per-request feature flag evaluation
84+
// - WithFilter(func) - custom filtering
85+
//
86+
// Example:
87+
//
88+
// inv := github.CachedInventoryBuilder().
89+
// WithReadOnly(cfg.ReadOnly).
90+
// WithToolsets(cfg.EnabledToolsets).
91+
// WithFeatureChecker(createFeatureChecker(cfg.EnabledFeatures)).
92+
// Build()
93+
func CachedInventoryBuilder() *inventory.Builder {
94+
// Ensure cache is initialized (with NullTranslationHelper as fallback)
95+
globalInventoryCache.init(translations.NullTranslationHelper)
96+
97+
return inventory.NewBuilder().
98+
SetTools(globalInventoryCache.tools).
99+
SetResources(globalInventoryCache.resources).
100+
SetPrompts(globalInventoryCache.prompts)
101+
}
102+
103+
// IsCacheInitialized returns true if the inventory cache has been initialized.
104+
// This is primarily useful for testing.
105+
func IsCacheInitialized() bool {
106+
// We can't directly check sync.Once state, but we can check if tools are populated
107+
return len(globalInventoryCache.tools) > 0
108+
}
109+
110+
// ResetInventoryCache resets the global inventory cache, allowing it to be
111+
// reinitialized with a different translator. This should only be used in tests.
112+
//
113+
// WARNING: This is not thread-safe and should never be called in production code.
114+
func ResetInventoryCache() {
115+
globalInventoryCache = &CachedInventory{}
116+
}

pkg/github/inventory_cache_test.go

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
package github
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/github/github-mcp-server/pkg/translations"
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
func TestCachedInventory(t *testing.T) {
13+
// Reset cache before test
14+
ResetInventoryCache()
15+
16+
t.Run("InitInventoryCache initializes tools", func(t *testing.T) {
17+
ResetInventoryCache()
18+
19+
// Cache should be empty before init
20+
assert.False(t, IsCacheInitialized())
21+
22+
// Initialize with null translator
23+
InitInventoryCache(translations.NullTranslationHelper)
24+
25+
// Cache should now be initialized
26+
assert.True(t, IsCacheInitialized())
27+
})
28+
29+
t.Run("CachedInventoryBuilder returns builder with cached tools", func(t *testing.T) {
30+
ResetInventoryCache()
31+
InitInventoryCache(translations.NullTranslationHelper)
32+
33+
builder := CachedInventoryBuilder()
34+
require.NotNil(t, builder)
35+
36+
inv := builder.Build()
37+
require.NotNil(t, inv)
38+
39+
// Should have tools available
40+
tools := inv.AllTools()
41+
assert.Greater(t, len(tools), 0)
42+
})
43+
44+
t.Run("CachedInventoryBuilder auto-initializes if not initialized", func(t *testing.T) {
45+
ResetInventoryCache()
46+
47+
// Don't call InitInventoryCache
48+
49+
// CachedInventoryBuilder should still work (auto-init with NullTranslationHelper)
50+
builder := CachedInventoryBuilder()
51+
require.NotNil(t, builder)
52+
53+
inv := builder.Build()
54+
require.NotNil(t, inv)
55+
56+
tools := inv.AllTools()
57+
assert.Greater(t, len(tools), 0)
58+
})
59+
60+
t.Run("InitInventoryCache is idempotent", func(t *testing.T) {
61+
ResetInventoryCache()
62+
63+
// First init with a custom translator that tracks calls
64+
callCount := 0
65+
customTranslator := func(_, defaultValue string) string {
66+
callCount++
67+
return defaultValue
68+
}
69+
70+
InitInventoryCache(customTranslator)
71+
firstCallCount := callCount
72+
73+
// Second init should be no-op
74+
callCount = 0
75+
InitInventoryCache(translations.NullTranslationHelper)
76+
77+
assert.Equal(t, 0, callCount, "second InitInventoryCache should not call translator")
78+
79+
// Verify first translator was used (tools are still populated from first call)
80+
assert.True(t, IsCacheInitialized())
81+
assert.Greater(t, firstCallCount, 0, "first init should have called translator")
82+
})
83+
84+
t.Run("cached inventory preserves per-request configuration", func(t *testing.T) {
85+
ResetInventoryCache()
86+
InitInventoryCache(translations.NullTranslationHelper)
87+
88+
// Build with read-only filter
89+
invReadOnly := CachedInventoryBuilder().
90+
WithReadOnly(true).
91+
WithToolsets([]string{"all"}).
92+
Build()
93+
94+
// Build without read-only filter
95+
invAll := CachedInventoryBuilder().
96+
WithReadOnly(false).
97+
WithToolsets([]string{"all"}).
98+
Build()
99+
100+
// AvailableTools applies filters - all tools should have more than read-only
101+
ctx := context.Background()
102+
allTools := invAll.AvailableTools(ctx)
103+
readOnlyTools := invReadOnly.AvailableTools(ctx)
104+
105+
assert.Greater(t, len(allTools), len(readOnlyTools),
106+
"read-only inventory should have fewer tools")
107+
})
108+
109+
t.Run("cached inventory supports toolset filtering", func(t *testing.T) {
110+
ResetInventoryCache()
111+
InitInventoryCache(translations.NullTranslationHelper)
112+
113+
// Build with only one toolset
114+
invOneToolset := CachedInventoryBuilder().
115+
WithToolsets([]string{"context"}).
116+
Build()
117+
118+
// Build with all toolsets
119+
invAll := CachedInventoryBuilder().
120+
WithToolsets([]string{"all"}).
121+
Build()
122+
123+
ctx := context.Background()
124+
oneToolsetTools := invOneToolset.AvailableTools(ctx)
125+
allTools := invAll.AvailableTools(ctx)
126+
127+
assert.Greater(t, len(allTools), len(oneToolsetTools),
128+
"all toolsets should have more tools than single toolset")
129+
})
130+
}
131+
132+
func TestNewInventoryVsCachedInventoryBuilder(t *testing.T) {
133+
ResetInventoryCache()
134+
135+
// NewInventory creates fresh tools each time
136+
inv1 := NewInventory(translations.NullTranslationHelper).Build()
137+
inv2 := NewInventory(translations.NullTranslationHelper).Build()
138+
139+
// Both should have the same number of tools
140+
assert.Equal(t, len(inv1.AllTools()), len(inv2.AllTools()))
141+
142+
// CachedInventoryBuilder uses cached tools
143+
InitInventoryCache(translations.NullTranslationHelper)
144+
cachedInv := CachedInventoryBuilder().Build()
145+
146+
// Should also have the same number of tools
147+
assert.Equal(t, len(inv1.AllTools()), len(cachedInv.AllTools()))
148+
}

0 commit comments

Comments
 (0)