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

Attempt to help users avoid unbuffered small reads #445

Open
joshtriplett opened this issue Sep 22, 2024 · 8 comments
Open

Attempt to help users avoid unbuffered small reads #445

joshtriplett opened this issue Sep 22, 2024 · 8 comments

Comments

@joshtriplett
Copy link
Member

Users regularly come to the /r/rust subreddit looking for help when their Rust code doesn't perform as well as they expect. The second most common culprit (after debug vs release build) is doing unbuffered small reads from a file.

I'm wondering if there's anything we could do to help users catch or avert this more easily.

@the8472
Copy link
Member

the8472 commented Sep 22, 2024

The File docs already mention it. All I can think of is a clippy lint, the list doesn't list anything relevant when I search for "buff".

Don't even have to scroll to see it in the RA hover:

Image

Edit: there already is an open issue rust-lang/rust-clippy#1805

@Mark-Simulacrum
Copy link
Member

Perhaps a debug-assertion and/or eprintln that warns if we see repeated reads of under (say) 4kb on a File? If we had some notion of metrics or other "debugging opt-in" I could imagine gating it on that -- similar to e.g. go's support for profiling mutex contention when opting in.

@the8472
Copy link
Member

the8472 commented Sep 22, 2024

We'd need either build-std or yet another mechanism to enable debug asserts in std. The existing one is only meant for UB checks. And it'd probbaly be bad for compile times to make all of them toggleable.

@joshtriplett
Copy link
Member Author

@Mark-Simulacrum I was thinking about exactly that: some way to easily support a cargo perf-smoketest or similar that helps people figure out why their Rust performance is a problem.

Honestly, perhaps a script that ptraces their process and looks at read/write syscall sizes would suffice. The same script could also look at whether the binary was built in debug or release mode.

@cuviper
Copy link
Member

cuviper commented Sep 22, 2024

Another way to help users is to make the "right" thing easier, like:

impl File {
    pub fn open_buffered<P: AsRef<Path>>(path: P) -> Result<BufReader<File>>;
    pub fn create_buffered<P: AsRef<Path>>(path: P) -> Result<BufWriter<File>>;
}

This has an advantage over docs because auto-complete will suggest it.

@joshtriplett
Copy link
Member Author

Another way to help users is to make the "right" thing easier, like:

impl File {
pub fn open_buffered<P: AsRef>(path: P) -> Result<BufReader>;
pub fn create_buffered<P: AsRef>(path: P) -> Result<BufWriter>;
}

This has an advantage over docs because auto-complete will suggest it.

That sounds like a great idea. Want to write an ACP?

@programmerjake

This comment has been minimized.

@cuviper
Copy link
Member

cuviper commented Sep 23, 2024

That sounds like a great idea. Want to write an ACP?

See #446.

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

No branches or pull requests

5 participants