Performance: Stream files vs. all-at-once linting

[TomSeldon]

trafficstars

Currently, when parsing multiple files they are each piped to the Closure Linter one at a time. This was done as it seems to be the "Gulp way" of doing things- but it has some significant potential performance problems. Namely, the overhead of having to run the Closure Linter O(n) times (each parsing a single file) vs. running it once, with all files parsed by a single Closure Linter process.

With no metrics yet available to properly assess this, a subjective observation is that the time taken to start the Closure Linter process is longer than the time it takes to parse a single file and so it seems to be a large bottleneck.

To do:

• [ ] Collect representative performance stats for current implementation.
• [ ] Implement an 'all in one' approach (separate branch).
• [ ] Collect representative performance stats for 'all in one' implementation.
• [ ] Compare results and decide whether performance difference (if any) warrants departing from Gulp standard practice (i.e. streaming files one at a time).

[TomSeldonBoxUK]

Note to add... if parsing all files as once, may be pointless even using this plugin. Can use closure-linter-wrapper directly.

[learntoswim]

I'm attempting to lint a rather large codebase, I've noticed that gulp-gjslint cannot handle the amount of JS files (around 350). And just putters out at around 9 or so. Could this be related?

gulp.task('lint', function() {
  var sourceList = ['./**/*.js']
  _gjsLint(sourceList);
});

function _gjsLint(sourceList) {
  var gjslint = require('gulp-gjslint');
  var options = {
    flags: [
      '--strict'
    ]
  };

  // Lint files and output results to the console
  return gulp.src(sourceList)
    .pipe(gjslint(options))
    .pipe(gjslint.reporter('console'));

}

[TomSeldon]

Hi @karlstanton,

I'll take a look at this, but the short answer is it shouldn't be a problem. I've been using with it with a similarly large project and it works. It's slow, but it works.

This could be an issue with gulp-gjslint, but looking at the example in your comment it looks like you're not returning the stream from your task definition. You're returning it from _gjsLint but in your task definition (line 3) you don't return anything.

It looks like you should have:

gulp.task('lint', function() {
  var sourceList = ['./**/*.js'];
  return _gjsLint(sourceList);
});

That would explain why you're only seeing the output from a small number of files, as Gulp just thinks your task has finished and so stops execution (despite gjslint still running at the time).

Can you try that and let me know if you still have any issues?

[learntoswim]

Oh! Much better! Thanking you.

[rhoggSugarcrm]

Pardon me, but do you have an approximate timeline for when the comparisons will be complete? From my initial tests, it looks like gulp-gjslint is a significantly bigger bottleneck on our codebase than gulp-jshint is.

[TomSeldon]

Hi @rhoggSugarcrm,

I haven't really looked at the performance since I opened this issue, but I'll try and take a look this weekend.

My main concern would be that doing things the "Gulp way" and piping files through one at a time doesn't really seem to fit with how gjslint is expecting to run (i.e. get a list of files and lint them all in one go). There seems to be a lot of overhead in starting up a gjslint process, and so doing this for every file rather than one process that lints multiple files means you end up with a very slow linting task. Presumably jshint doesn't suffer as much from this type of overhead.

It should be a lot faster to run gjslint with all files at the same time, and that's what I'll look to do, but it's been so long since I've looked at this it may not be possible.

I'll update you once I've taken a look :)

Tom