Skip to content

Commit

Permalink
Performance Improvements (#307)
Browse files Browse the repository at this point in the history
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons.

In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions).

All Commits:

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+)

* Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests

* Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests

* Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList)

* Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues;

* Correct wrong ESLint update for eslintrc.js

* Update cql4browsers.js

* Update ESLint config to use ecmaVersion 8 to align with es2017 update

* Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty

* Resolve cql4broswers, eslint, and prettier issues

* Using 'has' has better performance

* Change type checks in uncertainty; Add test to distinguish possible solutions

* Update test comment

* Change normalization to use fixed names where possible

* Change normalization to use constructor (instead of constructor.name)

* Add a _typeHierarchy check in normalization for model objects

* Reword comment.

* Rename memoizer.ts; Simplify; Update descriptions.

* Fix Context.childContext(null) to default to empty context_values object (#282)

* Fix Context.childContext to default to {} on undefined AND null

Fixes #277

* Context type updates & UnfilteredContext bug fix

Updated Context constructor to indicate that a Library can be passed in as the parent.  Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions.  Oops!  Fixed it.

* Upgrade gradle wrapper

* npm audit fix + npm install --save-dev mocha@latest

* v2.4.3 (#284)

* Re-up to package lock v2 after merge from master

* Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too

* Resolve npm security vulnerability via audit

* Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil

* Revert doUnion

* Update cql4browsers post doUnion reversion

* Prettier spacing

* Update var names to reflect change from kvp to just keys

* Correct build/watch scripts for windows/mac

* Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename

* Add .version/.display for normalized key for codes

* Update test name

* Add description for immutableUtil

* Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule

* Browserify, lint, prettier update post changes

* Post Rebase Corrections

* Matt/Chris Review updates

* Resolve commit issues

Co-authored-by: Chris Moesel <[email protected]>
  • Loading branch information
2 people authored and mgramigna committed Mar 28, 2023
1 parent c58f017 commit 6cb8ec9
Show file tree
Hide file tree
Showing 31 changed files with 12,165 additions and 1,270 deletions.
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# /node_modules/* and /bower_components/* ignored by default

.eslintrc.json
lib
lib-test
examples/browser/cql4browsers.js
Expand Down
46 changes: 24 additions & 22 deletions .eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,33 +1,35 @@
{
"parser": "@typescript-eslint/parser",
"parserOptions": {
"ecmaVersion": 6,
"sourceType": "module",
"ecmaFeatures": {
"experimentalObjectRestSpread": true
}
"ecmaVersion": 6,
"sourceType": "module",
"ecmaFeatures": {
"experimentalObjectRestSpread": true
}
},
"rules": {
"@typescript-eslint/no-explicit-any": "off",
"@typescript-eslint/no-unused-vars": ["error", { "argsIgnorePattern": "^_" }],
"indent": ["error", 2, { "SwitchCase": 1 }],
"linebreak-style": ["error", "unix"],
"no-console": ["warn", { "allow": ["warn", "error"] }],
"no-loss-of-precision": "off",
"no-unused-vars": ["error", { "vars": "all", "args": "none" }],
"quotes": ["error", "single", { "allowTemplateLiterals": true, "avoidEscape": true }],
"semi": ["error", "always"]
"@typescript-eslint/no-explicit-any": "off",
"@typescript-eslint/no-unused-vars": ["error", { "argsIgnorePattern": "^_" }],
"indent": ["error", 2, { "SwitchCase": 1 }],
"no-console": ["warn", { "allow": ["warn", "error"] }],
"no-loss-of-precision": "off",
"no-unused-vars": ["error", { "vars": "all", "args": "none" }],
"quotes": ["error", "single", { "allowTemplateLiterals": true, "avoidEscape": true }],
"semi": ["error", "always"],
"curly": "error"
},
"overrides": [{
"files": ["test/**/*.{js,ts}", "examples/**/*.js", "bin/**/*.js"],
"rules": {
"@typescript-eslint/no-var-requires": "off"
"overrides": [
{
"files": ["test/**/*.{js,ts}", "examples/**/*.js", "bin/**/*.js"],
"rules": {
"@typescript-eslint/no-var-requires": "off"
}
}
}],
],
"env": {
"es6": true,
"node": true,
"mocha": true
"es6": true,
"node": true,
"mocha": true
},
"extends": ["plugin:@typescript-eslint/recommended", "prettier"]
}
76 changes: 38 additions & 38 deletions .github/workflows/ci-workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,52 +7,52 @@ jobs:
name: Check npm audit
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- uses: actions/setup-node@v1
with:
node-version: '12.x'
- run: npm audit
env:
CI: true
- uses: actions/checkout@v1
- uses: actions/setup-node@v1
with:
node-version: '18.x'
- run: npm audit
env:
CI: true
cql4browsers:
name: Check cql4browsers
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- uses: actions/setup-node@v1
with:
node-version: '12.x'
- run: ./bin/validate_cql4browsers.sh
env:
CI: true
- uses: actions/checkout@v1
- uses: actions/setup-node@v1
with:
node-version: '18.x'
- run: ./bin/validate_cql4browsers.sh
env:
CI: true
lint:
name: Check lint and prettier
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- uses: actions/setup-node@v1
with:
node-version: '12.x'
- run: npm install
- run: npm run lint
- run: npm run prettier
env:
CI: true
- uses: actions/checkout@v1
- uses: actions/setup-node@v1
with:
node-version: '18.x'
- run: npm install
- run: npm run lint
- run: npm run prettier
env:
CI: true
test:
name: Test on node 12 and ubuntu-latest
name: Test on node 18 and ubuntu-latest
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- uses: actions/setup-node@v1
with:
node-version: '12.x'
- name: Set timezone to America/New_York
uses: zcong1993/setup-timezone@master
with:
timezone: America/New_York
- run: date +"%Y-%m-%d %T"
- run: npm install
- run: ./bin/check_for_nonassertive_tests.sh
- run: npx nyc --reporter=lcov npm test && npx codecov
env:
CI: true
- uses: actions/checkout@v1
- uses: actions/setup-node@v1
with:
node-version: '18.x'
- name: Set timezone to America/New_York
uses: zcong1993/setup-timezone@master
with:
timezone: America/New_York
- run: date +"%Y-%m-%d %T"
- run: npm install
- run: ./bin/check_for_nonassertive_tests.sh
- run: npx nyc --reporter=lcov npm test && npx codecov
env:
CI: true
Loading

0 comments on commit 6cb8ec9

Please sign in to comment.