-
-
Notifications
You must be signed in to change notification settings - Fork 609
Benchmark graphql-core PR #249 (msgspec performance) #4086
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
base: main
Are you sure you want to change the base?
Conversation
Test graphql-core PR #249 which introduces significant performance improvements using msgspec.Struct for AST nodes. The PR claims: - 5.3x faster parsing - 13x faster serialization with msgpack - 4.7x faster deserialization See: graphql-python/graphql-core#249
|
Hi, thanks for contributing to Strawberry 🍓! We noticed that this PR is missing a So as soon as this PR is merged, a release will be made 🚀. Here's an example of Release type: patch
Description of the changes, ideally with some examples, if adding a new feature.Release type can be one of patch, minor or major. We use semver, so make sure to pick the appropriate type. If in doubt feel free to ask :) Here's the tweet text: |
Greptile SummaryThis PR temporarily switches the Key Changes:
Important Notes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant PR as Pull Request
participant CI as GitHub Actions
participant Poetry as Poetry Install
participant GC as graphql-core (msgspec branch)
participant Tests as Benchmark Tests
participant CS as CodSpeed
PR->>CI: Push triggers workflow
CI->>Poetry: Install dependencies
Poetry->>GC: Install from git+https://github.com/corydolphin/graphql-core.git@msgspec-performance
GC-->>Poetry: msgspec-based AST nodes
Poetry-->>CI: Dependencies ready
CI->>Tests: Run pytest tests/benchmarks --codspeed
Tests->>GC: Parse GraphQL queries (5.3x faster)
Tests->>GC: Serialize/deserialize AST (13x/4.7x faster)
Tests-->>CS: Performance metrics
CS-->>CI: Benchmark results vs main branch
CI-->>PR: CodSpeed report with performance comparison
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 file reviewed, 1 comment
| requires-python = ">=3.10,<4.0" | ||
| dependencies = [ | ||
| "graphql-core>=3.2.0,<3.4.0", | ||
| "graphql-core @ git+https://github.com/corydolphin/graphql-core.git@msgspec-performance", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Ensure this git dependency is reverted before any production release - it's intended only for temporary benchmarking of the msgspec performance improvements.
Prompt To Fix With AI
This is a comment left during a code review.
Path: pyproject.toml
Line: 18:18
Comment:
**style:** Ensure this git dependency is reverted before any production release - it's intended only for temporary benchmarking of the msgspec performance improvements.
How can I resolve this? If you propose a fix, please make it concise.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4086 +/- ##
=======================================
Coverage 94.22% 94.23%
=======================================
Files 540 540
Lines 35499 35499
Branches 1876 1876
=======================================
+ Hits 33449 33451 +2
+ Misses 1739 1737 -2
Partials 311 311 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #4086 will not alter performanceComparing Summary
|
Summary
Test graphql-core PR graphql-python/graphql-core#249 which introduces significant performance improvements using
msgspec.Structfor AST nodes.The PR claims:
This PR temporarily uses the
msgspec-performancebranch from that PR to run our benchmarks and see the actual impact on Strawberry.Test plan