Movatterモバイル変換


[0]ホーム

URL:


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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
valgarf wants to merge2 commits intographql-python:main
base:main
Choose a base branch
Loading
fromvalgarf:main

Conversation

valgarf
Copy link

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.0benchmark: 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.cfgplugins: benchmark-3.4.1, cov-3.0.0, asyncio-0.16.0, anyio-3.5.0, timeout-2.1.0, describe-2.0.1timeout: 100.0stimeout method: signaltimeout func_only: Falsecollected 2370 items / 2359 deselected / 11 selectedtests/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           1test_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           1test_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           1test_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           1test_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           1test_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           1test_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           1test_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           1test_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           1test_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           1test_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.0benchmark: 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.cfgplugins: benchmark-3.4.1, cov-3.0.0, anyio-3.5.0, timeout-2.1.0, describe-2.0.1timeout: 100.0stimeout method: signaltimeout func_only: Falsecollected 2516 items / 2504 deselected / 12 selectedtests/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           1test_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           1test_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           1test_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           1test_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           1test_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           1test_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           1test_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           1test_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           1test_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           1test_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           1test_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
@Cito
Copy link
Member

Cito commentedApr 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 freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@CitoCitoAwaiting requested review from CitoCito will be requested when the pull request is marked ready for reviewCito 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
@valgarf@Cito

[8]ページ先頭

©2009-2025 Movatter.jp