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.

Sign up for GitHub

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

Jump to bottom

Document that File does not buffer reads/writes #119319

Merged
merged 1 commit into from
Jan 3, 2024
Merged

Document that File does not buffer reads/writes #119319

merged 1 commit into from
Jan 3, 2024

Conversation

chfogelman
Copy link
Contributor

...and refer to BufReader/BufWriter.

This is a common source of efficiency issues in Rust programs written naively. Including this information with the File docs, and adding a link to the wrapper types, will help discoverability.

@rustbot
Copy link
Collaborator

rustbot commented Dec 26, 2023

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 26, 2023
the8472
the8472 reviewed Dec 26, 2023
View reviewed changes
library/std/src/fs.rs Outdated
@@ -31,6 +31,11 @@ use crate::time::SystemTime;
/// on closing are ignored by the implementation of `Drop`. Use the method
/// [`sync_all`] if these errors must be manually handled.
///
/// `File` objects do not buffer reads and writes by default. For efficiency,
/// consider wrapping the file in a [`BufReader`] or [`BufWriter`] when performing
/// multiple [`read`] or [`write`] calls, unless unbuffered reads and writes
Copy link
Member

Choose a reason for hiding this comment

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

It should say small reads and writes. You won't get any speedup if you're performing multiple large operations that exceed the buffer size.

library/std/src/fs.rs Outdated
@@ -31,6 +31,11 @@ use crate::time::SystemTime;
/// on closing are ignored by the implementation of `Drop`. Use the method
/// [`sync_all`] if these errors must be manually handled.
///
/// `File` objects do not buffer reads and writes by default. For efficiency,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// `File` objects do not buffer reads and writes by default. For efficiency,
/// `File` does not buffer reads and writes. For efficiency,

The "by default" doesn't add anything, it can't be changed. And wrapping it in a buffer makes it a different type.

@Mark-Simulacrum
Copy link
Member

(It seems like you pushed a new commit but it didn't actually contain any changes according to GitHub? Maybe you forgot to add the changes to the commit)

@the8472 the8472 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 1, 2024
@chfogelman
Copy link
Contributor Author

Yes, sorry, you're exactly right. The requested changes are now pushed.

@the8472
Copy link
Member

the8472 commented Jan 1, 2024

@bors r+ rollup=always

@bors
Copy link
Contributor

bors commented Jan 1, 2024

📌 Commit 5cbe41a has been approved by the8472

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 1, 2024
fmease added a commit to fmease/rust that referenced this pull request Jan 2, 2024
@fmease
Rollup merge of rust-lang#119319 - chfogelman:buffered-file-doc, r=th…
018f07b 
…e8472

Document that File does not buffer reads/writes

...and refer to `BufReader`/`BufWriter`.

This is a common source of efficiency issues in Rust programs written naively. Including this information with the `File` docs, and adding a link to the wrapper types, will help discoverability.
@fmease fmease mentioned this pull request Jan 2, 2024
Closed
fmease added a commit to fmease/rust that referenced this pull request Jan 3, 2024
@fmease
Rollup merge of rust-lang#119319 - chfogelman:buffered-file-doc, r=th…
383daa7 
…e8472

Document that File does not buffer reads/writes

...and refer to `BufReader`/`BufWriter`.

This is a common source of efficiency issues in Rust programs written naively. Including this information with the `File` docs, and adding a link to the wrapper types, will help discoverability.
This was referenced Jan 3, 2024
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 3, 2024
@bors
Auto merge of rust-lang#119549 - fmease:rollup-jxvbfes, r=fmease
139fb22 
Rollup of 21 pull requests

Successful merges:

 - rust-lang#119086 (Query panic!() to useful diagnostic)
 - rust-lang#119239 (Remove unnecessary arm in `check_expr_yield`)
 - rust-lang#119298 (suppress change-tracker warnings in CI containers)
 - rust-lang#119319 (Document that File does not buffer reads/writes)
 - rust-lang#119434 (rc: Take *const T in is_dangling)
 - rust-lang#119444 (Rename `TyCtxt::is_closure` to `TyCtxt::is_closure_or_coroutine`)
 - rust-lang#119474 (Update tracking issue of naked_functions)
 - rust-lang#119476 (Pretty-print always-const trait predicates correctly)
 - rust-lang#119477 (rustdoc ui: adjust tooltip z-index to be above sidebar)
 - rust-lang#119479 (Remove two unused feature gates from rustc_query_impl)
 - rust-lang#119487 (Minor improvements in comment on `freshen.rs`)
 - rust-lang#119492 (Update books)
 - rust-lang#119494 (Deny defaults for higher-ranked generic parameters)
 - rust-lang#119498 (Update deadlinks of `strict_provenance` lints)
 - rust-lang#119505 (Don't synthesize host effect params for trait associated functions marked const)
 - rust-lang#119510 (Report I/O errors from rmeta encoding with emit_fatal)
 - rust-lang#119512 (Mark myself as back from leave)
 - rust-lang#119514 (coverage: Avoid a query stability hazard in `function_coverage_map`)
 - rust-lang#119523 (llvm: Allow `noundef` in codegen tests)
 - rust-lang#119534 (Update `thread_local` examples to use `local_key_cell_methods`)
 - rust-lang#119544 (Fix: Properly set vendor in i686-win7-windows-msvc target)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4e265d9 into rust-lang:master Jan 3, 2024
11 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Jan 3, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 3, 2024
@rust-timer
Unrolled build for rust-lang#119319
4fbd256 
Rollup merge of rust-lang#119319 - chfogelman:buffered-file-doc, r=the8472

Document that File does not buffer reads/writes

...and refer to `BufReader`/`BufWriter`.

This is a common source of efficiency issues in Rust programs written naively. Including this information with the `File` docs, and adding a link to the wrapper types, will help discoverability.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@the8472 the8472 the8472 left review comments

Assignees

@the8472 the8472

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.77.0
Development

Successfully merging this pull request may close these issues.
5 participants
@chfogelman @rustbot @Mark-Simulacrum @the8472 @bors