Repository navigation
Fix loop.close() hanging on in-flight DNS resolution - #783
JoshCLWren wants to merge 6 commits into
Conversation
JoshCLWren
changed the title
|
Superseded: The executor implementation measured below has been replaced by native DNS with deferred shutdown cleanup. See the new optimized-build measurements. Original measurements are preserved below. Performance follow-up: these measurements expose substantial regressions in the current executor-backed design. I am returning this PR to draft; passing the correctness/compatibility matrix is insufficient to recommend merging it as implemented. Compared baseline
The contention case explicitly configures four executor workers, occupies all four with application work waiting on an event, and releases that event after 200 ms. The baseline's libuv DNS pool remains independent; the PR queues DNS behind that application work. This is a deliberate saturation demonstration, not a claim about typical application latency. Forward lookups use The current design needs revision: concurrent forward throughput has a much lower median, numeric reverse lookups are roughly 8–10× slower, and DNS inherits application-executor queueing. The executor handoffs and worker-owned loop initialization are candidates for profiling; these measurements do not isolate their individual costs. Feedback on a dedicated bounded resolver pool versus a different ownership/shutdown design would be welcome before developing this approach further. Raw samples (lookups/s, except contention latency in ms)Benchmark scriptBuild both checkouts in place with matching optimized build settings, save this script as python benchmark-dns.py --baseline /path/to/baseline --candidate /path/to/pr --samples 7 --output results.json"""Compare DNS on two optimized uvloop builds without external DNS traffic.
Run each sample in a fresh process. The parent alternates build order and
reports medians; individual samples and build paths remain in JSON output.
"""
import argparse
import asyncio
import concurrent.futures
import json
import os
import pathlib
import platform
import socket
import statistics
import subprocess
import sys
import threading
import time
def child(args):
sys.path.insert(0, str(pathlib.Path(args.repo).resolve()))
import uvloop
import uvloop.loop
loop = uvloop.new_event_loop()
assert not loop._debug_cc, 'benchmark requires an optimized build'
assert pathlib.Path(uvloop.loop.__file__).is_relative_to(
pathlib.Path(args.repo).resolve())
asyncio.set_event_loop(loop)
async def lookup(reverse=False, numeric=False):
if reverse:
return await loop.getnameinfo(
('127.0.0.1', 443), socket.NI_NUMERICHOST | socket.NI_NUMERICSERV)
return await loop.getaddrinfo(
'127.0.0.1' if numeric else 'localhost', 443,
family=socket.AF_INET, type=socket.SOCK_STREAM)
async def run():
if args.case == 'executor_saturated':
# Explicitly use four workers so this case is reproducible across
# machines. Occupy every worker with sleeping application work.
loop.set_default_executor(
concurrent.futures.ThreadPoolExecutor(max_workers=4))
release = threading.Event()
ready = [threading.Event() for _ in range(4)]
def application_work(event):
event.set()
release.wait()
tasks = [loop.run_in_executor(None, application_work, e)
for e in ready]
while not all(e.is_set() for e in ready):
await asyncio.sleep(0.001)
timer = loop.call_later(0.2, release.set)
start = time.perf_counter()
try:
await lookup()
elapsed = time.perf_counter() - start
finally:
release.set()
timer.cancel()
await asyncio.gather(*tasks)
return {'dns_latency_ms': elapsed * 1000}
reverse = args.case.startswith('reverse')
numeric = args.case == 'numeric_fast_path'
concurrency = 32 if args.case.endswith('concurrent') else 1
count = 32768 if numeric else (4096 if reverse else 8192)
for _ in range(100):
await lookup(reverse, numeric)
async def worker():
for _ in range(count // concurrency):
result = await lookup(reverse, numeric)
assert result
start = time.perf_counter()
await asyncio.gather(*(worker() for _ in range(concurrency)))
elapsed = time.perf_counter() - start
return {'lookups_per_second': count / elapsed,
'microseconds_per_lookup': elapsed * 1e6 / count,
'lookups': count, 'concurrency': concurrency}
try:
result = loop.run_until_complete(run())
loop.run_until_complete(loop.shutdown_default_executor())
finally:
loop.close()
result.update(module=uvloop.loop.__file__, version=uvloop.__version__,
libuv_version=uvloop.loop.libuv_get_version())
print(json.dumps(result))
def main():
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument('--repo')
parser.add_argument('--case')
parser.add_argument('--baseline')
parser.add_argument('--candidate')
parser.add_argument('--samples', type=int, default=7)
parser.add_argument('--output')
args = parser.parse_args()
if args.repo:
child(args)
return
cases = ('forward_sequential', 'forward_concurrent',
'reverse_sequential', 'reverse_concurrent',
'numeric_fast_path', 'executor_saturated')
result = {'environment': {'python': sys.version,
'platform': platform.platform(),
'cpu_count': os.cpu_count(),
'uv_threadpool_size': os.getenv('UV_THREADPOOL_SIZE', 'default (4)'),
'samples': args.samples}, 'cases': {}}
env = dict(os.environ)
env.pop('PYTHONASYNCIODEBUG', None)
env.pop('LD_PRELOAD', None)
env['UV_THREADPOOL_SIZE'] = '4'
for case in cases:
samples = {name: [] for name in ('baseline', 'candidate')}
for i in range(args.samples):
order = ('baseline', 'candidate') if i % 2 == 0 else ('candidate', 'baseline')
for name in order:
output = subprocess.check_output(
[sys.executable, str(pathlib.Path(__file__).resolve()),
'--repo', getattr(args, name), '--case', case],
text=True, env=env, timeout=60)
samples[name].append(json.loads(output))
key = 'dns_latency_ms' if case == 'executor_saturated' else 'lookups_per_second'
medians = {name: statistics.median(s[key] for s in runs)
for name, runs in samples.items()}
result['cases'][case] = {'samples': samples, 'median': medians,
'candidate_over_baseline': medians['candidate'] / medians['baseline']}
print(case, json.dumps(medians), flush=True)
pathlib.Path(args.output).write_text(json.dumps(result, indent=2) + '\n')
if __name__ == '__main__':
main() |
|
Replaced the executor implementation with native libuv DNS plus deferred cleanup of executing requests at close (cdbfbb4). The earlier performance warning applies to the superseded executor implementation. Current comparison against 07b10a4: both uvloop and libuv built with
Paired bootstrap 95% intervals for each throughput ratio include 1.0. Numeric forward uses socktype=0 and AI_NUMERICHOST to exercise native requests rather than uvloop's numeric fast path. Concurrent localhost lookup measured 13,933 vs 13,890 calls/s (-0.31%). With the application's executor occupied, native DNS latency remains about 2 ms rather than waiting for application work. Methodology caveats: unrestricted host scheduling was noisy enough that identical-baseline A/A comparisons differed by ~12%; fixed affinity reduced this noise. Localhost sequential NSS timings remain noisy. These results show no material regression in the measured workloads, rather than proving universal performance parity. Also, the earlier executor comparison used the same unoptimized libuv archive on both sides; this new comparison corrects that build configuration and uses optimized libuv on both sides. A reproducible harness, including A/A controls and executor saturation, is now included at examples/bench/dns.py. All 18 Linux/macOS configurations and the aggregate regression check now pass on cdbfbb4 (CI run); the PR is ready for review. |
1 participant
Fixes #777.
loop.close()currently waits for executing libuv DNS requests, so a slow or stuck resolver can block shutdown indefinitely. This revision keeps DNS on libuv's existing resolver pool and makes close return without waiting for executing lookups. It replaces the earlier executor-based implementation, which regressed throughput and shared the application's executor queue.DNS requests are tracked with intrusive C links. Close cancels queued requests, drains handle close callbacks without blocking, and hands the native loop to a daemon cleanup thread only if executing DNS remains. That thread retains the loop until native completion; closed-loop callbacks discard results and free returned address data. Normal DNS uses the original native request and callback path without Python executor submissions, locks, or additional Python allocations. Closed-loop
time()avoids accessing the native loop during cleanup.An executing libc resolver cannot be interrupted: a lookup that never returns retains its native loop and cleanup thread. This change makes synchronous loop close nonblocking; it does not promise to interrupt DNS or change process-exit behavior.
Validation includes deterministic Linux subprocess tests for forward/reverse resolution, successful and failed late results, queued cancellation, idempotent close, retained loop lifetime and eventual collection, plus independence from a saturated application executor. Python 3.9 and 3.10 DNS suites pass locally. The Python 3.13 native debug build passes all 535 locally runnable tests (32 existing skips); the original blocked-resolver reproducer returns from close in 1 ms while native DNS sleeps for 10 seconds. A free-threaded debug build passed 180 repeated shutdown/lifetime cases with debug allocation enabled. All 18 Linux/macOS configurations and the aggregate Regression Tests check pass on cdbfbb4 (CI run), covering Python 3.9–3.15 and free-threaded 3.14/3.15, normal/debug builds, and asyncio debug mode on/off. Locally, IPv6 UDP sends to ::1 fail with EPERM even through a plain socket, so two unrelated full-suite tests cannot run successfully in this environment.
The reproducible benchmark is
examples/bench/dns.py. Matching optimized uvloop and libuv builds, 15 alternating process pairs, and fixed CPU affinity give native numeric forward throughput changes of -0.54% sequential / +0.08% concurrent, and reverse changes of -1.29% / +2.14%. All paired bootstrap 95% intervals include parity. Localhost concurrent throughput changes by -0.31%; DNS still completes in about 2 ms with the application executor saturated. These are local measurements, not a guarantee for every workload. The revised implementation is ready for review.