Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

io: make read_to_end not grow unnecessarily #5610

Merged
merged 2 commits into from Apr 16, 2023
Merged

Conversation

tzx
Copy link
Contributor

@tzx tzx commented Apr 8, 2023

Fixes: #5594

Motivation

Explained in #5594

Solution

Make buf.reserve(32) do an additional check with our original capacity, so we can tell read_to_end to allocate a small buffer to read before reading into the main buffer. Doing so allows us to not allocate again when we reach capacity at an EOF, since we don't allocate with the check and read nothing into the small buffer. If we do not have reach an EOF however, we append out read data and continue as normal.

@Darksonn Darksonn added A-tokio Area: The main tokio crate M-io Module: tokio/io labels Apr 8, 2023
@tzx
Copy link
Contributor Author

tzx commented Apr 8, 2023

Alright I have moved logic outside of reserve and into its own function that poll_read_to_end uses. I also changed Vec::with_capacity to using a MaybeUninit array which uses unsafe.

@tzx tzx requested a review from Darksonn April 8, 2023 21:21
tokio/src/io/util/read_to_end.rs Outdated Show resolved Hide resolved
tokio/src/io/util/read_to_end.rs Outdated Show resolved Hide resolved
tokio/src/io/util/read_to_end.rs Outdated Show resolved Hide resolved
@tzx tzx force-pushed the rte-cap branch 2 times, most recently from 52392e5 to 3378a4e Compare April 8, 2023 22:24
@tzx tzx requested a review from Darksonn April 8, 2023 22:53
@tzx
Copy link
Contributor Author

tzx commented Apr 8, 2023

Really wish I can catch these edge cases like you ahaha

Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good to me.

@Darksonn Darksonn merged commit 7aea597 into tokio-rs:master Apr 16, 2023
51 checks passed
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Apr 29, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [tokio](https://tokio.rs) ([source](https://github.com/tokio-rs/tokio)) | dependencies | minor | `1.27.0` -> `1.28.0` |
| [tokio](https://tokio.rs) ([source](https://github.com/tokio-rs/tokio)) | dev-dependencies | minor | `1.27.0` -> `1.28.0` |

---

### Release Notes

<details>
<summary>tokio-rs/tokio</summary>

### [`v1.28.0`](https://github.com/tokio-rs/tokio/releases/tag/tokio-1.28.0): Tokio v1.28.0

[Compare Source](tokio-rs/tokio@tokio-1.27.0...tokio-1.28.0)

##### 1.28.0 (April 25th, 2023)

##### Added

-   io: add `AsyncFd::async_io` ([#&#8203;5542])
-   io: impl BufMut for ReadBuf ([#&#8203;5590])
-   net: add `recv_buf` for `UdpSocket` and `UnixDatagram` ([#&#8203;5583])
-   sync: add `OwnedSemaphorePermit::semaphore` ([#&#8203;5618])
-   sync: add `same_channel` to broadcast channel ([#&#8203;5607])
-   sync: add `watch::Receiver::wait_for` ([#&#8203;5611])
-   task: add `JoinSet::spawn_blocking` and `JoinSet::spawn_blocking_on` ([#&#8203;5612])

##### Changed

-   deps: update windows-sys to 0.48 ([#&#8203;5591])
-   io: make `read_to_end` not grow unnecessarily ([#&#8203;5610])
-   macros: make entrypoints more efficient ([#&#8203;5621])
-   sync: improve Debug impl for `RwLock` ([#&#8203;5647])
-   sync: reduce contention in `Notify` ([#&#8203;5503])

##### Fixed

-   net: support `get_peer_cred` on AIX ([#&#8203;5065])
-   sync: avoid deadlocks in `broadcast` with custom wakers ([#&#8203;5578])

##### Documented

-   sync: fix typo in `Semaphore::MAX_PERMITS` ([#&#8203;5645])
-   sync: fix typo in `tokio::sync::watch::Sender` docs ([#&#8203;5587])

[#&#8203;5065]: tokio-rs/tokio#5065

[#&#8203;5503]: tokio-rs/tokio#5503

[#&#8203;5542]: tokio-rs/tokio#5542

[#&#8203;5578]: tokio-rs/tokio#5578

[#&#8203;5583]: tokio-rs/tokio#5583

[#&#8203;5587]: tokio-rs/tokio#5587

[#&#8203;5590]: tokio-rs/tokio#5590

[#&#8203;5591]: tokio-rs/tokio#5591

[#&#8203;5607]: tokio-rs/tokio#5607

[#&#8203;5610]: tokio-rs/tokio#5610

[#&#8203;5611]: tokio-rs/tokio#5611

[#&#8203;5612]: tokio-rs/tokio#5612

[#&#8203;5618]: tokio-rs/tokio#5618

[#&#8203;5621]: tokio-rs/tokio#5621

[#&#8203;5645]: tokio-rs/tokio#5645

[#&#8203;5647]: tokio-rs/tokio#5647

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS42My4xIiwidXBkYXRlZEluVmVyIjoiMzUuNjQuMCJ9-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1875
Reviewed-by: crapStone <crapstone@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-io Module: tokio/io
Projects
None yet
Development

Successfully merging this pull request may close these issues.

read_to_end grows buf beyond take limit
3 participants