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: increase MAX_BUF from 16384 to 2MiB #5397

Merged
merged 2 commits into from Jan 27, 2023
Merged

io: increase MAX_BUF from 16384 to 2MiB #5397

merged 2 commits into from Jan 27, 2023

Conversation

brxken128
Copy link
Contributor

Motivation

While working on Spacedrive, we were noticing some oddities during file reading in regards to encryption and decryption.

We use a stream reader where 1MiB of the source file is read, encrypted, written and this repeats until the end. We arguably need to use read for this, as opposed to read_exact, as we need to know the exact amount of bytes read and guarantee that if the full 1MiB isn't read, the buffer still contains valid data.

I noticed some oddities while calling read, and that it was only reading 16KB at a time. This would have been fine, but this results in 64 read calls until the 1MiB buffer is populated.

Solution

The 64 read calls to populate a 1MiB buffer turned out to be extremely slow. By increasing the buffer size to 2MiB, we were able to bring encryption time down from 24~ seconds to 8.7~ seconds.

I have done some testing regarding the 2MiB value. 1MiB seemed to be a little too low, and gave us a time-to-encrypt of 15~ seconds. 4MiB gave us 8.7~ seconds also, but 2MiB should be cheaper and more universal.

This value could likely be configurable somewhere, but I'm not too sure how the API would work there.

On another note, 2MiB may be too large for some platforms and I'm not too sure if this is allocated on the stack or heap. In the event it's stack allocated, we may be better off increasing the default to 32/64KB and allowing it to be configurable.

@Darksonn Darksonn added A-tokio Area: The main tokio crate M-fs Module: tokio/fs labels Jan 27, 2023
@Darksonn
Copy link
Contributor

It is heap allocated.

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.

LGTM.

@Darksonn Darksonn merged commit fe2dcb9 into tokio-rs:master Jan 27, 2023
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Feb 2, 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.24.2` -> `1.25.0` |
| [tokio](https://tokio.rs) ([source](https://github.com/tokio-rs/tokio)) | dev-dependencies | minor | `1.24.2` -> `1.25.0` |

---

### Release Notes

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

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

##### 1.25.0 (January 28, 2023)

##### Fixed

-   rt: fix runtime metrics reporting ([#&#8203;5330])

##### Added

-   sync: add `broadcast::Sender::len` ([#&#8203;5343])

##### Changed

-   fs: increase maximum read buffer size to 2MiB ([#&#8203;5397])

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

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

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

</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:eyJjcmVhdGVkSW5WZXIiOiIzNC4xMTYuMSIsInVwZGF0ZWRJblZlciI6IjM0LjExOS4yIn0=-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1761
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-fs Module: tokio/fs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants