Skip to content

Commit

Permalink
Performance improvements
Browse files Browse the repository at this point in the history
This commit improves performance by allowing to reduce the amount of data
that is included in instrumented files and the amount of data sent to the
node-coverage server with the submit method.

There are two kinds of data:
- "staticInfo": data about the file itself (list of statement ids,
condition ids, function ids, highlighted code)
- "run": info about the code that was actually run. This includes the
counters for each statement, condition or function that were actually
executed at least once.

There is normally no need to include the "staticInfo" part in the instrumented
files and have that info be sent to the browser and back to the server.
It is only needed on the server, along with the "run" part to compute and
display coverage results on the server.

This commit clearly separates those two kinds of information and allows to
create a JSON file containing the static part which will allow not to store
it in instrumented files themselves, thus reducing the size of instrumented
files and improving performance when calling submit.

This commit also removes the stack of all function calls that was built
when function coverage was enabled, as it is usually very big, takes a lot
of time to be sent to the server, and is not very useful. Instead, there
is now a counter for each function in the same way as it is done for
statement coverage.
  • Loading branch information
divdavem committed Oct 25, 2018
1 parent 1d9f650 commit 5466b96
Show file tree
Hide file tree
Showing 19 changed files with 3,927 additions and 232 deletions.
5 changes: 3 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
language: node_js
node_js:
- 0.6
- 0.8
- 6.9.1
before_install:
- npm install -g [email protected]
10 changes: 7 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ It's also possible to specify a report name from the `submit` function
* `-a` or `--admin-port` administrative server port. Default `8787`
* `--condition`, `--no-condition` Enable or disable condition coverage. By default it's enabled.
* `--function`, `--no-function` Enable or disable function coverage. By default it's disabled.
* `--session`, `--no-session` Enable or disable session storage for information not strictly needed by the browser. By default it's enabled. Disabling this means that more code is sent to and from the client.
* `--static-info` In case files are pre-instrumented, path to the JSON file containing static information about instrumented files.
* `--session`, `--no-session` Enable or disable storage of information not strictly needed by the browser. By default it's enabled. Disabling this means that more code is sent to and from the client.
* `-i` or `--ignore` Ignore file or folder. This file/folder won't be instrumented. Path is relative to document root.
* `--proxy` Proxy mode. You can use node-coverage to instrument files on a differnt host.
* `--exit-on-submit` The default behavior is to keep the server running in order to collect multiple reports. By enabling this options the server will automatically shut down when a coverage report is received. This is useful for some continous integration environment. If you want to collect more coverage reports but still be able to shut down the server when tests are done you can submit a request to '/node-coverage-please-exit'.
Expand Down Expand Up @@ -135,6 +136,7 @@ You can then run the server with
* `-t` ot `test` run unit tests
* `--condition`, `--no-condition` enable or disable condition coverage. By default it's enabled.
* `--function`, `--no-function` enable or disable function coverage. By default it's disabled.
* `--static-info` Path to a JSON output file which will contain static information about instrumented files. Using this option reduces the size of instrumented files.
* `-i` or `--ignore` Ignore file or folder. This file/folder is copied in target folder but not instrumented. Path relative to the source folder.
* `-x` or `--exclude` Exclude file or folder. This file/folder won't be copied in target folder. Path relative to the source folder.

Expand All @@ -148,7 +150,7 @@ or

to disable condition coverage.

The code generated offline is equal to the one generated by the server when session storage is disabled with `--no-session`.
The code generated offline is equal to the one generated by the server when storage is disabled with `--no-session`, unless `--static-info` is used.

You can also instrument a single file launching

Expand Down Expand Up @@ -320,11 +322,13 @@ Filter object specifies which files are handled by the module.
* `options` Coverage options
* `function` boolean, enable function coverage
* `condition` boolean, enable condition coverage
* `staticInfo` boolean, whether to include static information inside the instrumented code
* `submit` boolean, whether to include the submit function inside the instrumented code

this function must return an object containing

* `clientCode` the instrumented code, this is sent to the client
* `highlightedCode` an array of file lines, this matches the original file statements to the corresponding line of code.
* `staticInfo` an object describing static information about the file

## Proxy

Expand Down
53 changes: 30 additions & 23 deletions instrument.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
#!/usr/bin/env node
var fileSystem = require("./lib/fileSystem");
var fs = require("fs");
var argv = require("optimist")
.usage("Instrument a folder for code coverage.\n$0 source destination")
.boolean("h").alias("h", "help")
.boolean("t").alias("t", "test").describe("t", "Run unit tests.")
.boolean("function")
.default("function", false)
.describe("function", "Enable function coverage. Disable with --no-function")
.boolean("condition")
.default("condition", true)
.describe("condition", "Enable condition coverage. Disable with --no-condition")
.boolean("submit")
.default("submit", true)
.describe("submit", "Include submit code in instrumented file. Disable with --no-submit")
.options("s", {
"alias" : "static-info"
}).describe("s", "Path to a JSON output file which will contain static information about instrumented files. Using this option reduces the size of instrumented files.")
.options("x", {
"alias" : "exclude"
}).describe("x", "Exclude file or folder. This file/folder won't be copied in target folder. Path relative to the source folder")
Expand All @@ -21,43 +27,41 @@ var argv = require("optimist")



