Enable caching for ESLint

[Trott]

Checklist

• make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
• commit message follows commit guidelines

Affected core subsystem(s)

tools

Description of change

This PR is in 3 separate commits:

• Update ESLint to 3.4.0 (which includes a bugfix for the ESLint cache that we strictly don't need but are probably better off having than not having)
• Adding the .eslintcache file to .gitignore
• Enabling caching (and disabling the parallelizing script) on the non-CI lint task in Makefile and vcbuild.bat (Oh, hey, can someone on Windows grab this branch and test the vcbuild.bat change to the jslint task?)

Here's my crude benchmarks:

• Running make jslint the first time (no cache) with this change: 11 seconds or so
• Running make jslint on current master: 8 seconds or so
• Running make jslint subsequent times (populated cache) with this change: 1 second

Here's my thinking:

make jslint is something that gets done again and again in a dev environment. It's worth an extra 3 seconds or so on the first run if it shaves 7 seconds or so on subsequent runs.

However, in CI, it just gets run once. So the current parallelizing script is the faster option on CI.

It is possible to enable caching in the parallelizing script, but I'm not sure it's worth the complexity. That can be a subsequent enhancement if it is highly desirable. It's possible (I think) that it will not be measurably better than one job with one cache. Would have to be measured, of course.

/cc @mscdex

[mscdex]

How does the cache work? Especially, how does it detect changes to files? Does it just compare mtime?

[Trott]

mtime and size of the file

It also stores the hash of the config file for the last time that particular file was linted. That means there is a pitfall if you change your .eslintrc file, you've invalidated your entire cache and it will have to be recreated on the next run. But I suspect that those of us who tweak the .eslintrc file with any frequency are a tiny minority of contributors...

[Trott]

If you want more info on how it determines if a file has changed:

• It is using this module
• Specifically, it is calling getFileDescriptor() and then checking the changed property which is set if the size or mtime have changed

[Trott]

The main challenge with using caching in the parallelized script is that each worker would need its own cache file (cacheLocation property in the config object). Obviously possible, and really easy if (for example) there is a one-to-one relationship between directory and worker. (That is, if one worker gets everything in test and another gets everything in lib, then put the cache files in test/.eslintcache and lib/.eslintcache respectively.)

[cjihrig]

I'd squash the last two commits together, but LGTM. Speeding up the linter is 😍

[silverwind]

Will the cache be invalidated when a new ESLint version is used?

[jasnell]

Rubber stamp LGTM.

[Trott]

Will the cache be invalidated when a new ESLint version is used?

Yes.

[Trott]

CI: https://ci.nodejs.org/job/node-test-pull-request/3859/

[benjamingr]

Looks fine in windows, LGTM

[Trott]

(CI is green)

[silverwind]

LGTM, maybe you can squash the second and third commit as mentioned.

[Trott]

Landed in 2011158 and 5a7a6d9

[MylesBorins]

@Trott do you want to backport these linter changes?

[Trott]

@thealphanerd It looks like we've skipped a number of ESLint updates on v4.x at this point. My inclination would be to not start now, if there's a lot of annoyance involved and not much benefit. Fine by you if we mark this as dont-land and revisit if it becomes necessary? (We can always do a separate update to whatever ESLint version just for v4.x if we want to.)