Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 4, 2025

Closes: #767

Integrates schema caching optimization from go-sdk fork to eliminate expensive reflection-based schema generation on every AddTool call. Critical for stateless server patterns where a new server is created per HTTP request.

Benchmark improvements:

  • 132x faster tool registration
  • 51x fewer allocations
  • 19% faster per-tool-call validation

Changes

  • Added replace directive in go.mod pointing to github.com/SamMorrowDrums/go-sdk fork with schema caching
  • Updated go.sum with fork dependencies
  • Updated third-party license files via script/licenses

No code changes required—optimization is transparent to integrators.

Tradeoffs

Using a fork temporarily until the optimization is merged upstream. The fork tracks the perf/phase1-schema-cache branch from SamMorrowDrums/go-sdk#1.

Alternatives

Could wait for upstream merge, but the performance impact on stateless server patterns is significant enough to warrant immediate integration.

Original prompt

This section details on the original issue you should resolve

<issue_title>perf: integrate go-sdk schema caching optimization (Phase 1)</issue_title>
<issue_description>## Summary

Integrate the schema caching optimization from the go-sdk fork to dramatically improve tool registration performance.

Background

The go-sdk currently performs expensive reflection-based schema generation on every AddTool call. In stateless server patterns (like github-mcp-server where a new server is created per HTTP request), this causes significant overhead.

The optimization adds automatic schema caching that provides:

  • 132x faster tool registration
  • 51x fewer allocations
  • 19% faster per-tool-call validation

Implementation

1. Update go.mod to use the fork

Add a replace directive to use the optimized fork:

replace github.com/modelcontextprotocol/go-sdk => github.com/SamMorrowDrums/go-sdk v0.0.0-20251204132239-46310a70a79a

Or use the branch reference:

replace github.com/modelcontextprotocol/go-sdk => github.com/SamMorrowDrums/go-sdk perf/phase1-schema-cache

2. Run go mod tidy

go mod tidy

3. Verify all tests pass

go test ./...

Expected Impact

No code changes should be required in github-mcp-server. The optimization is automatic and transparent to integrators.

Fork Branch

Benchmark Results

BenchmarkAddTool_TypedHandler-22          977080    1223 ns/op     1208 B/op    21 allocs/op
BenchmarkAddTool_TypedHandler_NoCache-22    6764  161463 ns/op    39262 B/op  1072 allocs/op
```</issue_description>

## Comments on the Issue (you are @copilot in this section)

<comments>
</comments>


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Add replace directive to use github.com/SamMorrowDrums/go-sdk fork
with schema caching optimization for improved tool registration performance.

Expected improvements:
- 132x faster tool registration
- 51x fewer allocations
- 19% faster per-tool-call validation

Co-authored-by: SamMorrowDrums <[email protected]>
Copilot AI changed the title [WIP] Integrate go-sdk schema caching optimization for performance perf: integrate go-sdk schema caching optimization (Phase 1) Dec 4, 2025
Copilot AI requested a review from SamMorrowDrums December 4, 2025 13:41
Copilot finished work on behalf of SamMorrowDrums December 4, 2025 13:41
@SamMorrowDrums
Copy link
Collaborator

@copilot Run licenses check

Run script/licenses to update license files after adding the
replace directive for github.com/SamMorrowDrums/go-sdk fork.

Co-authored-by: SamMorrowDrums <[email protected]>
Copy link
Contributor Author

Copilot AI commented Dec 4, 2025

@copilot Run licenses check

Ran script/licenses and the license check now passes. Updated third-party license files committed in bc465b4.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • go.yaml.in
    • Triggering command: /home/REDACTED/go/bin/go-licenses go-licenses report ./... --template .github/licenses.tmpl -c=4 -nolocalimports -importcfg /tmp/go-build2213730874/b348/importcfg -pack /home/REDACTED/go/pkg/mod/k8s.io/klog/[email protected]/internal/buffer/buffer.go x64/pkg/tool/lin-I (dns block)
    • Triggering command: /home/REDACTED/go/bin/go-licenses go-licenses report ./... --template .github/licenses.tmpl -D GOAMD64_v1 -o /tmp/go-build2213730874/b356/debug.o /opt/hostedtoolcache/go/1.24.10/x64/src/runtime/debug/debug.s (dns block)
    • Triggering command: /home/REDACTED/go/bin/go-licenses go-licenses report ./... --template .github/licenses.tmpl -c=4 -nolocalimports -importcfg /tmp/go-build2213730874/b365/importcfg -pack /home/REDACTED/go/pkg/mod/golang.org/x/[email protected]/internal/tag/tag.go x64/pkg/tool/lin-I (dns block)
  • gopkg.in
    • Triggering command: /home/REDACTED/go/bin/go-licenses go-licenses report ./... --template .github/licenses.tmpl -c=4 -nolocalimports -importcfg /tmp/go-build2213730874/b348/importcfg -pack /home/REDACTED/go/pkg/mod/k8s.io/klog/[email protected]/internal/buffer/buffer.go x64/pkg/tool/lin-I (dns block)
    • Triggering command: /home/REDACTED/go/bin/go-licenses go-licenses report ./... --template .github/licenses.tmpl -D GOAMD64_v1 -o /tmp/go-build2213730874/b356/debug.o /opt/hostedtoolcache/go/1.24.10/x64/src/runtime/debug/debug.s (dns block)
    • Triggering command: /home/REDACTED/go/bin/go-licenses go-licenses report ./... --template .github/licenses.tmpl -c=4 -nolocalimports -importcfg /tmp/go-build2213730874/b365/importcfg -pack /home/REDACTED/go/pkg/mod/golang.org/x/[email protected]/internal/tag/tag.go x64/pkg/tool/lin-I (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot finished work on behalf of SamMorrowDrums December 4, 2025 13:50
@SamMorrowDrums
Copy link
Collaborator

Update Required

The upstream go-sdk PR has been cleaned up based on reviewer feedback. The input validation optimization (ApplyDefaults on structs) was removed because it's fundamentally broken per the jsonschema-go maintainer.

What changed in go-sdk PR #685:

  • ❌ Removed: Input validation optimization commit (ApplyDefaults is broken for structs)
  • ✅ Kept: Schema caching (the main performance win)
  • ✅ Fixed: Use sync.Map.Clear() instead of reassignment
  • ✅ Fixed: Removed underscores from benchmark names

New commit to reference:

replace github.com/modelcontextprotocol/go-sdk => github.com/SamMorrowDrums/go-sdk v0.0.0-20251205102140-988974db6bfb

The old commit 46310a70a79a includes the broken input validation optimization. This PR needs to be updated to use the new commit 988974db6bfb.

Performance impact: Still get the 132x faster tool registration from schema caching, just without the per-call validation optimization (which was wrong anyway).

The upstream go-sdk PR was cleaned up based on reviewer feedback:
- Removed broken input validation optimization (ApplyDefaults on structs)
- Fixed sync.Map.Clear() usage
- Fixed benchmark naming

New commit 988974db6bfb contains only the schema caching optimization.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf: integrate go-sdk schema caching optimization (Phase 1) I cannot Start Server

2 participants