Skip to content

Resolve data race issues with concurrent field evaluation & added benchmark tests #22

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

Open
wants to merge 5 commits into
base: sogko/experiment-parallel-resolve
Choose a base branch
from

Conversation

sogko
Copy link
Owner

@sogko sogko commented Jun 11, 2016

Follow up to PR #20 and #21

Resolve data race issues and added initial benchmark tests

  • Protect shared fields across routines for Object, Enum and Schema
  • Discovered through go test ./... -race
  • Standardise mutex field names *Mutex
  • Some optimisation in lazy-eval functions in Enum. getValueLookup() and Schema.IsPossibleType()

/cc @andreas @chris-ramon @Mleonard87

sogko added 3 commits June 11, 2016 23:19
- Protect shared fields across routines for `Object`, `Enum` and `Schema`
- Discovered through `go test ./... -race`
- Standardise mutex field names *Mutex
… from here

for perf-related PRs.
- simple hello world equivalent
- with a list
- deeper query with list
- query with many root fields and list in each of them
- deeper query with many root fields and lists

To really bring out probable issues with query perf, a latency is
introduced to the data retrieval (previously it was just in-memory data fetch)

When testing against branch before PR #20 and #21, this really highlighted the problem with evaluating list fields.

In the future, more benchmarks for queries with fragments probably would be useful.
@sogko sogko changed the title Resolve data race issues with concurrent field evaluation Resolve data race issues with concurrent field evaluation & added benchmark tests Jun 11, 2016
@sogko
Copy link
Owner Author

sogko commented Jun 11, 2016

/cc @andreas @chris-ramon @Mleonard87

Initial benchmark tests for basic queries; something we can work upon from here for perf-related PRs.

Current benchmark suite:

  • simple hello world equivalent
  • with a list
  • deeper query with list
  • query with many root fields and list in each of them
  • deeper query with many root fields and lists

Added simulate latency for data retrieval
See: 8a5ddd4#diff-d85486dab010c333b6e0880a348f537cR327
To really bring out probable issues with query perf, a latency is introduced to the data retrieval (previously it was just in-memory data fetch).

When testing against branch before PR #20 and #21, this really highlighted the problem with evaluating list fields.

(Using time.Sleep probably is not the best idea, anybody has a better approach?)

Other possible queries to benchmark
In the future, more benchmarks for queries with fragments probably would be useful.


Preliminary benchmark results notes:

resolve-data-race #22

hafiz@Hafizs-MacBook-Pro-Retina ~/d/g/s/g/g/graphql> go test -bench=.
PASS
BenchmarkQuery_BasicQuery-8                                500       3043239 ns/op
BenchmarkQuery_BasicQueryWithList-8                        300       5066747 ns/op
BenchmarkQuery_DeeperQueryWithList-8                       200       6845800 ns/op
BenchmarkQuery_QueryWithManyRootFieldsAndList-8            200       8260212 ns/op
BenchmarkQuery_DeeperQueryWithManyRootFieldsAndList-8      100      16675410 ns/op
ok      github.com/graphql-go/graphql   10.511s
hafiz@Hafizs-MacBook-Pro-Retina ~/d/g/s/g/g/graphql> go test -bench=.
PASS
BenchmarkQuery_BasicQuery-8                                500       3048275 ns/op
BenchmarkQuery_BasicQueryWithList-8                        300       5054268 ns/op
BenchmarkQuery_DeeperQueryWithList-8                       200       6953351 ns/op
BenchmarkQuery_QueryWithManyRootFieldsAndList-8            200       8095983 ns/op
BenchmarkQuery_DeeperQueryWithManyRootFieldsAndList-8      100      15922608 ns/op
ok      github.com/graphql-go/graphql   10.408s
hafiz@Hafizs-MacBook-Pro-Retina ~/d/g/s/g/g/graphql> go test -bench=.
PASS
BenchmarkQuery_BasicQuery-8                                500       3047346 ns/op
BenchmarkQuery_BasicQueryWithList-8                        300       5040302 ns/op
BenchmarkQuery_DeeperQueryWithList-8                       200       7040065 ns/op
BenchmarkQuery_QueryWithManyRootFieldsAndList-8            200       8283751 ns/op
BenchmarkQuery_DeeperQueryWithManyRootFieldsAndList-8      100      16155489 ns/op
ok      github.com/graphql-go/graphql   10.487s
hafiz@Hafizs-MacBook-Pro-Retina ~/d/g/s/g/g/graphql>

experiment-parallel, after #20/#21

