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

[WIP] Proposal: replace asyncio with anyio #165

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

Draft
valgarf wants to merge 2 commits into graphql-python:main
base: main
Choose a base branch
Loading
from valgarf:main

Conversation

Copy link

@valgarf valgarf commented Apr 2, 2022

This PR replaces almost all asyncio usage with anyio, making graphql-core run with asyncio and trio.

This is only a proposal. It is implemented completely and tests run with asyncio and trio, but the PR also includes a few decisions that should probably be discussed before adopting the changes.

Current state:

  • tox runs successfully for python 3.7, 3.8, 3.9, and 3.10 (python 3.6 is currently broken on my machine, cannot run the tests with it)
  • every async test is run in asyncio and in trio
  • performance of the existing async benchmark seems to be unaffected when using the asyncio event loop (see benchmarks below)

The following changes were necessary to use anyio:

  • minium python version had to be increased from 3.6.0 to 3.6.2
  • replacing asyncio functions with their anyio counterpart (e.g. asyncio.sleep -> anyio.sleep)
  • all create_task / ensure_future calls are forbidden in anyio. These code parts were rewritten using anyio task groups.

The following decisions should probably be discussed:

  • exception groups are caught and only the first exception is raised.
  • SimplePubSub / SimplePubSubIterator are unchanged, adapting them to anyio would change their API too much. MemoryObjectBroadcastStream was added to take its place.

ToDo:

  • documentation probably needs to be updated (I did not change anything)
  • run tests & fix issues for python 3.6

I am aware that this is a large change and it comes with (small) downsides,
e.g. a minimum python version of 3.6.2 and some extra effort for the users of SimplePubSub.
But it would allow the use of graphql with the trio event loop.
Having spent countless hours debugging due to some asyncio task that did not close correctly, I would
be extremely happy to have a structured concurrency approach available for graphql.

If you are at all interested in this proposal, please let me know.

Benchmarks before changes:
============================= test session starts ==============================
platform linux -- Python 3.7.13, pytest-6.2.5, py-1.11.0, pluggy-1.0.0
benchmark: 3.4.1 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=5 calibration_precision=10 warmup=True warmup_iterations=100000)
rootdir: /home/stefan/ws/graphql-core, configfile: setup.cfg
plugins: benchmark-3.4.1, cov-3.0.0, asyncio-0.16.0, anyio-3.5.0, timeout-2.1.0, describe-2.0.1
timeout: 100.0s
timeout method: signal
timeout func_only: False
collected 2370 items / 2359 deselected / 11 selected
tests/benchmarks/test_build_ast_schema.py . [ 9%]
tests/benchmarks/test_build_client_schema.py . [ 18%]
tests/benchmarks/test_execution_async.py . [ 27%]
tests/benchmarks/test_execution_sync.py . [ 36%]
tests/benchmarks/test_introspection_from_schema.py . [ 45%]
tests/benchmarks/test_parser.py . [ 54%]
tests/benchmarks/test_validate_gql.py . [ 63%]
tests/benchmarks/test_validate_invalid_gql.py . [ 72%]
tests/benchmarks/test_validate_sdl.py . [ 81%]
tests/benchmarks/test_visit.py .. [100%]
--------------------------------------------------------------------------------------------------------- benchmark: 11 tests ----------------------------------------------------------------------------------------------------------
Name (time in us) Min Max Mean StdDev Median IQR Outliers OPS Rounds Iterations
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_execute_basic_sync 679.1020 (1.0) 24,167.3440 (1.08) 795.1533 (1.0) 615.8840 (1.92) 778.2310 (1.0) 110.2390 (1.54) 157;381 1,257.6191 (1.0) 7358 1
test_execute_basic_async 764.3180 (1.13) 24,417.7180 (1.09) 880.1823 (1.11) 648.0944 (2.02) 832.9540 (1.07) 114.0020 (1.59) 65;265 1,136.1283 (0.90) 6536 1
test_parse_kitchen_sink 1,361.3850 (2.00) 22,441.8060 (1.0) 1,461.0742 (1.84) 677.6062 (2.11) 1,435.4575 (1.84) 71.7050 (1.0) 4;199 684.4280 (0.54) 3680 1
test_validate_introspection_query 4,038.9070 (5.95) 48,622.5970 (2.17) 4,381.5331 (5.51) 1,802.7149 (5.62) 4,194.7020 (5.39) 111.0915 (1.55) 2;122 228.2306 (0.18) 1241 1
test_validate_invalid_query 37,922.6710 (55.84) 40,026.3080 (1.78) 38,436.2805 (48.34) 365.8637 (1.14) 38,341.9050 (49.27) 330.5452 (4.61) 17;9 26.0171 (0.02) 131 1
test_build_schema_from_ast 40,436.8810 (59.54) 91,369.1450 (4.07) 52,498.9107 (66.02) 19,934.7782 (62.14) 41,902.6470 (53.84) 1,569.7452 (21.89) 29;29 19.0480 (0.02) 123 1
test_build_schema_from_introspection 46,932.1740 (69.11) 87,449.3990 (3.90) 56,032.1161 (70.47) 15,406.7749 (48.02) 47,648.6120 (61.23) 1,379.6577 (19.24) 25;25 17.8469 (0.01) 107 1
test_visit_all_ast_nodes 57,287.9200 (84.36) 58,812.3470 (2.62) 57,831.8654 (72.73) 320.8184 (1.0) 57,809.7725 (74.28) 468.2680 (6.53) 29;1 17.2915 (0.01) 88 1
test_validate_sdl_document 116,616.9430 (171.72) 119,051.0130 (5.30) 117,596.4405 (147.89) 634.0356 (1.98) 117,494.7540 (150.98) 847.3555 (11.82) 13;0 8.5037 (0.01) 43 1
test_execute_introspection_query 215,637.0560 (317.53) 254,215.4660 (11.33) 222,730.8371 (280.11) 13,984.2738 (43.59) 216,851.9790 (278.65) 877.4840 (12.24) 4;4 4.4897 (0.00) 24 1
test_visit_all_ast_nodes_in_parallel 611,041.8340 (899.78) 617,905.6140 (27.53) 614,241.2548 (772.48) 2,416.0767 (7.53) 613,784.0130 (788.69) 3,670.1400 (51.18) 3;0 1.6280 (0.00) 9 1
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Legend:
 Outliers: 1 Standard Deviation from Mean; 1.5 IQR (InterQuartile Range) from 1st Quartile and 3rd Quartile.
 OPS: Operations Per Second, computed as 1 / Mean
