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

Memory leak & poor performance when using Rust Language Server on Linux #72

Closed
alexheretic opened this issue May 31, 2019 · 4 comments
Closed

Comments

@alexheretic
Copy link

alexheretic commented May 31, 2019

I've traced a atom memory & performance issue when using RLS (Rust Language Server) to Atom's version of this project.

When watching a rust project directory with nsfw and running RLS, nsfw will leak memory and consume cpu.

Downstream: atom#7, rust-lang/atom-ide-rust#104

Steps to reproduce

  • Install node & rustup latest stable (see https://rustup.rs, v1.35.0 at time of writing)
  • Install rls rustup component add rls
  • Checkout a rust project that isn't tiny, say regex d4b9419
git clone https://github.com/rust-lang/regex
cd regex
git checkout d4b9419
  • Construct a minimal node project with nsfw in another directory
{
  "name": "ntmp",
  "version": "1.0.0",
  "dependencies": {
    "nsfw": "1.2.2"
  }
}
  • Watch the rust project
node                                                                                                                              
> require('nsfw')('/home/alex/project/regex', events => {}).then(w => w.start())
  • Run RLS in the rust project
cd regex
rls --cli
  • Restart RLS & re-run to see further leakage.

With these steps it's easy enough to see nsfw consume several GB of ram & lots of CPU as RLS is running. This memory usage/leakage is worse on larger rust projects.

RLS uses cargo/rustc to compile code and generates lots of data in the ./target directory in the project which is what triggers this issue. Strangely though running cargo directly doesn't cause this memory issue. I not sure why this is RLS specific.

@implausible
Copy link
Contributor

Thanks for reporting this. I will take a look at this when I get availability for this project next.

@Leo1003
Copy link
Contributor

Leo1003 commented Jun 11, 2019

+1 for this issue. The watcherService used by Visual Studio Code is actually nsfw. It is using more and more memory while I was coding...


Here are some screenshots I posted in vscode repo previously:
Coding for a while...
ScreenShot1
Coding for a while again...
ScreenShot2


Here are parts of outputs of pmap -x of vscode watcherService:
(Sorted by RSS, in KB)


22207:   /opt/visual-studio-code/code /opt/visual-studio-code/resources/app/out/bootstrap-fork --type=watcherService
Address           Kbytes     RSS   Dirty Mode  Mapping
000018907da9f000   18588   17584   17584 rw---   [ anon ]
000018908a12d000   17564   14304   14304 rw---   [ anon ]
0000189080317000   13192   13188   13188 rw---   [ anon ]
000018908ffd9000   13044   13044   13044 rw---   [ anon ]
000018908244a000   12892   12892   12892 rw---   [ anon ]
000018909428e000   12832   12832   12832 rw---   [ anon ]
0000189096367000   12820   12820   12820 rw---   [ anon ]
000018908df29000   12700   12692   12692 rw---   [ anon ]
0000189085152000   12812   12644   12644 rw---   [ anon ]
000018908c8d0000   12512   12512   12512 rw---   [ anon ]
0000189085dd6000   12444   12340   12340 rw---   [ anon ]
000018907ce67000   12508   12240   12240 rw---   [ anon ]
0000189088ac3000   13468   11856   11856 rw---   [ anon ]
00001890921e7000   11608   11608   11608 rw---   [ anon ]
000018908b255000   12616   11476   11476 rw---   [ anon ]
0000189087f46000   11760   11444   11444 rw---   [ anon ]
0000189092d3e000   11420   11420   11420 rw---   [ anon ]
00001890916bf000   11420   11420   11420 rw---   [ anon ]
000018907f7ef000   11420   11372   11372 rw---   [ anon ]
0000189087426000   11388   11344   11344 rw---   [ anon ]
000018907ecc7000   11420   11236   11236 rw---   [ anon ]
0000189083b02000   11420   11204   11204 rw---   [ anon ]
000018908462a000   11420   11080   11080 rw---   [ anon ]
00007fb11b0ad000   13012   10924       0 r-x-- libnode.so
000018909593f000   10396   10396   10396 rw---   [ anon ]
0000189094f17000   10396   10396   10396 rw---   [ anon ]
0000189093866000   10396   10396   10396 rw---   [ anon ]
0000189090c97000   10396   10396   10396 rw---   [ anon ]
000018908eb91000   10396   10396   10396 rw---   [ anon ]
000018908bea8000   10396   10396   10396 rw---   [ anon ]
......

@Leo1003
Copy link
Contributor

Leo1003 commented Jun 11, 2019

I used @alexheretic 's way and valgrind to find memory leak, I found some interesting outputs:
A part of valgrind output:

==22401== 4,103,856 bytes in 129,451 blocks are definitely lost in loss record 1,769 of 1,769
==22401==    at 0x483877F: malloc (vg_replace_malloc.c:299)
==22401==    by 0x57076FE: strdup (in /usr/lib/libc-2.29.so)
==22401==    by 0x485A226: InotifyEventLoop::work() (in /home/leo/library/ntmp/node_modules/nsfw/build/Release/nsfw.node)
==22401==    by 0x485BD78: InotifyEventLoop::InotifyEventLoop(int, InotifyService*)::{lambda(void*)#1}::_FUN(void*) (in /home/leo/library/ntmp/node_modules/nsfw/build/Release/nsfw.node)
==22401==    by 0x5665A91: start_thread (in /usr/lib/libpthread-2.29.so)
==22401==    by 0x5779CD2: clone (in /usr/lib/libc-2.29.so)

And the codes in src/linux/InotifyEventLoop.cpp and includes/linux/InotifyService.h:
Focus on the function call using strdup(), and the argument type is std::string!

I wrote some code to verify if this would cause leaks

#include <iostream>
#include <string>
#include <string.h>
using namespace std;

const char *test_string = "This is a string just for testing.";

void printString(string s)
{
    cout << s << endl;
}

int main()
{
    // Cause memory leaks!!!
    //printString(strdup(test_string));

    // No memory leaks occurred
    printString(test_string);
    return 0;
}

And the result is YES!

Since std::string's constructor is accepting const char* and copy the data by itself. So, it won't take the ownership of the c-string allocated by strdup(). Therefore, the one allocated by strdup() won't be freed!

@alexheretic
Copy link
Author

Nice one @Leo1003 I can no longer reproduce the leaked memory usage.

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

No branches or pull requests

3 participants