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

Jump to bottom

io/ioutil: support Seeker in ReadAll #9563

Closed
dvyukov opened this issue Jan 11, 2015 · 2 comments
Closed

io/ioutil: support Seeker in ReadAll #9563

dvyukov opened this issue Jan 11, 2015 · 2 comments
Labels
FrozenDueToAge

Comments

@dvyukov
Copy link
Member

dvyukov commented Jan 11, 2015

Currently when you read a large file using ioutil.ReadAll the following bad things happen:

  1. ReadAll makes log(N) unnecessary allocations of total size N which become garbage.
  2. ReadAll makes an unnecessary copy of N bytes.
  3. ReadAll issues log(N) read requests (which is especially unpleasant for remote files).
  4. ReadAll can over-allocate up to 2x more memory than necessary.

ReadAll should support Seeker interface, query size of (the remaining part of) the file, and use it as capacity hint (which is already present in the implementation). That would eliminate all of the above negative effects (no garbage, no overallocation, no small reads).

Objections?

@bradfitz
@robpike
Copy link
Contributor

robpike commented Jan 11, 2015

Don't do that. ReadAll is for Readers, not Files, and putting this kind of specialization into libraries is a recipe for long-term bloat.

Put another way, ReadAll is a simple, general convenience function. If you have performance concerns specific to a particular kind of input, program that case separately. Maybe make it a library function, but not part of ioutil.ReadAll.

@bradfitz
Copy link
Contributor

Per other email thread, I think it's been decided that this shouldn't happen.

@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@bradfitz @dvyukov @robpike @gopherbot