Skip to content

fix: performance regression when sniffio not installed - #1103

Open
CedricCabessa wants to merge 1 commit into
encode:masterfrom
CedricCabessa:fix/perf-sniffio
Open

fix: performance regression when sniffio not installed#1103
CedricCabessa wants to merge 1 commit into
encode:masterfrom
CedricCabessa:fix/perf-sniffio

Conversation

@CedricCabessa

Copy link
Copy Markdown

discussion: #1102

sniffio is optional and lazily imported in current_async_library, when absent we use asyncio

However the cost of import failure is paid every time the function is called.

This patch moves the import at module level, like it is already done for anyio or trio

Why it happens now?:

Httpcore never explicitly declared sniffio but it was installed via anyio until they stopped depending on it:
agronholm/anyio#1021

Here is a sample to reproduce the issue

import asyncio
import time

import httpcore

REQUESTS = 500
RESPONSE = b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\nok"

async def handle(reader, writer):
    try:
        while await reader.readuntil(b"\r\n\r\n"):
            writer.write(RESPONSE)
            await writer.drain()
    except (asyncio.IncompleteReadError, ConnectionResetError):
        pass
    writer.close()

async def main():
    try:
        import sniffio  # noqa: F401

        status = "INSTALLED"
    except ImportError:
        status = "ABSENT"

    server = await asyncio.start_server(handle, "127.0.0.1", 0)
    url = f"http://127.0.0.1:{server.sockets[0].getsockname()[1]}/"

    async with httpcore.AsyncConnectionPool() as pool:
        await pool.request("GET", url)  # warm up the pool

        cpu = time.process_time()
        for _ in range(REQUESTS):
            await pool.request("GET", url)
        cpu = time.process_time() - cpu

    print(f"sniffio {status}: {cpu / REQUESTS * 1e6:.0f} us CPU per request")
    server.close()

asyncio.run(main())

On my machine:

httpcore sniffio CPU / req
master installed ~330 µs
master missing ~500 µs
patched missing ~330 µs
patched installed ~330 µs

`sniffio` is optional and lazily imported in `current_async_library`,
when absent we use `asyncio`

However the cost of import failure is paid every time the function is
called.

This patch moves the import at module level, like it is already done for
`anyio` or `trio`

Why it happens now?:

Httpcore never explicitly declared `sniffio` but it was installed via
`anyio` until they stopped depending on it:
agronholm/anyio#1021

Here is a sample to reproduce the issue

```
import asyncio
import time

import httpcore

REQUESTS = 500
RESPONSE = b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\nok"

async def handle(reader, writer):
    try:
        while await reader.readuntil(b"\r\n\r\n"):
            writer.write(RESPONSE)
            await writer.drain()
    except (asyncio.IncompleteReadError, ConnectionResetError):
        pass
    writer.close()

async def main():
    try:
        import sniffio  # noqa: F401

        status = "INSTALLED"
    except ImportError:
        status = "ABSENT"

    server = await asyncio.start_server(handle, "127.0.0.1", 0)
    url = f"http://127.0.0.1:{server.sockets[0].getsockname()[1]}/"

    async with httpcore.AsyncConnectionPool() as pool:
        await pool.request("GET", url)  # warm up the pool

        cpu = time.process_time()
        for _ in range(REQUESTS):
            await pool.request("GET", url)
        cpu = time.process_time() - cpu

    print(f"sniffio {status}: {cpu / REQUESTS * 1e6:.0f} us CPU per request")
    server.close()

asyncio.run(main())
```

On my machine:

| httpcore | sniffio   | CPU / req |
|----------|-----------|-----------|
| master   | installed | ~330 µs   |
| master   | missing   | ~500 µs   |
| patched  | missing   | ~330 µs   |
| patched  | installed | ~330 µs   |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant