Skip to content

gh-150621: avoid quadratic bytes slicing in asyncio.protocols._feed_data_to_buffered_proto#150622

Open
deadlovelll wants to merge 3 commits into
python:mainfrom
deadlovelll:gh-150621-feed_buff_mv
Open

gh-150621: avoid quadratic bytes slicing in asyncio.protocols._feed_data_to_buffered_proto#150622
deadlovelll wants to merge 3 commits into
python:mainfrom
deadlovelll:gh-150621-feed_buff_mv

Conversation

@deadlovelll
Copy link
Copy Markdown
Contributor

@deadlovelll deadlovelll commented May 30, 2026

Track the offset into data instead of reslicing. Drops total work from O(N^2) to O(N).

See gh-150621 for the benchmark table.

To get the results of new version run: ./python.exe buffer_memory_view_bench.py --variant new -o new.json
To get the results of old one: ./python.exe buffer_memory_view_bench.py --variant old -o old.json
Get the compare table: ./python.exe -m pyperf compare_to old.json new.json --table

Benchmark script
import pyperf
from asyncio import BufferedProtocol


def feed_old(proto, data):
    data_len = len(data)
    while data_len:
        buf = proto.get_buffer(data_len)
        buf_len = len(buf)
        if not buf_len:
            raise RuntimeError('get_buffer() returned an empty buffer')

        if buf_len >= data_len:
            buf[:data_len] = data
            proto.buffer_updated(data_len)
            return
        else:
            buf[:buf_len] = data[:buf_len]
            proto.buffer_updated(buf_len)
            data = data[buf_len:]
            data_len = len(data)


def feed_new(proto, data):
    data_len = len(data)
    start = 0
    while data_len:
        buf = proto.get_buffer(data_len)
        buf_len = len(buf)
        if not buf_len:
            raise RuntimeError('get_buffer() returned an empty buffer')

        if buf_len >= data_len:
            buf[:data_len] = data[start:start + data_len] if start else data
            proto.buffer_updated(data_len)
            return
        else:
            buf[:buf_len] = data[start:start + buf_len]
            proto.buffer_updated(buf_len)
            start += buf_len
            data_len -= buf_len


class Proto(BufferedProtocol):
    def __init__(self, buf_size):
        self._buf = bytearray(buf_size)

    def get_buffer(self, sizehint):
        return self._buf

    def buffer_updated(self, nbytes):
        pass


def bench(loops, feed, data, proto):
    range_it = range(loops)
    t0 = pyperf.perf_counter()
    for _ in range_it:
        feed(proto, data)
    return pyperf.perf_counter() - t0


def add_cmdline_args(cmd, args):
    cmd.extend(("--variant", args.variant))


runner = pyperf.Runner(add_cmdline_args=add_cmdline_args)
runner.argparser.add_argument(
    "--variant", choices=["old", "new"], required=True,
    help="impl to bench",
)
args = runner.parse_args()
feed = feed_old if args.variant == "old" else feed_new

scenarios = [
    (64 * 1024,        4096),
    (256 * 1024,       4096),
    (1024 * 1024,      4096),
    (4 * 1024 * 1024,  4096),
    (1024 * 1024,      65536),

    (100,              4096),    
    (1024,             4096),    
    (4096,             4096),    
    (32 * 1024,        65536),
]

for ds, bs in scenarios:
    proto = Proto(bs)
    data = b"x" * ds
    runner.bench_time_func(f"data={ds:>8} buf={bs:>5}", bench, feed, data, proto)

@picnixz picnixz changed the title gh-150621 asyncio.protocols._feed_data_to_buffered_proto: avoid O(N^2) bytes slicing gh-150621: avoid quadratic bytes slicing in asyncio.protocols._feed_data_to_buffered_proto May 30, 2026
@picnixz picnixz changed the title gh-150621: avoid quadratic bytes slicing in asyncio.protocols._feed_data_to_buffered_proto gh-150621: avoid quadratic bytes slicing in asyncio.protocols._feed_data_to_buffered_proto May 30, 2026
@picnixz
Copy link
Copy Markdown
Member

picnixz commented May 30, 2026

FTR, the titles of your PRs are always missing a colon after the gh-XXXXXX issue number (I've added it)

@deadlovelll
Copy link
Copy Markdown
Contributor Author

FTR, the titles of your PRs are always missing a colon after the gh-XXXXXX issue number (I've added it)

Thank you! I'll take this into account for the future

@deadlovelll deadlovelll force-pushed the gh-150621-feed_buff_mv branch from 7e04c75 to cfda592 Compare May 30, 2026 18:43
Copy link
Copy Markdown
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Can you update your benchmarks?
  2. If there is a way to see the improvement with a visible example (like, where is this function being called in a real-world application, or if there is some public endpoint we can refer to), please add a NEWS entry mentioning this improvement.
  3. Can you add some tests that would exercise this change if none exists?
  4. Please don't force-push in the future (here it's ok because I didn't review it yet, but forcepushing discards commit history and incremental reviews are hard).

@deadlovelll
Copy link
Copy Markdown
Contributor Author

  1. Can you update your benchmarks?
  2. If there is a way to see the improvement with a visible example (like, where is this function being called in a real-world application, or if there is some public endpoint we can refer to), please add a NEWS entry mentioning this improvement.
  3. Can you add some tests that would exercise this change if none exists?
  4. Please don't force-push in the future (here it's ok because I didn't review it yet, but forcepushing discards commit history and incremental reviews are hard).

Updated benchmarks. Removed memoryview mentions from issue and PR description.
Added NEWS entry mentioning the call site
Added FeedDataToBufferedProtoTests inLib/test/test_asyncio/test_protocols.py with two tests:

  1. test_large_multi_iteration cacthes offset drift
  2. test_memoryview_input verifies that offset-tracking works with memoryview input too
    Got it about force-pushing, I will avoid it in future contribs

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants