refactor: Rename read_chunk to read_bytes and make it return just a Bytes#535
Open
refactor: Rename read_chunk to read_bytes and make it return just a Bytes#535
Conversation
Also rename read_chunks to read_bytes_many. It already works with Bytes, so the name was a bit weird before. We no longer return the offset, but getting the offset is now possible with bytes_read, but it is rare that people care about the offset for an ordered stream, so it seems justified to put this into a separate fn.
Performance Comparison Report
|
| Scenario | noq | upstream | Delta | CPU (avg/max) |
|---|---|---|---|---|
| large-single | 5942.0 Mbps | 7871.7 Mbps | -24.5% | 96.4% / 122.0% |
| medium-concurrent | 5535.5 Mbps | 7633.5 Mbps | -27.5% | 95.0% / 121.0% |
| medium-single | 3930.1 Mbps | 4189.3 Mbps | -6.2% | 94.2% / 122.0% |
| small-concurrent | 3922.0 Mbps | 5047.3 Mbps | -22.3% | 86.5% / 96.7% |
| small-single | 3583.2 Mbps | 4400.0 Mbps | -18.6% | 85.4% / 95.8% |
Netsim Benchmarks (network simulation)
| Condition | noq | upstream | Delta |
|---|---|---|---|
| ideal | 3205.0 Mbps | 4038.4 Mbps | -20.6% |
| lan | 791.7 Mbps | 810.4 Mbps | -2.3% |
| lossy | 69.8 Mbps | 69.8 Mbps | ~0% |
| wan | 83.8 Mbps | 83.8 Mbps | ~0% |
Summary
noq is 20.7% slower on average
b9f118afbafe958c0f12a2bbb407ae5ed5e7abac - artifacts
Raw Benchmarks (localhost)
| Scenario | noq | upstream | Delta | CPU (avg/max) |
|---|---|---|---|---|
| large-single | 5478.2 Mbps | 8080.0 Mbps | -32.2% | 92.2% / 97.3% |
| medium-concurrent | 5313.6 Mbps | 7793.1 Mbps | -31.8% | 89.8% / 96.4% |
| medium-single | 4391.5 Mbps | 4749.5 Mbps | -7.5% | 98.1% / 136.0% |
| small-concurrent | 3878.0 Mbps | 5247.7 Mbps | -26.1% | 89.8% / 98.6% |
| small-single | 3472.2 Mbps | 4715.3 Mbps | -26.4% | 86.0% / 96.1% |
Netsim Benchmarks (network simulation)
| Condition | noq | upstream | Delta |
|---|---|---|---|
| ideal | 3130.0 Mbps | N/A | N/A |
| lan | 782.4 Mbps | N/A | N/A |
| lossy | 55.9 Mbps | N/A | N/A |
| wan | 83.8 Mbps | N/A | N/A |
Summary
noq is 26.3% slower on average
59dbb3cc108e92e44da4e51cfaa21b3e339631cd - artifacts
Raw Benchmarks (localhost)
| Scenario | noq | upstream | Delta | CPU (avg/max) |
|---|---|---|---|---|
| large-single | 6015.3 Mbps | 7929.2 Mbps | -24.1% | 98.1% / 99.4% |
| medium-concurrent | 5822.8 Mbps | 7782.0 Mbps | -25.2% | 97.0% / 98.4% |
| medium-single | 4088.6 Mbps | 4677.0 Mbps | -12.6% | 96.2% / 98.6% |
| small-concurrent | 3941.7 Mbps | 5213.4 Mbps | -24.4% | 97.4% / 99.5% |
| small-single | 3692.7 Mbps | 4733.1 Mbps | -22.0% | 96.4% / 98.6% |
Netsim Benchmarks (network simulation)
| Condition | noq | upstream | Delta |
|---|---|---|---|
| ideal | 2905.3 Mbps | N/A | N/A |
| lan | 782.6 Mbps | N/A | N/A |
| lossy | 69.8 Mbps | N/A | N/A |
| wan | 83.8 Mbps | N/A | N/A |
Summary
noq is 22.3% slower on average
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/noq/pr/535/docs/noq/ Last updated: 2026-03-26T12:28:22Z |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Rename read_chunk to read_bytes and make it return just a Bytes. Also rename read_chunks to read_bytes_many. It already works with Bytes, so the name was a bit weird before.
We no longer return the offset. Getting the offset is now possible with bytes_read, but it is rare that people care about the offset for an ordered stream, so it seems justified to put this into a separate fn.
Not sure if people agree, but there was an inconsistency before between read_chunk (returns a Chunk, including offset) and read_chunks (fills a bunch of Bytes, no offset).
Also it doesn't seem useful to have Chunk at all for ordered streams. You usually don't care about the offset unless you are reading unordered streams. So I would like to confine Chunk usage to only the (now separate) unordered read API.
Breaking Changes
noq::RecvStream::read_chunk renamed to read_bytes and returns a Bytes.
noq::RecvStream::read_chunks renamedto read_bytes_many.
Notes & open questions
Note: while the other related PR is just renaming, this one is I think actually removing some weirdness.
Why does read_chunk give you an offset but read_chunks does not. And there is no way to get the offset if you need it if you use read_chunks.