asyncio: Optimize get_event_loop and _get_running_loop

[jimmylai]

BPO	31350
Nosy	@vstinner, @1st1, @Mariatta, @jimmylai
PRs	bpo-31350: Optimize get_event_loop and _get_running_loop #3347 [3.6] bpo-31350: Optimize get_event_loop and _get_running_loop (GH-3347) #3373
Files	bench_asyncio.py bench_get_event_loop.py

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2017-09-06.03:06:58.314>
created_at = <Date 2017-09-05.17:19:35.454>
labels = ['expert-asyncio', '3.7', 'performance']
title = 'asyncio: Optimize get_event_loop and _get_running_loop'
updated_at = <Date 2017-09-06.03:06:58.313>
user = 'https://github.com/jimmylai'

bugs.python.org fields:

activity = <Date 2017-09-06.03:06:58.313>
actor = 'Mariatta'
assignee = 'none'
closed = True
closed_date = <Date 2017-09-06.03:06:58.314>
closer = 'Mariatta'
components = ['asyncio']
creation = <Date 2017-09-05.17:19:35.454>
creator = 'jimmylai'
dependencies = []
files = ['47120', '47122']
hgrepos = []
issue_num = 31350
keywords = []
message_count = 11.0
messages = ['301342', '301343', '301344', '301348', '301356', '301357', '301358', '301359', '301416', '301433', '301434']
nosy_count = 4.0
nosy_names = ['vstinner', 'yselivanov', 'Mariatta', 'jimmylai']
pr_nums = ['3347', '3373']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue31350'
versions = ['Python 3.6', 'Python 3.7']

[jimmylai]

get_event_loop() and _get_running_loop() can be faster.

Case Time Mean Improve
No Change 7.323 +- 0.172 7.323 0.00%
Remove class _RunningLoop 6.513 +- 0.115 6.513 -11.06%
Expand _get_running_loop() inside get_event_loop() 5.851 +- 0.160 5.851 -20.10%
Use Tuple instead of two attributes 6.179 +- 0.099 6.179 -15.62%
Tuple + Remove _RunningLoop 6.026 +- 0.123 6.026 -17.71%
Tuple + return ternary + Remove _RunningLoop 6.060 +- 0.111 6.06 -17.25%
Combine all four optimizations 4.735 +- 0.111 4.735 -35.34%
Remove class _RunningLoop + Use Tuple instead of two attributes 6.241 +- 0.097 6.241 -14.78%

Experimenting with different techniques to optimize get_event_loop and _get_running_loop.

After discuss with Yuri, decide not to expand _get_running_loop inside get_event_loop.
Combine tuple in _running_loop and Remove _RunningLoop (just use threading.local) can achieve the best improvement: 17.71% faster.

[vstinner]

Can you please provide the code of your benchmark?

[jimmylai]

Benchmark script: Run 10 times to get mean and stdev

import asyncio
import time

async def async_get_loop():
    start_time = time.time()
    for _ in range(5000000):
        asyncio.get_event_loop()
    return time.time() - start_time

loop = asyncio.get_event_loop()
results = []
for _ in range(10):
    start_time = time.time()
    result = loop.run_until_complete(async_get_loop())
    results.append(result)

import statistics
print("elapse time: %.3lf +- %.3lf secs" % (statistics.mean(results), statistics.stdev(results)))

[vstinner]

I suggest to use my perf module to run benchmark, especially if the tested function takes less than 1 ms, which is the case here.

Attached benchmark script calls asyncio.get_event_loop(). Result on the master branch with PR 3347:

haypo@selma$ ./python ~/bench_asyncio.py --inherit=PYTHONPATH -o patch.json
haypo@selma$ ./python ~/bench_asyncio.py --inherit=PYTHONPATH -o ref.json
haypo@selma$ ./python -m perf compare_to ref.json patch.json

Mean +- std dev: [ref] 881 ns +- 42 ns -> [patch] 859 ns +- 14 ns: 1.03x faster (-3%)

I'm not convinced that the PR is worth it. 3% is not interesting on a micro benchmark.

Or is there an issue in my benchmark?

[1st1]

I'm not convinced that the PR is worth it. 3% is not interesting on a micro benchmark.

I found a small issue in the PR (left a comment in the PR).

I think using a tuple is still a good idea (even if the speedup is tiny) because logically, both attributes on that threading.local() object are always set and read at the same time. Essentially, it's a pair of (loop, pid), so using a tuple here makes the code easier to reason about.

[vstinner]

If the motivation is correctness and not performance, please adjust the issue and PR description :-)

[1st1]

Or is there an issue in my benchmark?

Yes. The correct benchmark would be to measure get_event_loop performance from within a running event loop.

[vstinner]

According to Jimmy, asyncio.get_event_loop() behaves differently if it's called while an event loop is running. So my first benchmark was wrong.

Attached bench_get_event_loop.py measures asyncio.get_event_loop() performance when an event loop is running. I get a different result:

haypo@selma$ ./python -m perf compare_to ref.json patch.json
Mean +- std dev: [ref] 555 ns +- 11 ns -> [patch] 498 ns +- 11 ns: 1.11x faster (-10%)

Ok, now it's 10% faster :-)

[1st1]

New changeset 80bbe6a by Yury Selivanov (jimmylai) in branch 'master':
bpo-31350: Optimize get_event_loop and _get_running_loop (bpo-3347)
80bbe6a

[Mariatta]

New changeset ff125e1 by Mariatta (Miss Islington (bot)) in branch '3.6':
bpo-31350: Optimize get_event_loop and _get_running_loop (GH-3347) (GH-3373)
ff125e1

[Mariatta]

This has been backported.
Thanks all :)