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

Bug: out of memory #18

Closed
powerman opened this issue Jul 10, 2024 · 7 comments · Fixed by #21
Closed

Bug: out of memory #18

powerman opened this issue Jul 10, 2024 · 7 comments · Fixed by #21

Comments

@powerman
Copy link

nerdfix check --format json -r . 2>/dev/null >nerdfix.json started in my home dir in a minute takes all my RAM (32GB) and begin swapping, so I killed it.

$ nerdfix --version
nerdfix 0.4.0
cheat-sheet: c3e3d5fec7c8744eae6f1cdfb90a38cdc1033519
@loichyan
Copy link
Owner

loichyan commented Jul 11, 2024

Should be fixed in #21, I tested it on my home dir and did not encounter infinite memory growth. BTW, I also added a binary files filtering, as the endless "Invalid UTF-8" errors were really annoying :|

@powerman
Copy link
Author

powerman commented Jul 11, 2024

I don't think it's fixed. Just tried again: I noticed it may take a lot of RAM but then release and then repeats. But, still, I've seen it takes about 29GB once before releasing, so it's still far away from fair RAM use for such a tool.

@loichyan
Copy link
Owner

I noticed it may take a lot of RAM but then release and then repeats.

It must be loading a super large file, I don't have very large files under my home dir so I'm not experiencing this. I'm in the process of implementing stream processing so that nerdfix only needs to read a small segment of the file instead of the whole into memory.

@loichyan loichyan reopened this Jul 11, 2024
@powerman
Copy link
Author

Well, sure, there are large files in a home dir. Some of them are even related to this tool 😄

4718852415 bytes ./proj/fonts/nerd-fonts/.git/objects/pack/pack-aaca106c27527881903673e6138df3ddb304514e.pack

but the largest ones are some game files in .local/share/Steam/ (24GB) and images in ./VirtualBox VMs/ (up to 34GB).

And I believe this is normal use case for the tool.

Some ideas:

  • Do not even try to process large (e.g. 10MB by default) files. Unlikely they'll contain icons to fix. Provide an option to configure this.
  • Do not load whole file in memory, this is not needed for tool's functionality, so process it as a stream using small buffer (say, 64KB). May need special handling for case when icon's bytes will be split between current and following buffer.

@loichyan
Copy link
Owner

Done in #22. Could you help me test it?

@powerman
Copy link
Author

Sure.
The memory issue looks fixed: it uses about 26MB RAM.
This version is also 2x faster: 6m43s vs 12m29s (v0.4.0).

There are some differences in output between these two versions, but at a glance all of them are okay:

  • New version outputs some icons before it fail on invalid UTF8 error, v0.4.0 fail on this error without any icons output.
  • New version skips large files.
  • New version skips binary files.

So, looks good to me!

@loichyan
Copy link
Owner

New version outputs some icons before it fail on invalid UTF8 error, v0.4.0 fail on this error without any icons output

This is interesting, it may be checking a partially UTF8 file, or reading a binary file unexpectedly (I experienced this problem sometimes, as the content inspector library we use is not 100% accurate and occasionally detects some binary files as UTF-8 files). Anyway, this should be acceptable and the main problem is solved, thanks for your feedback!

loichyan added a commit that referenced this issue Jul 14, 2024
### ✨ Overview

This release mainly addresses the high memory usage issue reported in #18: fixed a potential memory leak (#21) and implemented stream processing (#22).

Also, some UI changes were introduced in #21, as we switched the diagnostic reporter from [codespan_reporting](https://docs.rs/codespan-reporting/latest/codespan_reporting) to [miette](https://docs.rs/miette/latest/miette).

### 🚀 Features

- **(runtime)** Set exact file size limit
- **(runtime)** Add file limitation
- **(runtime)** Filter out binary files

### 🐛 Bug Fixes

- **(cli)** Report the source path of diagnostics (#23)
- **(cli)** Subtract with overflow

### 🚜 Refactor

- **(runtime)** Implement stream processing
- **(runtime)** Zero-copy diagnostics reporting
- Replace `codespan-reporting` with `miette`
- **(util)** Rename `tryb!` to `try!`

### 📚 Documentation

- **(readme)** Update badge URLs

### 🎨 Styling

- Format with prettier
- Make nightly rustfmt compatible with the stable rustfmt
- Format with nightly rustfmt

### ⚙️ Miscellaneous Tasks

- Generate UTC release date
- Report Cargo check results
- Rewrite
- Fix typos
- Fix typos
- Update author name
- Add checks (#17)

### 🛠️ Build

- **(nix)** Update flakes
- **(cargo)** Update dependencies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants