Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

perf: integrate go-sdk schema caching optimization (Phase 1)#1533

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
Copilot wants to merge4 commits intomain
base:main
Choose a base branch
Loading
fromcopilot/integrate-go-sdk-schema-caching

Conversation

Copy link
Contributor

CopilotAI commentedDec 4, 2025
edited
Loading

Closes:#767

Integrates schema caching optimization from go-sdk fork to eliminate expensive reflection-based schema generation on everyAddTool 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

  • Addedreplace directive ingo.mod pointing togithub.com/SamMorrowDrums/go-sdk fork with schema caching
  • Updatedgo.sum with fork dependencies
  • Updated third-party license files viascript/licenses

No code changes required—optimization is transparent to integrators.

Tradeoffs

Using a fork temporarily until the optimization is merged upstream. The fork tracks theperf/phase1-schema-cache branch fromSamMorrowDrums/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 everyAddTool 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:

replacegithub.com/modelcontextprotocol/go-sdk=>github.com/SamMorrowDrums/go-sdkv0.0.0-20251204132239-46310a70a79a

Or use the branch reference:

replacegithub.com/modelcontextprotocol/go-sdk=>github.com/SamMorrowDrums/go-sdkperf/phase1-schema-cache

2. Run go mod tidy

go mod tidy

3. Verify all tests pass

gotest ./...

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/opBenchmarkAddTool_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 moreCopilot coding agent tips in the docs.

Add replace directive to use github.com/SamMorrowDrums/go-sdk forkwith schema caching optimization for improved tool registration performance.Expected improvements:- 132x faster tool registration- 51x fewer allocations- 19% faster per-tool-call validationCo-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
CopilotAI changed the title[WIP] Integrate go-sdk schema caching optimization for performanceperf: integrate go-sdk schema caching optimization (Phase 1)Dec 4, 2025
@SamMorrowDrums
Copy link
Collaborator

@copilot Run licenses check

Run script/licenses to update license files after adding thereplace directive for github.com/SamMorrowDrums/go-sdk fork.Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
Copy link
ContributorAuthor

CopilotAI commentedDec 4, 2025
edited
Loading

@copilot Run licenses check

Ranscript/licenses and the license check now passes. Updated third-party license files committed inbc465b4.

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/v2@v2.80.1/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/text@v0.6.0/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/v2@v2.80.1/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/text@v0.6.0/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:

@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: Usesync.Map.Clear() instead of reassignment
  • ✅ Fixed: Removed underscores from benchmark names

New commit to reference:

replacegithub.com/modelcontextprotocol/go-sdk=>github.com/SamMorrowDrums/go-sdkv0.0.0-20251205102140-988974db6bfb

The old commit46310a70a79a includes the broken input validation optimization. This PR needs to be updated to use the new commit988974db6bfb.

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 namingNew commit 988974db6bfb contains only the schema caching optimization.
@kritsada1998mt-create

ghcr.io/github/github-mcp-server

@kritsada1998mt-create

906abab

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@SamMorrowDrumsSamMorrowDrumsAwaiting requested review from SamMorrowDrums

At least 1 approving review is required to merge this pull request.

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

3 participants

@SamMorrowDrums@kritsada1998mt-create

[8]ページ先頭

©2009-2025 Movatter.jp