-
Notifications
You must be signed in to change notification settings - Fork 0
Add performance testing framework #3
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
Conversation
tests/timing_client.py
Outdated
| def __init__(self, *args, **kwargs): | ||
| super().__init__(*args, **kwargs) | ||
| self._timings: List[RequestTiming] = [] | ||
| self._lock = Lock() |
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.
Is this aliasing to threading.Lock? Should we be using asyncio?
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.
Yes, good call! That was intentional as IMO we'd want asyncio.Lock if we were doing async I/O inside the lock, but here we're just protecting a simple list mutation so everything remains in memory, and this eliminates the need for the extra helper methods reqd by asyncio. That being said, I don't see the need for async operation of the perf test at all, so I just removed the async client entirely since we no longer need it. LMK if you feel differently about me removing that functionality.
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.
Nope I'm not a python expert just checking. I actually decided to modify some of my work based on what I saw here. https://linear.app/you/issue/DX-148/centralize-performance-tests-and-automate-weekly-performancemd-updates
EdwardIrby
left a comment
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.
Did we want to add pref regression checks?
Yes, that's a great idea! Do you have something similar for MCP, if so what thresholds did you land on? CI for this repo will come together when @kevmalek 's schema unification changes are in, and I'd be happy to add a regression check for performance then |
Nope I'm making it part of https://linear.app/you/issue/DX-148/centralize-performance-tests-and-automate-weekly-performancemd-updates |
This PR introduces a comprehensive performance testing infrastructure to measure SDK overhead versus API latency across all endpoints.
Core Infrastructure:
Test Coverage:
Tooling:
Results
Testing against staging with 5 iterations per test:
Example bash command to run perf test (5 iterations of each call) of our search API against staging:
./scripts/run_performance_tests.sh \ -t custom \ -u https://api-staging.you.com \ -k your_api_key_here \ -i 5 \ --searchoutput: