-
-
Notifications
You must be signed in to change notification settings - Fork 140
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
Add pytest-benchmark and port the benchmarks from GraphQL.js #55
Conversation
c7c6c26
to
e302c24
Compare
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.
Thanks a lot for this contribution. I always wanted to add the benchmarks - being pushed by others helps a lot. Overall, this looks very good. See below for a few minor suggestions for changes.
[tool:pytest] | ||
# Only run benchmarks as tests. | ||
# To actually run the benchmarks, use --benchmark-enable on the command line. | ||
addopts = --benchmark-disable |
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.
We could also use benchmark-skip to completely skip them, but I agree this is better. They are probably a bit redundant, but not too slow if they run only as test.
I've ported the rest of the benchmarks. Impact on the test suite, as measured with
So even running the benchmarks as tests adds 3-4s to the whole test suite. Main offenders are:
|
Update: there was an optimisation in GraphQL.js test setup that I've missed when porting, here are the current times:
So even running the benchmarks as tests adds 2-3s to the whole test suite. Main offenders are:
|
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.
Great job, thanks. See two small issues below. And maybe the test function names should match with the names used in GraphQL.js (specified as const name =...
.
From src/validation/__tests__/validateGQL-benchmark.js
From src/validation/__tests__/validateSDL-benchmark.js
From src/utilities/__tests__/buildASTSchema-benchmark.js
From src/utilities/__tests__/buildClientSchema-benchmark.js
GraphQL.js does that too, and it shaves ~0.9s off the execution time: 1.13s -> 0.69s for test_introspection_from_schema.py 0.67s -> 0.21s for test_validate_gql.py
I've renamed the test functions. |
Thanks. Don't worry, I can resolve that conflict. Will be very busy the next 2 days and look at this later. |
This is a start for working on #3 - currently only one benchmark is ported from GraphQL.js, but when this code is in an acceptable shape I'll try porting the rest.
Benchmarks can be run with
pytest --benchmark-enable -k benchmark
.By default benchmarks work like simple tests (they only run once), so they shouldn't slow things down too much.