[gjs/wip/ptomato/develop: 17/18] coverage: Use Map objects for function counters
- From: Philip Chimento <pchimento src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gjs/wip/ptomato/develop: 17/18] coverage: Use Map objects for function counters
- Date: Wed, 27 Sep 2017 04:42:23 +0000 (UTC)
commit 94159f3248b6a8ee7534c6fa9b48c324644fed82
Author: Philip Chimento <philip chimento gmail com>
Date: Sun Sep 24 23:25:30 2017 -0700
coverage: Use Map objects for function counters
We can hopefully make the function counters more efficient by using Map
objects instead of plain objects, and since we now always have accurate
info about the line where a function is defined, we can combine the
function's name and line into one key, thereby only maintaining two
levels of Map instead of three.
This also refers to a function map key (name:line:nArgs) as a 'key'
everywhere, and not a 'name'.
https://bugzilla.gnome.org/show_bug.cgi?id=788166
gjs/coverage.cpp | 6 ++-
installed-tests/js/testCoverage.js | 54 +++++++++++------------
modules/_bootstrap/coverage.js | 85 +++++++++++++++---------------------
test/gjs-test-coverage.cpp | 2 +-
4 files changed, 66 insertions(+), 81 deletions(-)
---
diff --git a/gjs/coverage.cpp b/gjs/coverage.cpp
index d79fad3..c0de42b 100644
--- a/gjs/coverage.cpp
+++ b/gjs/coverage.cpp
@@ -548,8 +548,9 @@ convert_and_insert_function_decl(GArray *array,
JS::RootedObject object(context, &element.toObject());
JS::RootedValue function_name_property_value(context);
- if (!gjs_object_require_property(context, object, NULL, GJS_STRING_NAME,
- &function_name_property_value))
+ JS::RootedId key_name(context, gjs_intern_string_to_id(context, "key"));
+ if (!gjs_object_require_property(context, object, "function element",
+ key_name, &function_name_property_value))
return false;
GjsAutoJSChar utf8_string(context);
@@ -991,6 +992,7 @@ gjs_get_file_checksum(GFile *file)
* } branch_info ] branches;
* array [ tuple {
* int line;
+ * int n_params;
* string key;
* } function ] functions;
* } file ] files;
diff --git a/installed-tests/js/testCoverage.js b/installed-tests/js/testCoverage.js
index e1def31..e66319e 100644
--- a/installed-tests/js/testCoverage.js
+++ b/installed-tests/js/testCoverage.js
@@ -854,7 +854,7 @@ describe('Coverage', function () {
let detectedFunctions = Coverage.functionsForAST(ast);
let functionCounters =
Coverage._functionsToFunctionCounters('script', detectedFunctions);
- expect(functionCounters.name['1']['0'].hitCount).toEqual(0);
+ expect(functionCounters.get('name:1').get(0).hitCount).toEqual(0);
});
it('reports an error when two indistinguishable functions are present', function () {
@@ -869,26 +869,24 @@ describe('Coverage', function () {
});
it('converts function counters to an array', function () {
- let functionsMap = {
- '(anonymous)': {
- '2': {
- '0': {
- hitCount: 1
- },
- },
- },
- 'name': {
- '1': {
- '0': {
- hitCount: 0
- },
- },
- }
- };
+ let functionsMap = new Map([
+ ['(anonymous):2', new Map([[0, {hitCount: 1}]])],
+ ['name:1', new Map([[0, {hitCount: 0}]])],
+ ]);
let expectedFunctionCountersArray = [
- jasmine.objectContaining({ name: '(anonymous):2:0', hitCount: 1 }),
- jasmine.objectContaining({ name: 'name:1:0', hitCount: 0 })
+ {
+ key: '(anonymous):2:0',
+ line: 2,
+ nArgs: 0,
+ hitCount: 1,
+ },
+ {
+ key: 'name:1:0',
+ line: 1,
+ nArgs: 0,
+ hitCount: 0,
+ },
];
let convertedFunctionCounters = Coverage._convertFunctionCountersToArray(functionsMap);
@@ -906,7 +904,7 @@ describe('Coverage.incrementFunctionCounters', function () {
]);
Coverage._incrementFunctionCounters(functionCounters, 'f', 1, 0);
- expect(functionCounters.f['1']['0'].hitCount).toEqual(1);
+ expect(functionCounters.get('f:1').get(0).hitCount).toEqual(1);
});
it('can disambiguate two functions with the same name', function () {
@@ -921,8 +919,8 @@ describe('Coverage.incrementFunctionCounters', function () {
Coverage._incrementFunctionCounters(functionCounters, '(anonymous)', 1, 0);
Coverage._incrementFunctionCounters(functionCounters, '(anonymous)', 2, 0);
- expect(functionCounters['(anonymous)']['1']['0'].hitCount).toEqual(1);
- expect(functionCounters['(anonymous)']['2']['0'].hitCount).toEqual(1);
+ expect(functionCounters.get('(anonymous):1').get(0).hitCount).toEqual(1);
+ expect(functionCounters.get('(anonymous):2').get(0).hitCount).toEqual(1);
});
it('can disambiguate two functions on same line with different params', function () {
@@ -937,8 +935,8 @@ describe('Coverage.incrementFunctionCounters', function () {
Coverage._incrementFunctionCounters(functionCounters, '(anonymous)', 1, 0);
Coverage._incrementFunctionCounters(functionCounters, '(anonymous)', 1, 1);
- expect(functionCounters['(anonymous)']['1']['0'].hitCount).toEqual(1);
- expect(functionCounters['(anonymous)']['1']['1'].hitCount).toEqual(1);
+ expect(functionCounters.get('(anonymous):1').get(0).hitCount).toEqual(1);
+ expect(functionCounters.get('(anonymous):1').get(1).hitCount).toEqual(1);
});
it('can disambiguate two functions on same line by guessing closest params', function () {
@@ -956,8 +954,8 @@ describe('Coverage.incrementFunctionCounters', function () {
* latter function as opposed to the former. */
Coverage._incrementFunctionCounters(functionCounters, '(anonymous)', 1, 2);
- expect(functionCounters['(anonymous)']['1']['0'].hitCount).toEqual(0);
- expect(functionCounters['(anonymous)']['1']['3'].hitCount).toEqual(1);
+ expect(functionCounters.get('(anonymous):1').get(0).hitCount).toEqual(0);
+ expect(functionCounters.get('(anonymous):1').get(3).hitCount).toEqual(1);
});
it('throws an error on unexpected function', function () {
@@ -1143,8 +1141,8 @@ describe('Coverage statistics container', function () {
it('have same function keys as reflection', function () {
/* Functions start on line 1 */
- expect(Object.keys(statisticsWithNoCaching.functionCounters))
- .toEqual(Object.keys(statistics.functionCounters));
+ expect([...statisticsWithNoCaching.functionCounters.keys()])
+ .toEqual([...statistics.functionCounters.keys()]);
});
});
});
diff --git a/modules/_bootstrap/coverage.js b/modules/_bootstrap/coverage.js
index e83e25c..0c2ec6c 100644
--- a/modules/_bootstrap/coverage.js
+++ b/modules/_bootstrap/coverage.js
@@ -499,21 +499,23 @@ function _branchesToBranchCounters(branches, nLines) {
}
function _functionsToFunctionCounters(script, functions) {
- let functionCounters = {};
+ let functionCounters = new Map();
functions.forEach(function(func) {
- let [name, line, args] = func.key.split(':');
+ let [name] = func.key.split(':', 1);
+ let {line, n_params} = func;
- if (functionCounters[name] === undefined) {
- functionCounters[name] = {};
- }
-
- if (functionCounters[name][line] === undefined) {
- functionCounters[name][line] = {};
+ let argsMap;
+ let key = `${name}:${line}`;
+ if (functionCounters.has(key)) {
+ argsMap = functionCounters.get(key);
+ } else {
+ argsMap = new Map();
+ functionCounters.set(key, argsMap);
}
- if (functionCounters[name][line][args] === undefined) {
- functionCounters[name][line][args] = { hitCount: 0 };
+ if (!argsMap.has(n_params)) {
+ argsMap.set(n_params, {hitCount: 0});
} else {
log(script + ':' + line + ' Function identified as ' +
func.key + ' already seen in this file. Function coverage ' +
@@ -524,24 +526,20 @@ function _functionsToFunctionCounters(script, functions) {
return functionCounters;
}
-function _identifyFunctionCounterInLinePartForDescription(linePart,
- nArgs) {
+function _identifyFunctionCounterInArgsMap(argsMap, nArgs) {
/* There is only one potential option for this line. We might have been
* called with the wrong number of arguments, but it doesn't matter. */
- if (Object.getOwnPropertyNames(linePart).length === 1)
- return linePart[Object.getOwnPropertyNames(linePart)[0]];
+ if (argsMap.size === 1)
+ return argsMap.values().next().value;
/* Have to disambiguate using nArgs and we have an exact match. */
- if (linePart[nArgs] !== undefined)
- return linePart[nArgs];
+ if (argsMap.has(nArgs))
+ return argsMap.get(nArgs);
/* There are multiple options on this line. Pick the one where
* we satisfy the number of arguments exactly, or failing that,
* go through each and pick the closest. */
- let allNArgsOptions = Object.keys(linePart).map(function(key) {
- return parseInt(key);
- });
-
+ let allNArgsOptions = [...argsMap.keys()];
let closest = allNArgsOptions.reduce(function(prev, current, index, array) {
let nArgsOption = array[index];
if (Math.abs(nArgsOption - nArgs) < Math.abs(current - nArgs)) {
@@ -551,26 +549,24 @@ function _identifyFunctionCounterInLinePartForDescription(linePart,
return current;
});
- return linePart[String(closest)];
+ return argsMap.get(closest);
}
/**
* _incrementFunctionCounters
*
- * functionCounters: An object which is a key-value pair with the following schema:
- * {
- * "key" : { line, hitCount }
- * }
+ * functionCounters: a Map object returned from _functionsToFunctionCounters()
* name: The name of the function or "(anonymous)" if it has no name
* line: The line at which this function is defined.
* nArgs: The number of arguments this function has.
*/
function _incrementFunctionCounters(functionCounters, name, line, nArgs) {
let functionCountersForKey;
- let linePart = functionCounters[name][line];
- if (linePart) {
+ let key = `${name}:${line}`;
+ if (functionCounters.has(key)) {
+ let argsMap = functionCounters.get(key);
functionCountersForKey =
- _identifyFunctionCounterInLinePartForDescription(linePart, nArgs);
+ _identifyFunctionCounterInArgsMap(argsMap, nArgs);
}
if (functionCountersForKey) {
@@ -659,30 +655,23 @@ var _BranchTracker = class {
function _convertFunctionCountersToArray(functionCounters) {
let arrayReturn = [];
- /* functionCounters is an object so explore it to create a
+ /* functionCounters is a map so iterate through it to create a
* set of function keys and then convert it to
* an array-of-object using the key as a property
* of that object */
- for (let name of Object.getOwnPropertyNames(functionCounters)) {
- let namePart = functionCounters[name];
-
- for (let line of Object.getOwnPropertyNames(namePart)) {
- let linePart = functionCounters[name][line];
-
- for (let nArgs of Object.getOwnPropertyNames(linePart)) {
- let functionKey = [name, line, nArgs].join(':');
- arrayReturn.push({ name: functionKey,
- line: Number(line),
- nArgs: nArgs,
- hitCount: linePart[nArgs].hitCount });
- }
+ for (let [nameLineKey, linePart] of functionCounters) {
+ let [name, line] = nameLineKey.split(':');
+ line = Number(line);
+ for (let [nArgs, {hitCount}] of linePart) {
+ let key = [name, line, nArgs].join(':');
+ arrayReturn.push({key, line, nArgs, hitCount});
}
}
arrayReturn.sort(function(left, right) {
- if (left.name < right.name)
+ if (left.key < right.key)
return -1;
- else if (left.name > right.name)
+ else if (left.key > right.key)
return 1;
else
return 0;
@@ -785,12 +774,8 @@ var CoverageStatisticsContainer = class {
checksum: mtime === null ? getFileChecksum(filename) : null,
lines: [],
branches: [],
- functions:
_convertFunctionCountersToArray(statisticsForFilename.functionCounters).map(function(func) {
- return {
- key: func.name,
- line: func.line
- };
- })
+ functions: _convertFunctionCountersToArray(statisticsForFilename.functionCounters)
+ .map(({key, line, nArgs}) => ({key, line, n_params: nArgs})),
};
/* We're using a index based loop here since we need access to the
diff --git a/test/gjs-test-coverage.cpp b/test/gjs-test-coverage.cpp
index c7b2824..cb6fbf4 100644
--- a/test/gjs-test-coverage.cpp
+++ b/test/gjs-test-coverage.cpp
@@ -2303,7 +2303,7 @@ void gjs_test_add_tests_for_coverage()
"resource://org/gnome/gjs/mock/test/gjs-test-coverage/cache_notation/simple_function.js",
"1,2",
"",
- "\"key\":\"f:1:0\",\"line\":1"
+ "\"key\":\"f:1:0\",\"line\":1,\"n_params\":0"
}
};
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
