Skip to content

Commit fbd22a7

Browse files
Merge branch 'bugfix/LF-2789/improve-compare-performance-task' into 'master'
Improve "compare-performance" task See merge request lfor/fhirpath.js!4
2 parents e8c9338 + bfceb9f commit fbd22a7

12 files changed

Lines changed: 213 additions & 119 deletions

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@
33
This log documents significant changes for each release. This project follows
44
[Semantic Versioning](http://semver.org/).
55

6+
## [3.10.1] - 2024-01-29
7+
### Changed
8+
- Improved performance comparison task: added command line options and enabled
9+
Ctrl+C for stopping the tests.
10+
611
## [3.10.0] - 2024-01-23
712
### Added
813
- Support for comparison and math operations with Quantity values.

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "fhirpath",
3-
"version": "3.10.0",
3+
"version": "3.10.1",
44
"description": "A FHIRPath engine",
55
"main": "src/fhirpath.js",
66
"dependencies": {

test/benchmark.js

Lines changed: 45 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -7,102 +7,60 @@
77
*
88
* You can also specify a specific version to compare with the current version,
99
* for example:
10-
* `npm run compare-performance -- 2.14.0`
10+
* `npm run compare-performance -- -v 2.14.0`
11+
*
12+
* To see all available options:
13+
* npm run compare-performance -- -h
1114
*
1215
* The results will be saved in the folder `./benchmark/results`
1316
*/
14-
const { spawn } = require('child_process');
15-
const myArgs = process.argv.slice(2);
16-
const prevVersion = myArgs.length === 1 ? '@' + myArgs[0] : '@latest';
17+
const { spawn, fork } = require('child_process');
1718

18-
const benny = require('benny');
19-
const open = require('open');
20-
const current_fhirpath = require('../src/fhirpath');
21-
const current_r4_model = require('../fhir-context/r4');
22-
const minimumDataset = require('./resources/Minimum-Data-Set---version-3.0.R4.json');
23-
const _ = require('lodash');
24-
const currentVersion = 'current';
19+
// Insert performance test suites here:
20+
const availableTests = [
21+
'intersect',
22+
'member-invocation',
23+
'union',
24+
'exclude',
25+
'subsetof',
26+
'distinct',
27+
'contains'
28+
];
2529

30+
const options = require('commander');
31+
options.description('Compare performance between the latest published version and the current version.');
32+
options.option('-v, --prevVersion <version>', 'use a specific version instead of latest published version.', 'latest');
33+
options.option('-t, --tests <list>', `list of comma-separated tests (e.g. "${availableTests.join(',')}")`, availableTests.join(','));
34+
options.option('-c, --compileOnly', 'skip tests for evaluate().', 'latest');
35+
options.parse(process.argv);
2636

27-
const child = spawn('npm', ['i','--prefix', './test/benchmark/prev-fhirpath', 'fhirpath' + prevVersion], {
37+
const npmInstallProcess = spawn('npm', ['i', '--prefix', './test/benchmark/prev-fhirpath', 'fhirpath@' + options.prevVersion], {
2838
stdio: 'inherit'
2939
});
3040

31-
child.on('exit', code => {
32-
if (code === 0) {
33-
const previous_fhirpath = require('./benchmark/prev-fhirpath/node_modules/fhirpath');
34-
const previous_r4_model = require('./benchmark/prev-fhirpath/node_modules/fhirpath/fhir-context/r4');
35-
const previousVersion = require('./benchmark/prev-fhirpath/node_modules/fhirpath/package.json').version;
36-
const bigItems = current_fhirpath.evaluate(minimumDataset,'repeat(item)', {}, current_r4_model);
37-
const bigItemsCopy = _.cloneDeep(bigItems);
38-
const smallItems = current_fhirpath.evaluate(minimumDataset,'repeat(item).repeat(code)', {}, current_r4_model)
39-
const smallItemsCopy = _.cloneDeep(smallItems);
40-
// Insert performance test suites here:
41-
[
42-
'intersect',
43-
'member-invocation',
44-
'union',
45-
'exclude',
46-
'subsetof',
47-
'distinct',
48-
'contains'
49-
].forEach(filename => {
50-
const suites = require('./benchmark/'+ filename)({
51-
benny,
52-
open,
53-
previous_fhirpath,
54-
previous_r4_model,
55-
current_fhirpath,
56-
current_r4_model,
57-
minimumDataset,
58-
currentVersion,
59-
previousVersion,
60-
bigItems,
61-
bigItemsCopy,
62-
smallItems,
63-
smallItemsCopy
64-
});
65-
suites.forEach(suite => {
66-
const cases = suite.cases.reduce((arr, item) => {
67-
arr.push(
68-
benny.add(
69-
`${item.name} [${previousVersion}]`,
70-
item.testFunction.bind(
71-
this,
72-
previous_fhirpath,
73-
previous_r4_model,
74-
previous_fhirpath.compile(suite.expression, previous_r4_model)
75-
)
76-
)
77-
);
78-
arr.push(
79-
benny.add(
80-
`${item.name} [${currentVersion}]`,
81-
item.testFunction.bind(
82-
this,
83-
current_fhirpath,
84-
current_r4_model,
85-
current_fhirpath.compile(suite.expression, current_r4_model)
86-
)
87-
)
88-
);
89-
return arr;
90-
}, []);
41+
// Process for running benchmarks. We need a separate process to run the tests
42+
// to free the main process from synchronous code to listen for the SIGINT event.
43+
let benchmarkingProcess;
44+
45+
process.on('SIGINT', () => {
46+
// Kill the benchmarking process
47+
if (benchmarkingProcess) {
48+
benchmarkingProcess.kill('SIGKILL');
49+
// Displays the bash prompt on a new line.
50+
console.log('');
51+
}
9152

92-
benny.suite(
93-
suite.name,
94-
...cases,
95-
benny.cycle(),
96-
benny.complete(),
97-
benny.configure({
98-
minDisplayPrecision: 2
99-
}),
100-
benny.save({ file: suite.filename, folder: __dirname + '/results', version: currentVersion }),
101-
benny.save({ file: suite.filename, folder: __dirname + '/results', format: 'chart.html' }),
102-
).then(() => {
103-
open(__dirname + `/results/${suite.filename}.chart.html`);
104-
});
105-
});
106-
})
53+
// The value of the SIGINT signal code is 2 (See https://man7.org/linux/man-pages/man7/signal.7.html).
54+
// If Node.js receives a fatal signal such as SIGKILL or SIGHUP, then its exit
55+
// code will be 128 plus the value of the signal code:
56+
// 128 + 2 = 130 (see https://nodejs.org/api/process.html#exit-codes)
57+
process.exit(130);
58+
});
59+
60+
npmInstallProcess.on('exit', code => {
61+
if (code === 0) {
62+
benchmarkingProcess = fork(__dirname + '/benchmark/runner.js', { stdio: 'inherit'});
63+
// Pass options to the benchmarking process to run benchmarks
64+
benchmarkingProcess.send(options);
10765
}
10866
});

test/benchmark/common-benchmark-utils.js

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,14 @@
1414
* "%items" variable.
1515
* @param {Array} desc.smallItemsCopy - copy of desc.smallItems in case
1616
* expression uses it as "%itemsCopy".
17+
* @param {Object} desc.options - command line options. See benchmark.js.
1718
* @return an object describing a suite with 4 cases for use in benchmark.js.
1819
*/
1920
function createSuiteForExpression(desc) {
2021
const collectionOfBigItems = desc.bigItems;
2122
const collectionOfBigItemsCopy = desc.bigItemsCopy;
2223
const numberOfBigItems = collectionOfBigItems.length;
24+
const options = desc.options;
2325

2426
const collectionOfSmallItems = desc.smallItems;
2527
const collectionOfSmallItemsCopy = desc.smallItemsCopy;
@@ -30,24 +32,33 @@ function createSuiteForExpression(desc) {
3032
filename: desc.filename,
3133
expression: desc.expression,
3234
cases: [
33-
{
34-
name: `${numberOfBigItems} big items using evaluate()`,
35-
testFunction: (fhirpath, model) => {
36-
fhirpath.evaluate({}, desc.expression, { items: collectionOfBigItems, itemsCopy: collectionOfBigItemsCopy }, model);
37-
}
38-
},
35+
...(options.compileOnly
36+
? []
37+
: [{
38+
name: `${numberOfBigItems} big items using evaluate()`,
39+
testFunction: (fhirpath, model) => {
40+
fhirpath.evaluate({}, desc.expression, {
41+
items: collectionOfBigItems,
42+
itemsCopy: collectionOfBigItemsCopy
43+
}, model);
44+
}
45+
}]
46+
),
3947
{
4048
name: `${numberOfBigItems} big items using compile()`,
4149
testFunction: (fhirpath, model, compiledFn) => {
4250
compiledFn({}, { items: collectionOfBigItems, itemsCopy: collectionOfBigItemsCopy });
4351
}
4452
},
45-
{
46-
name: `${numberOfSmallItems} small items using evaluate()`,
47-
testFunction: (fhirpath, model) => {
48-
fhirpath.evaluate({}, desc.expression, { items: collectionOfSmallItems, itemsCopy: collectionOfSmallItemsCopy }, model);
49-
}
50-
},
53+
...(options.compileOnly
54+
? []
55+
: [{
56+
name: `${numberOfSmallItems} small items using evaluate()`,
57+
testFunction: (fhirpath, model) => {
58+
fhirpath.evaluate({}, desc.expression, { items: collectionOfSmallItems, itemsCopy: collectionOfSmallItemsCopy }, model);
59+
}
60+
}]
61+
),
5162
{
5263
name: `${numberOfSmallItems} small items using compile()`,
5364
testFunction: (fhirpath, model, compiledFn) => {

test/benchmark/distinct.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const { createSuiteForExpression } = require('./common-benchmark-utils');
44
module.exports = ({
55
bigItems,
66
smallItems,
7+
options
78
}) => {
89
const expression = '%items.distinct()';
910

@@ -12,13 +13,15 @@ module.exports = ({
1213
filename: 'distinct',
1314
expression,
1415
bigItems,
15-
smallItems
16+
smallItems,
17+
options
1618
}, {
1719
name: 'distinct() of a small collection with',
1820
filename: 'distinct-for-small-collections',
1921
expression,
2022
bigItems: bigItems.slice(-maxCollSizeForDeepEqual),
21-
smallItems: smallItems.slice(-maxCollSizeForDeepEqual)
23+
smallItems: smallItems.slice(-maxCollSizeForDeepEqual),
24+
options
2225
}].map(createSuiteForExpression);
2326

2427
}

test/benchmark/exclude.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ module.exports = ({
66
bigItems,
77
bigItemsCopy,
88
smallItems,
9-
smallItemsCopy
9+
smallItemsCopy,
10+
options
1011
}) => {
1112
const smallCollectionLength = Math.floor(maxCollSizeForDeepEqual/2);
1213
const expression = '%items.exclude(%itemsCopy)';
@@ -18,15 +19,17 @@ module.exports = ({
1819
bigItems,
1920
bigItemsCopy,
2021
smallItems,
21-
smallItemsCopy
22+
smallItemsCopy,
23+
options
2224
}, {
2325
name: 'exclude() all elements from a small collection with',
2426
filename: 'exclude-for-small-collections',
2527
expression,
2628
bigItems: bigItems.slice(-smallCollectionLength),
2729
bigItemsCopy: bigItemsCopy.slice(-smallCollectionLength),
2830
smallItems: smallItems.slice(-smallCollectionLength),
29-
smallItemsCopy: smallItemsCopy.slice(-smallCollectionLength)
31+
smallItemsCopy: smallItemsCopy.slice(-smallCollectionLength),
32+
options
3033
}].map(createSuiteForExpression);
3134

3235
}

test/benchmark/intersect.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ module.exports = ({
66
bigItems,
77
bigItemsCopy,
88
smallItems,
9-
smallItemsCopy
9+
smallItemsCopy,
10+
options
1011
}) => {
1112
const smallCollectionLength = Math.floor(maxCollSizeForDeepEqual/2);
1213
const expression = '%items.intersect(%itemsCopy)';
@@ -18,15 +19,17 @@ module.exports = ({
1819
bigItems,
1920
bigItemsCopy,
2021
smallItems,
21-
smallItemsCopy
22+
smallItemsCopy,
23+
options
2224
}, {
2325
name: 'intersect() of two small collections with',
2426
filename: 'intersect-for-small-collections',
2527
expression,
2628
bigItems: bigItems.slice(-smallCollectionLength),
2729
bigItemsCopy: bigItemsCopy.slice(-smallCollectionLength),
2830
smallItems: smallItems.slice(-smallCollectionLength),
29-
smallItemsCopy: smallItemsCopy.slice(-smallCollectionLength)
31+
smallItemsCopy: smallItemsCopy.slice(-smallCollectionLength),
32+
options
3033
}].map(createSuiteForExpression);
3134

3235
}

test/benchmark/member-invocation.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ const _ = require('lodash');
33
module.exports = ({
44
current_fhirpath,
55
current_r4_model,
6-
minimumDataset
6+
minimumDataset,
7+
options
78
}) => {
89

910
const numberOfItems = current_fhirpath.evaluate(minimumDataset,'Questionnaire.item.count()', {}, current_r4_model);
@@ -14,12 +15,14 @@ module.exports = ({
1415
filename: 'member-invocation',
1516
expression,
1617
cases: [
17-
{
18+
...(options.compileOnly
19+
? []
20+
: [{
1821
name: `using evaluate()`,
1922
testFunction: (fhirpath, model) => {
2023
fhirpath.evaluate(minimumDataset, expression, {}, model);
2124
}
22-
},
25+
}]),
2326
{
2427
name: `using compile()`,
2528
testFunction: (fhirpath, model, compiledFn) => {

0 commit comments

Comments
 (0)