jscs-dev / node-jscs

:arrow_heading_up: JavaScript Code Style checker (unmaintained)
https://jscs-dev.github.io
MIT License
4.96k stars 513 forks source link

jscs reproducibly runs out of memory when linting files #2124

Closed kirbysayshi closed 8 years ago

kirbysayshi commented 8 years ago

jscs appears to have a memory leak that is tied to the size and number of the input files. I've made a branch that consistently runs out of memory on both node 0.12 and 5.1.1. The branch copies a single file from node_modules 1500 and then runs jscs using a single instance. While this is a synthetic case, and some may say that ~1500 files is a large number, the same issue is manifesting on our real-world source tree of about 3000 files (and that is of course excluding node_modules, bower, vendor, minified files, etc).

To reproduce:

Compare view: https://github.com/jscs-dev/node-jscs/compare/master...kirbysayshi:ksh/oom (I used the 2.9.0 tag because master was throwing an error.)

Note that even with no rules in place and using --esnext this condition still occurs, which rules out a specific parser.

I'm pretty unfamiliar with the jscs code base, but I'm guessing it's that every Errors object has a reference to _file which contains the entire esprima tree as well as file source, and that those Errors objects are kept around until the program exits.

Possible ways to fix this are to print errors as they arrive, but since jscs is given a file directory, it must wait until all promises resulting from that directory are completed before processing any errors.

markelog commented 8 years ago

Thank you for the clear example, we will investigate.

Right now you can workaround this either with max-old-space-size node option or processing files sequentially.

If you come with the fix, that would be cool too :)

hzoo commented 8 years ago

We're starting 3.0 so I moved 3.x to be master (and we're skipping a bunch of tests atm since we need to fix those). Sounds like a refactor would be needed to fix, so hopefully this can be addressed in 3.0 anyway.

Maybe @markelog, @mdevils has a better idea

kirbysayshi commented 8 years ago

@hzoo Ah, thanks for the explanation about master!

Using an increased max-old-space-size works (previously I'd been using it wrongly). For anyone else looking: node --max-old-space-size=4096 node_modules/.bin/jscs .

@markelog Processing files sequentially is possible, but tricky since the file filtering needs to happen outside of jscs. Seems like if a file is specified on the command line, then it's not checked against the contents of .jscsrc excludeFiles. Regarding "coming with a fix": I can't make any guarantees, but if I were to fix this should I do it against 2.9.0 or master?

hzoo commented 8 years ago

Probably 2.9.0 since I'm not sure when 3.0 will be out (and we can apply the same idea to 3.0)?

markelog commented 8 years ago

At this point only major and CST related bugs will be fixed.