hafiz@Hafizs-MacBook-Pro-Retina ~/d/g/s/g/g/graphql> go test -bench=.
PASS
BenchmarkQuery_BasicQuery-8                                500       3117531 ns/op
BenchmarkQuery_BasicQueryWithList-8                        300       5178783 ns/op
BenchmarkQuery_DeeperQueryWithList-8                       200       7543386 ns/op
BenchmarkQuery_QueryWithManyRootFieldsAndList-8            200       9689788 ns/op
BenchmarkQuery_DeeperQueryWithManyRootFieldsAndList-8      100      17462618 ns/op
ok      github.com/graphql-go/graphql   11.323s
hafiz@Hafizs-MacBook-Pro-Retina ~/d/g/s/g/g/graphql> go test -bench=.
PASS
BenchmarkQuery_BasicQuery-8                                500       3138994 ns/op
BenchmarkQuery_BasicQueryWithList-8                        300       5218583 ns/op
BenchmarkQuery_DeeperQueryWithList-8                       200       7670216 ns/op
BenchmarkQuery_QueryWithManyRootFieldsAndList-8            200       9498019 ns/op
BenchmarkQuery_DeeperQueryWithManyRootFieldsAndList-8      100      17297740 ns/op
ok      github.com/graphql-go/graphql   11.321s
hafiz@Hafizs-MacBook-Pro-Retina ~/d/g/s/g/g/graphql> go test -bench=.
PASS
BenchmarkQuery_BasicQuery-8                                500       3105576 ns/op
BenchmarkQuery_BasicQueryWithList-8                        300       5113446 ns/op
BenchmarkQuery_DeeperQueryWithList-8                       200       7567678 ns/op
BenchmarkQuery_QueryWithManyRootFieldsAndList-8            200       9460491 ns/op
BenchmarkQuery_DeeperQueryWithManyRootFieldsAndList-8      100      17654996 ns/op
ok      github.com/graphql-go/graphql   11.260s
hafiz@Hafizs-MacBook-Pro-Retina ~/d/g/s/g/g/graphql>

experiment-parallel, before #20/#21

hafiz@Hafizs-MacBook-Pro-Retina ~/d/g/s/g/g/graphql> go test -bench=.
PASS
BenchmarkQuery_BasicQuery-8                                500       3033379 ns/op
BenchmarkQuery_BasicQueryWithList-8                        200       7822866 ns/op
BenchmarkQuery_DeeperQueryWithList-8                        50      23591160 ns/op
BenchmarkQuery_QueryWithManyRootFieldsAndList-8            200       9611514 ns/op
BenchmarkQuery_DeeperQueryWithManyRootFieldsAndList-8      100      17038372 ns/op
ok      github.com/graphql-go/graphql   10.462s
hafiz@Hafizs-MacBook-Pro-Retina ~/d/g/s/g/g/graphql> go test -bench=.
PASS
BenchmarkQuery_BasicQuery-8                                500       3098141 ns/op
BenchmarkQuery_BasicQueryWithList-8                        200       7832854 ns/op
BenchmarkQuery_DeeperQueryWithList-8                        50      23174919 ns/op
BenchmarkQuery_QueryWithManyRootFieldsAndList-8            200       9431318 ns/op
BenchmarkQuery_DeeperQueryWithManyRootFieldsAndList-8      100      17079757 ns/op
ok      github.com/graphql-go/graphql   10.428s
hafiz@Hafizs-MacBook-Pro-Retina ~/d/g/s/g/g/graphql> go test -bench=.
PASS
BenchmarkQuery_BasicQuery-8                                500       3155781 ns/op
BenchmarkQuery_BasicQueryWithList-8                        200       7891615 ns/op
BenchmarkQuery_DeeperQueryWithList-8                        50      23166824 ns/op
BenchmarkQuery_QueryWithManyRootFieldsAndList-8            200       9624308 ns/op
BenchmarkQuery_DeeperQueryWithManyRootFieldsAndList-8      100      17587311 ns/op
ok      github.com/graphql-go/graphql   10.569s
hafiz@Hafizs-MacBook-Pro-Retina ~/d/g/s/g/g/graphql>

0.5.0 (before experiment-parallel)