=============== 11 passed, 2359 deselected in 184.28s (0:03:04) ================
Benchmarks after changes:
============================= test session starts ==============================
platform linux -- Python 3.7.13, pytest-6.2.5, py-1.11.0, pluggy-1.0.0
benchmark: 3.4.1 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=5 calibration_precision=10 warmup=True warmup_iterations=100000)
rootdir: /home/stefan/ws/graphql-core, configfile: setup.cfg
plugins: benchmark-3.4.1, cov-3.0.0, anyio-3.5.0, timeout-2.1.0, describe-2.0.1
timeout: 100.0s
timeout method: signal
timeout func_only: False
collected 2516 items / 2504 deselected / 12 selected
tests/benchmarks/test_build_ast_schema.py . [ 8%]
tests/benchmarks/test_build_client_schema.py . [ 16%]
tests/benchmarks/test_execution_async.py .. [ 33%]
tests/benchmarks/test_execution_sync.py . [ 41%]
tests/benchmarks/test_introspection_from_schema.py . [ 50%]
tests/benchmarks/test_parser.py . [ 58%]
tests/benchmarks/test_validate_gql.py . [ 66%]
tests/benchmarks/test_validate_invalid_gql.py . [ 75%]
tests/benchmarks/test_validate_sdl.py . [ 83%]
tests/benchmarks/test_visit.py .. [100%]
--------------------------------------------------------------------------------------------------------- benchmark: 12 tests ----------------------------------------------------------------------------------------------------------
Name (time in us) Min Max Mean StdDev Median IQR Outliers OPS Rounds Iterations
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_execute_basic_sync 661.9170 (1.0) 25,781.1220 (1.29) 763.7950 (1.0) 647.2897 (2.59) 727.1705 (1.0) 99.9350 (1.60) 10;310 1,309.2518 (1.0) 7574 1
test_execute_basic_async[asyncio] 789.3590 (1.19) 20,032.9120 (1.0) 867.8673 (1.14) 249.4849 (1.0) 906.9380 (1.25) 120.7202 (1.93) 6;8 1,152.2499 (0.88) 6343 1
test_execute_basic_async_trio[trio] 1,251.8120 (1.89) 25,944.9790 (1.30) 1,409.2772 (1.85) 680.1598 (2.73) 1,395.3320 (1.92) 129.8407 (2.08) 3;228 709.5836 (0.54) 4013 1
test_parse_kitchen_sink 1,330.0340 (2.01) 23,528.3300 (1.17) 1,424.0817 (1.86) 723.1457 (2.90) 1,402.3740 (1.93) 62.5005 (1.0) 4;185 702.2069 (0.54) 3772 1
test_validate_introspection_query 3,918.3780 (5.92) 47,985.1300 (2.40) 4,178.4954 (5.47) 1,758.7993 (7.05) 4,011.1160 (5.52) 78.7470 (1.26) 2;129 239.3206 (0.18) 1277 1
test_validate_invalid_query 37,027.1740 (55.94) 38,657.1470 (1.93) 37,451.0217 (49.03) 271.5596 (1.09) 37,385.6900 (51.41) 283.0698 (4.53) 30;8 26.7015 (0.02) 135 1
test_build_schema_from_ast 38,402.0660 (58.02) 87,811.6550 (4.38) 50,244.0027 (65.78) 19,427.7732 (77.87) 39,887.3900 (54.85) 1,784.4250 (28.55) 31;31 19.9029 (0.02) 130 1
test_build_schema_from_introspection 44,885.4960 (67.81) 83,229.9070 (4.15) 53,670.0072 (70.27) 14,769.6549 (59.20) 45,680.3725 (62.82) 1,343.1600 (21.49) 26;26 18.6324 (0.01) 112 1
test_visit_all_ast_nodes 55,416.7260 (83.72) 56,546.7950 (2.82) 55,881.9364 (73.16) 251.1629 (1.01) 55,881.2460 (76.85) 382.7483 (6.12) 31;0 17.8949 (0.01) 91 1
test_validate_sdl_document 112,529.1490 (170.00) 152,931.9640 (7.63) 114,881.9283 (150.41) 5,848.5372 (23.44) 113,915.4470 (156.66) 1,022.5075 (16.36) 1;2 8.7046 (0.01) 45 1
test_execute_introspection_query 210,025.7760 (317.30) 248,757.4280 (12.42) 216,474.8744 (283.42) 12,211.5627 (48.95) 212,063.4750 (291.63) 2,000.2005 (32.00) 3;3 4.6195 (0.00) 24 1
test_visit_all_ast_nodes_in_parallel 610,209.5660 (921.88) 614,215.2950 (30.66) 612,014.3380 (801.28) 1,352.9536 (5.42) 611,614.2970 (841.09) 1,486.0685 (23.78) 3;0 1.6339 (0.00) 9 1
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Legend:
 Outliers: 1 Standard Deviation from Mean; 1.5 IQR (InterQuartile Range) from 1st Quartile and 3rd Quartile.
 OPS: Operations Per Second, computed as 1 / Mean
=============== 12 passed, 2504 deselected in 199.34s (0:03:19) ================

Ambro17 reacted with rocket emoji
Copy link
Member

Cito commented Apr 2, 2022

Wow, @valgarf, that's a huge PR. I think this is something that can be considered for v3.3, maybe together with supporting GraphQL.js 17. The support for Python 3.6 would then be no problem, as I plan to drop it in the next minor release anyway.

But I'm currently not sure about the advantages and implications of this PR, will need some time to get aquainted with trio and anyio, and finding time for this is currently hard for me. I actually prefer to only use standard Python in GraphQL-core - using anyio would introduce an additional dependency that needs to be mangaged and can cause problems, or some day not be supported or well maintained any more. So we need really good reasons to do this. Also, we need to make sure that there are no performance drawbacks or other issues. There are some benchmarks already, but maybe we need to add some more for testing the consequences of these changes.

My suggestion is that you create an accompanying issue referencing this PR, outlining the advantages a bit more, as an invitation for other users to join the discussion - I would also like to get feedback from existing users regarding this change.

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

@Cito Cito Awaiting requested review from Cito Cito will be requested when the pull request is marked ready for review Cito is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants

AltStyle によって変換されたページ (->オリジナル) /