Skip to content

Commit a1dad91

Browse files
weswighamweswigham
authored
Parallel linting (#10313)
* A perilous thing, a parallel lint * Use work queue rather than scheduling work * Dont read files for lint on main thread * Fix style
1 parent a1eda3c commit a1dad91

File tree

3 files changed

+162
-123
lines changed

3 files changed

+162
-123
lines changed

‎Gulpfile.ts

Lines changed: 60 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ import through2 = require("through2");
3434
import merge2 = require("merge2");
3535
import intoStream = require("into-stream");
3636
import * as os from "os";
37-
import Linter = require("tslint");
3837
import fold = require("travis-fold");
3938
const gulp = helpMaker(originalGulp);
4039
const mochaParallel = require("./scripts/mocha-parallel.js");
@@ -929,26 +928,6 @@ gulp.task("build-rules", "Compiles tslint rules to js", () => {
929928
.pipe(gulp.dest(dest));
930929
});
931930

932-
function getLinterOptions() {
933-
return {
934-
configuration: require("./tslint.json"),
935-
formatter: "prose",
936-
formattersDirectory: undefined,
937-
rulesDirectory: "built/local/tslint"
938-
};
939-
}
940-
941-
function lintFileContents(options, path, contents) {
942-
const ll = new Linter(path, contents, options);
943-
console.log("Linting '" + path + "'.");
944-
return ll.lint();
945-
}
946-
947-
function lintFile(options, path) {
948-
const contents = fs.readFileSync(path, "utf8");
949-
return lintFileContents(options, path, contents);
950-
}
951-
952931
const lintTargets = [
953932
"Gulpfile.ts",
954933
"src/compiler/**/*.ts",
@@ -960,29 +939,72 @@ const lintTargets = [
960939
"tests/*.ts", "tests/webhost/*.ts" // Note: does *not* descend recursively
961940
];
962941

942+
function sendNextFile(files: {path: string}[], child: cp.ChildProcess, callback: (failures: number) => void, failures: number) {
943+
const file = files.pop();
944+
if (file) {
945+
console.log(`Linting '${file.path}'.`);
946+
child.send({ kind: "file", name: file.path });
947+
}
948+
else {
949+
child.send({ kind: "close" });
950+
callback(failures);
951+
}
952+
}
953+
954+
function spawnLintWorker(files: {path: string}[], callback: (failures: number) => void) {
955+
const child = cp.fork("./scripts/parallel-lint");
956+
let failures = 0;
957+
child.on("message", function(data) {
958+
switch (data.kind) {
959+
case "result":
960+
if (data.failures > 0) {
961+
failures += data.failures;
962+
console.log(data.output);
963+
}
964+
sendNextFile(files, child, callback, failures);
965+
break;
966+
case "error":
967+
console.error(data.error);
968+
failures++;
969+
sendNextFile(files, child, callback, failures);
970+
break;
971+
}
972+
});
973+
sendNextFile(files, child, callback, failures);
974+
}
963975

964976
gulp.task("lint", "Runs tslint on the compiler sources. Optional arguments are: --f[iles]=regex", ["build-rules"], () => {
965977
const fileMatcher = RegExp(cmdLineOptions["files"]);
966-
const lintOptions = getLinterOptions();
967-
let failed = 0;
968978
if (fold.isTravis()) console.log(fold.start("lint"));
969-
return gulp.src(lintTargets)
970-
.pipe(insert.transform((contents, file) => {
971-
if (!fileMatcher.test(file.path)) return contents;
972-
const result = lintFile(lintOptions, file.path);
973-
if (result.failureCount > 0) {
974-
console.log(result.output);
975-
failed += result.failureCount;
979+
980+
const files: {stat: fs.Stats, path: string}[] = [];
981+
return gulp.src(lintTargets, { read: false })
982+
.pipe(through2.obj((chunk, enc, cb) => {
983+
files.push(chunk);
984+
cb();
985+
}, (cb) => {
986+
files.filter(file => fileMatcher.test(file.path)).sort((filea, fileb) => filea.stat.size - fileb.stat.size);
987+
const workerCount = (process.env.workerCount && +process.env.workerCount) || os.cpus().length;
988+
for (let i = 0; i < workerCount; i++) {
989+
spawnLintWorker(files, finished);
976990
}
977-
return contents; // TODO (weswig): Automatically apply fixes? :3
978-
}))
979-
.on("end", () => {
980-
if (fold.isTravis()) console.log(fold.end("lint"));
981-
if (failed > 0) {
982-
console.error("Linter errors.");
983-
process.exit(1);
991+
992+
let completed = 0;
993+
let failures = 0;
994+
function finished(fails) {
995+
completed++;
996+
failures += fails;
997+
if (completed === workerCount) {
998+
if (fold.isTravis()) console.log(fold.end("lint"));
999+
if (failures > 0) {
1000+
throw new Error(`Linter errors: ${failures}`);
1001+
}
1002+
else {
1003+
cb();
1004+
}
1005+
}
9841006
}
985-
});
1007+
}));
9861008
});
9871009

9881010

‎Jakefile.js

Lines changed: 57 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ var fs = require("fs");
44
var os = require("os");
55
var path = require("path");
66
var child_process = require("child_process");
7-
var Linter = require("tslint");
87
var fold = require("travis-fold");
98
var runTestsInParallel = require("./scripts/mocha-parallel").runTestsInParallel;
109

@@ -1054,36 +1053,6 @@ task("build-rules-end", [] , function() {
10541053
if (fold.isTravis()) console.log(fold.end("build-rules"));
10551054
});
10561055

1057-
function getLinterOptions() {
1058-
return {
1059-
configuration: require("./tslint.json"),
1060-
formatter: "prose",
1061-
formattersDirectory: undefined,
1062-
rulesDirectory: "built/local/tslint"
1063-
};
1064-
}
1065-
1066-
function lintFileContents(options, path, contents) {
1067-
var ll = new Linter(path, contents, options);
1068-
console.log("Linting '" + path + "'.");
1069-
return ll.lint();
1070-
}
1071-
1072-
function lintFile(options, path) {
1073-
var contents = fs.readFileSync(path, "utf8");
1074-
return lintFileContents(options, path, contents);
1075-
}
1076-
1077-
function lintFileAsync(options, path, cb) {
1078-
fs.readFile(path, "utf8", function(err, contents) {
1079-
if (err) {
1080-
return cb(err);
1081-
}
1082-
var result = lintFileContents(options, path, contents);
1083-
cb(undefined, result);
1084-
});
1085-
}
1086-
10871056
var lintTargets = compilerSources
10881057
.concat(harnessSources)
10891058
// Other harness sources
@@ -1094,75 +1063,78 @@ var lintTargets = compilerSources
10941063
.concat(["Gulpfile.ts"])
10951064
.concat([nodeServerInFile, perftscPath, "tests/perfsys.ts", webhostPath]);
10961065

1066+
function sendNextFile(files, child, callback, failures) {
1067+
var file = files.pop();
1068+
if (file) {
1069+
console.log("Linting '" + file + "'.");
1070+
child.send({kind: "file", name: file});
1071+
}
1072+
else {
1073+
child.send({kind: "close"});
1074+
callback(failures);
1075+
}
1076+
}
1077+
1078+
function spawnLintWorker(files, callback) {
1079+
var child = child_process.fork("./scripts/parallel-lint");
1080+
var failures = 0;
1081+
child.on("message", function(data) {
1082+
switch (data.kind) {
1083+
case "result":
1084+
if (data.failures > 0) {
1085+
failures += data.failures;
1086+
console.log(data.output);
1087+
}
1088+
sendNextFile(files, child, callback, failures);
1089+
break;
1090+
case "error":
1091+
console.error(data.error);
1092+
failures++;
1093+
sendNextFile(files, child, callback, failures);
1094+
break;
1095+
}
1096+
});
1097+
sendNextFile(files, child, callback, failures);
1098+
}
10971099

10981100
desc("Runs tslint on the compiler sources. Optional arguments are: f[iles]=regex");
10991101
task("lint", ["build-rules"], function() {
11001102
if (fold.isTravis()) console.log(fold.start("lint"));
11011103
var startTime = mark();
1102-
var lintOptions = getLinterOptions();
11031104
var failed = 0;
11041105
var fileMatcher = RegExp(process.env.f || process.env.file || process.env.files || "");
11051106
var done = {};
11061107
for (var i in lintTargets) {
11071108
var target = lintTargets[i];
11081109
if (!done[target] && fileMatcher.test(target)) {
1109-
var result = lintFile(lintOptions, target);
1110-
if (result.failureCount > 0) {
1111-
console.log(result.output);
1112-
failed += result.failureCount;
1113-
}
1114-
done[target] = true;
1110+
done[target] = fs.statSync(target).size;
11151111
}
11161112
}
1117-
measure(startTime);
1118-
if (fold.isTravis()) console.log(fold.end("lint"));
1119-
if (failed > 0) {
1120-
fail('Linter errors.', failed);
1121-
}
1122-
});
11231113

1124-
/**
1125-
* This is required because file watches on Windows get fires _twice_
1126-
* when a file changes on some node/windows version configuations
1127-
* (node v4 and win 10, for example). By not running a lint for a file
1128-
* which already has a pending lint, we avoid duplicating our work.
1129-
* (And avoid printing duplicate results!)
1130-
*/
1131-
var lintSemaphores = {};
1132-
1133-
function lintWatchFile(filename) {
1134-
fs.watch(filename, {persistent: true}, function(event) {
1135-
if (event !== "change") {
1136-
return;
1137-
}
1114+
var workerCount = (process.env.workerCount && +process.env.workerCount) || os.cpus().length;
11381115

1139-
if (!lintSemaphores[filename]) {
1140-
lintSemaphores[filename] = true;
1141-
lintFileAsync(getLinterOptions(), filename, function(err, result) {
1142-
delete lintSemaphores[filename];
1143-
if (err) {
1144-
console.log(err);
1145-
return;
1146-
}
1147-
if (result.failureCount > 0) {
1148-
console.log("***Lint failure***");
1149-
for (var i = 0; i < result.failures.length; i++) {
1150-
var failure = result.failures[i];
1151-
var start = failure.startPosition.lineAndCharacter;
1152-
var end = failure.endPosition.lineAndCharacter;
1153-
console.log("warning " + filename + " (" + (start.line + 1) + "," + (start.character + 1) + "," + (end.line + 1) + "," + (end.character + 1) + "): " + failure.failure);
1154-
}
1155-
console.log("*** Total " + result.failureCount + " failures.");
1156-
}
1157-
});
1158-
}
1116+
var names = Object.keys(done).sort(function(namea, nameb) {
1117+
return done[namea] - done[nameb];
11591118
});
1160-
}
11611119

1162-
desc("Watches files for changes to rerun a lint pass");
1163-
task("lint-server", ["build-rules"], function() {
1164-
console.log("Watching ./src for changes to linted files");
1165-
for (var i = 0; i < lintTargets.length; i++) {
1166-
lintWatchFile(lintTargets[i]);
1120+
for (var i = 0; i < workerCount; i++) {
1121+
spawnLintWorker(names, finished);
11671122
}
1168-
});
1123+
1124+
var completed = 0;
1125+
var failures = 0;
1126+
function finished(fails) {
1127+
completed++;
1128+
failures += fails;
1129+
if (completed === workerCount) {
1130+
measure(startTime);
1131+
if (fold.isTravis()) console.log(fold.end("lint"));
1132+
if (failures > 0) {
1133+
fail('Linter errors.', failed);
1134+
}
1135+
else {
1136+
complete();
1137+
}
1138+
}
1139+
}
1140+
}, {async: true});

‎scripts/parallel-lint.js

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
var Linter = require("tslint");
2+
var fs = require("fs");
3+
4+
function getLinterOptions() {
5+
return {
6+
configuration: require("../tslint.json"),
7+
formatter: "prose",
8+
formattersDirectory: undefined,
9+
rulesDirectory: "built/local/tslint"
10+
};
11+
}
12+
13+
function lintFileContents(options, path, contents) {
14+
var ll = new Linter(path, contents, options);
15+
return ll.lint();
16+
}
17+
18+
function lintFileAsync(options, path, cb) {
19+
fs.readFile(path, "utf8", function (err, contents) {
20+
if (err) {
21+
return cb(err);
22+
}
23+
var result = lintFileContents(options, path, contents);
24+
cb(undefined, result);
25+
});
26+
}
27+
28+
process.on("message", function (data) {
29+
switch (data.kind) {
30+
case "file":
31+
var target = data.name;
32+
var lintOptions = getLinterOptions();
33+
lintFileAsync(lintOptions, target, function (err, result) {
34+
if (err) {
35+
process.send({ kind: "error", error: err.toString() });
36+
return;
37+
}
38+
process.send({ kind: "result", failures: result.failureCount, output: result.output });
39+
});
40+
break;
41+
case "close":
42+
process.exit(0);
43+
break;
44+
}
45+
});

0 commit comments

Comments
 (0)