hafiz@Hafizs-MacBook-Pro-Retina ~/d/g/s/g/g/graphql> go test -bench=.
PASS
BenchmarkQuery_BasicQuery-8                                500       3029577 ns/op
BenchmarkQuery_BasicQueryWithList-8                        200       7653557 ns/op
BenchmarkQuery_DeeperQueryWithList-8                       100      22426616 ns/op
BenchmarkQuery_QueryWithManyRootFieldsAndList-8            200       9564844 ns/op
BenchmarkQuery_DeeperQueryWithManyRootFieldsAndList-8      100      17378340 ns/op
ok      github.com/graphql-go/graphql   11.488s
hafiz@Hafizs-MacBook-Pro-Retina ~/d/g/s/g/g/graphql> go test -bench=.
PASS
BenchmarkQuery_BasicQuery-8                                500       2907495 ns/op
BenchmarkQuery_BasicQueryWithList-8                        200       7597287 ns/op
BenchmarkQuery_DeeperQueryWithList-8                       100      22343897 ns/op
BenchmarkQuery_QueryWithManyRootFieldsAndList-8            200       9477094 ns/op
BenchmarkQuery_DeeperQueryWithManyRootFieldsAndList-8      100      17438457 ns/op
ok      github.com/graphql-go/graphql   11.372s
hafiz@Hafizs-MacBook-Pro-Retina ~/d/g/s/g/g/graphql> go test -bench=.
PASS
BenchmarkQuery_BasicQuery-8                                500       3008776 ns/op
BenchmarkQuery_BasicQueryWithList-8                        200       7709018 ns/op
BenchmarkQuery_DeeperQueryWithList-8                       100      22476461 ns/op
BenchmarkQuery_QueryWithManyRootFieldsAndList-8            200       9392141 ns/op
BenchmarkQuery_DeeperQueryWithManyRootFieldsAndList-8      100      16938611 ns/op
ok      github.com/graphql-go/graphql   11.380s
hafiz@Hafizs-MacBook-Pro-Retina ~/d/g/s/g/g/graphql>

@andreas
Copy link

andreas commented Jun 11, 2016

Great work and interesting results! I'll try dive into the changes when time permits.

@sogko
Copy link
Owner Author

sogko commented Jun 11, 2016

If you guys were underwhelmed by the benchmark results (I kinda was lol), a preliminary benchmark after including changeset from graphql-go#137 into this PR, yielded promising results.

(That PR by @Matthias247 optimised the lexer, which is part of the query parser, before being passed to the executor).

resolve-data-race #22 + graphql-go#137

hafiz@Hafizs-MacBook-Pro-Retina ~/d/g/s/g/g/graphql> go test -bench=.
PASS
BenchmarkQuery_BasicQuery-8                                  500       2944621 ns/op
BenchmarkQuery_BasicQueryWithList-8                          300       4915931 ns/op
BenchmarkQuery_DeeperQueryWithList-8                         200       6974720 ns/op
BenchmarkQuery_QueryWithManyRootFieldsAndList-8              500       2649506 ns/op
BenchmarkQuery_DeeperQueryWithManyRootFieldsAndList-8        300       4372180 ns/op
ok      github.com/graphql-go/graphql   9.354s
hafiz@Hafizs-MacBook-Pro-Retina ~/d/g/s/g/g/graphql> go test -bench=.
PASS
BenchmarkQuery_BasicQuery-8                                  500       2977187 ns/op
BenchmarkQuery_BasicQueryWithList-8                          300       4810995 ns/op
BenchmarkQuery_DeeperQueryWithList-8                         200       6876937 ns/op
BenchmarkQuery_QueryWithManyRootFieldsAndList-8              500       2652618 ns/op
BenchmarkQuery_DeeperQueryWithManyRootFieldsAndList-8        300       4320122 ns/op
ok      github.com/graphql-go/graphql   9.284s
hafiz@Hafizs-MacBook-Pro-Retina ~/d/g/s/g/g/graphql> go test -bench=.
PASS
BenchmarkQuery_BasicQuery-8                                  500       2959330 ns/op
BenchmarkQuery_BasicQueryWithList-8                          300       4808262 ns/op
BenchmarkQuery_DeeperQueryWithList-8                         200       6762695 ns/op
BenchmarkQuery_QueryWithManyRootFieldsAndList-8              500       2570735 ns/op
BenchmarkQuery_DeeperQueryWithManyRootFieldsAndList-8        300       4335454 ns/op
ok      github.com/graphql-go/graphql   9.164s
hafiz@Hafizs-MacBook-Pro-Retina ~/d/g/s/g/g/graphql>

Compared to resolve-data-race #22 above, BenchmarkQuery_QueryWithManyRootFieldsAndList and BenchmarkQuery_DeeperQueryWithManyRootFieldsAndList were significantly improved.

Edit: Google docs table of above results

Cheers!

/cc @Matthias247

sogko added 2 commits June 12, 2016 06:10
Fix further typo.

Coding way early in the morning after a late night out is not a good idea - Tim Burton
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.

2 participants