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

excessive memory usage in more #6397

Open
mjguzik opened this issue May 13, 2024 · 6 comments
Open

excessive memory usage in more #6397

mjguzik opened this issue May 13, 2024 · 6 comments
Labels

Comments

@mjguzik
Copy link

mjguzik commented May 13, 2024

Running it over a test file of about 1G in size results in 15G RSS.

Problems start with slurping the entire thing upfront, I don't know how it manages to amass 15 x memory overhead on top of it.

more from gnu coreutils sits at around 2MB.

How to repro:

  1. generate a file of about 1G: perl -e 'print "meh\n" x (256 * 1024 * 1024)' > testfile
  2. cargo run -r testfile
@sylvestre
Copy link
Sponsor Contributor

note that we have been focusing on compatibility, not perf or memory usage for now (even if some programs are already doing better than GNU's)

@tertsdiepraam
Copy link
Member

True, but I also agree that 15x the filesize is a bit excessive 😄

@pstef
Copy link

pstef commented May 13, 2024

Even 100% would be excessive, consumed memory shouldn't scale linearly with the file size at all.

@tertsdiepraam
Copy link
Member

It's also just extremely slow to load such a file. I'm looking into making this at least a bit better.

@mjguzik
Copy link
Author

mjguzik commented May 26, 2024

note that we have been focusing on compatibility, not perf or memory usage for now (even if some programs are already doing better than GNU's)

For coreutils sensible memory usage is part of correctness, especially so for a tool like more.

For example say I logged in to a production machine with the intent of running more /var/log/log_file_sized_1G_or_more. Suppose the the explosive memory usage problem is fixed, but the entire file is still slurped upfront, resulting in 1G RSS.
If the machine I'm running this on does not have a free 1G we are running into trouble. And chances are things are already going wrong, hence why I'm logging in to poke around (including reading logs) in the first place.

The gnu coreutils variant uses few MB of ram making itself a perfectly viable tool in that scenario.

That is to say this code needs to deal with the file in chunks and at the moment is not fit for use whatsoever. Any work done on it which would be thrown away while making the adjustment is a waste of time, which would be better spent either reworking this or beating other tools to shape in a lasting manner.

@tertsdiepraam
Copy link
Member

tertsdiepraam commented May 26, 2024

Not useful for you does not mean useless for everyone. We've already agreed this is an issue and want to fix it. Your point has been made.

Edit: For a bit more context: there are many more parts to get right here that are independent of this issue: the keyboard input, the rendering, the argument parsing, etc. Reworking the way the buffer works can still be while other changes are being made.

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

No branches or pull requests

5 participants