if (argv.t) {
runTests();
} else {
switch (argv._.length) {
case 1:
instrumentFile(argv._[0], argv);
break;
case 2:
instrumentFolder(argv._[0], argv._[1], argv);
break;
default:
displayHelp();
break;
}
switch (argv._.length) {
case 1:
instrumentFile(argv._[0], argv);
break;
case 2:
instrumentFolder(argv._[0], argv._[1], argv);
break;
default:
displayHelp();
break;
}


function runTests () {
require("nodeunit").reporters.default.run(['test']);
};

function displayHelp () {
require("optimist").showHelp();
};

function instrumentFolder (source, destination, options) {
try {
var callback = fileSystem.writeFileTo(source, destination);
var staticInfoFile = options["static-info"];
var staticInfo = staticInfoFile ? {} : null;

fileSystem.statFileOrFolder([source], "", callback, {
"function" : options["function"],
"condition" : options["condition"],
"doHighlight" : true,
"submit" : options["submit"],
"staticInfo": !staticInfo,
"exclude" : options.exclude,
"ignore" : options.ignore,
"verbose" : options.verbose
});
}, staticInfo);

if (staticInfo) {
fs.writeFileSync(staticInfoFile, JSON.stringify(staticInfo));
}
} catch (ex) {
require("optimist").showHelp();
return console.error(ex);
Expand All @@ -72,7 +76,10 @@ function instrumentFile (fileName, options) {
fileSystem.statFileOrFolder([fileName], "", callback, {
"function" : options["function"],
"condition" : options["condition"],
"doHighlight" : true,
"submit" : options["submit"],
"staticInfo": !options["static-info"],
"exclude" : options.exclude,
"ignore" : options.ignore,
"verbose" : options.verbose
});
};
110 changes: 60 additions & 50 deletions lib/clientCode.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,74 +45,89 @@ function shouldBeExcluded (location, exclude) {
* These functions generate the additional code sent to the client.
* This code contains the logic to send reports back to the server
*/
var composeFile = function (fileName, allLines, allConditions, allFunctions, code, doHighlight) {
var composeFile = function (fileName, allLines, allConditions, allFunctions, code, options) {
var highlighted = highlight(code);
return {
clientCode : [
// header comment saying that this file is instrumented
header,
// closure that creates the global objects
"(", createGlobalObjects.toString(), ")(",
// arguments for the closure call
uglify.make_string(fileName), ",", JSON.stringify({
lines : allLines,
code : doHighlight ? highlighted : {},
allConditions : allConditions,
allFunctions : allFunctions
}), ");(",
var clientCodeHeader = [
// header comment saying that this file is instrumented
header,
// closure that creates the global objects
"(", createGlobalObjects.toString(), ")(",
// argument for the closure call
uglify.make_string(fileName),
");"
];
if (options.staticInfo !== false) {
clientCodeHeader.push("(", storeStaticFileInfo.toString(), ")(", uglify.make_string(fileName), ",", JSON.stringify({
code : highlighted,
lines : allLines,
conditions : allConditions,
functions : allFunctions
}), ");");
}
if (options.submit !== false) {
clientCodeHeader.push("(",
//second closure with the logic to send the report to the server
submitData.toString(), ")();"
].join("") + "\n" + code,
highlightedCode : highlighted,
lines: allLines,
conditions: allConditions,
functions: allFunctions,
);
}

return {
clientCode : clientCodeHeader.join("") + "\n" + code,
staticInfo: {
code : highlighted,
lines : allLines,
conditions : allConditions,
functions : allFunctions
},
error : false
};
};

var createGlobalObjects = function (f, args) {
var createGlobalObjects = function (f) {
var $$_l = this.$$_l;
if (!this.$$_l) {
$$_l = function (file, line) {
$$_l.runLines[file][line] += 1;
var counter = $$_l.run[file].lines[line]++;
if (!counter > 0) {
$$_l.run[file].lines[line] = 1;
}
};
$$_c = function (file, line, condition) {
$$_l.conditions[file].push([line, !!condition]);
var counter = $$_l.run[file].conditions[line + "=" + !!condition]++;
if (!counter > 0) {
$$_l.run[file].conditions[line + "=" + !!condition] = 1;
}
return condition;
};
$$_f = function (file, name, line) {
// to have a global view, fns are not divided by file here
$$_l.runFunctions.push([file, name, line]);
$$_f = function (file, line) {
var counter = $$_l.run[file].functions[line]++;
if (!counter > 0) {
$$_l.run[file].functions[line] = 1;
}
};
$$_l.runLines = {};
$$_l.lines = {};
$$_l.code = {};
$$_l.conditions = {};
$$_l.allConditions = {};
$$_l.allFunctions = {};
$$_l.runFunctions = [];

$$_l.run = {};
this.$$_l = $$_l;
}
$$_l.lines[f] = args.lines;
$$_l.runLines[f] = {};
for (var i = 0, len = args.lines.length; i < len; i += 1) {
$$_l.runLines[f][args.lines[i]] = 0;
}
$$_l.code[f] = args.code;
$$_l.conditions[f] = [];
$$_l.allConditions[f] = args.allConditions;
$$_l.allFunctions[f] = args.allFunctions;
$$_l.run[f] = {
lines: {},
conditions: {},
functions: {}
};
};

var storeStaticFileInfo = function (f, args) {
if (!this.$$_l.staticInfo) {
$$_l.staticInfo = {};
}
$$_l.staticInfo[f] = args;
};

// The serialize in this function is much simplified, values are string/object/array/boolean/Number
var submitData = function () {
var serialize = function (object) {
var properties = [];
for (var key in object) {
if (object.hasOwnProperty(key)) {
if (object.hasOwnProperty(key) && object[key] != null) {
properties.push('"' + key.replace('\\', '\\\\') + '":' + getValue(object[key]));
}
}
Expand Down Expand Up @@ -170,13 +185,8 @@ var submitData = function () {
$$_l.submit = function (name) {
$$_l.__send(serialize({
name : name || "",
lines : $$_l.lines,
runLines : $$_l.runLines,
code : $$_l.code,
allConditions : $$_l.allConditions,
conditions : $$_l.conditions,
allFunctions : $$_l.allFunctions,
runFunctions : $$_l.runFunctions
staticInfo : $$_l.staticInfo,
run : $$_l.run
}));
};
}
Expand Down
22 changes: 13 additions & 9 deletions lib/fileSystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ subModules.forEach(function (moduleName) {
});

/* These functions are sync to avoid too many opened files */
function statFileOrFolder (fileOrFolder, basePath, callback, options) {
function statFileOrFolder (fileOrFolder, basePath, callback, options, staticInfo) {
var toBeExluded = options ? options.exclude : null;

fileOrFolder.forEach(function (item) {
Expand All @@ -26,24 +26,28 @@ function statFileOrFolder (fileOrFolder, basePath, callback, options) {

if (!clientCode.shouldBeExcluded(osIndependentFileName(fileName), toBeExluded)) {
if (stat.isFile()) {
instrumentFile(item, basePath, callback, options);
instrumentFile(item, basePath, callback, options, staticInfo);
} else if (stat.isDirectory()) {
instrumentFolder(item, basePath, callback, options);
instrumentFolder(item, basePath, callback, options, staticInfo);
} else {
console.error("Unable to instrument, niether a file nor a folder.", item, stats);
console.error("Unable to instrument, neither a file nor a folder.", item, stats);
}
}
});
}

function instrumentFile (file, basePath, callback, options) {
function instrumentFile (file, basePath, callback, options, staticInfo) {
var fileName = path.join(basePath, file);
var osIndependentName = osIndependentFileName(fileName);

var encoding = "utf-8";
var content = fs.readFileSync(fileName, encoding);

var instructed = instrument(osIndependentName, content, options).clientCode;
var instrumentationResult = instrument(osIndependentName, content, options);
if (staticInfo && instrumentationResult.staticInfo) {
staticInfo[osIndependentName] = instrumentationResult.staticInfo;
}
var instructed = instrumentationResult.clientCode;
if (instructed === content) {
instructed = fs.readFileSync(fileName);
encoding = null;
Expand All @@ -54,16 +58,16 @@ function instrumentFile (file, basePath, callback, options) {
}
}

function instrumentFolder (folder, basePath, callback, options) {
function instrumentFolder (folder, basePath, callback, options, staticInfo) {
var folderPath = path.join(basePath, folder);
var files = fs.readdirSync(folderPath);

statFileOrFolder(files, folderPath, callback, options);
statFileOrFolder(files, folderPath, callback, options, staticInfo);
}

function writeFileTo (src, dest) {
var destinationRoot = path.resolve(dest);
if (path.existsSync(destinationRoot)) {
if (fs.existsSync(destinationRoot)) {
throw new Error(destinationRoot + " exists already");
}

Expand Down
4 changes: 2 additions & 2 deletions lib/highlight.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Looking for $$_l("filename", "lineId");
var regStatement = /\$\$_l\("[^"]+",\s?"([^"]+)"\);/g;
var regCondition = /\$\$_c\("[^"]+",\s?"([^"]+)",\s?/g;
var regFunction = /\$\$_f\("[^"]+",\s?"([^"]+)",\s?"([^"]+)"\);/g;
var regFunction = /\$\$_f\("[^"]+",\s?"([^"]+)"\);/g;

/**
* This function returns a cleaned representation of the generated code.
Expand Down Expand Up @@ -29,7 +29,7 @@ exports.highlight = function (code) {
var fnMatch = regFunction.exec(line);
if (fnMatch) {
// The previous line was a function
result.fns[fnMatch[2]] = mapped.length - 1;
result.fns[fnMatch[1]] = mapped.length - 1;
} else {
// Code, not necessarly mapped to statement
var tags = [], generatedLine = {};
Expand Down
Loading

0 comments on commit 5466b96

Please sign in to comment.