Perf optimization v3#307
Merged
Merged
Conversation
…age-lock to v2 (npm 7+)
…ix broken tests
…mparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests
…ntersect) and queries (toDistinctList)
…age-lock to v2 (npm 7+)
…ix broken tests
…mparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests
…ntersect) and queries (toDistinctList)
…ove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty
…ect (#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
…ipts to run in Windows env too
…ts implementation; Rename
… keeping curly rule
Codecov Report
@@ Coverage Diff @@
## next #307 +/- ##
==========================================
- Coverage 86.13% 86.09% -0.05%
==========================================
Files 50 51 +1
Lines 4427 4486 +59
Branches 1245 1265 +20
==========================================
+ Hits 3813 3862 +49
- Misses 321 324 +3
- Partials 293 300 +7
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
mgramigna
approved these changes
Jan 15, 2023
cmoesel
added a commit
that referenced
this pull request
Jan 16, 2023
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 <cmoesel@users.noreply.github.com>
mgramigna
pushed a commit
that referenced
this pull request
Jan 16, 2023
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 <cmoesel@users.noreply.github.com>
mgramigna
pushed a commit
that referenced
this pull request
Jan 16, 2023
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 <cmoesel@users.noreply.github.com>
mgramigna
pushed a commit
that referenced
this pull request
Mar 27, 2023
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 <cmoesel@users.noreply.github.com>
mgramigna
pushed a commit
that referenced
this pull request
Mar 27, 2023
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 <cmoesel@users.noreply.github.com>
mgramigna
pushed a commit
that referenced
this pull request
Mar 27, 2023
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 <cmoesel@users.noreply.github.com>
mgramigna
pushed a commit
that referenced
this pull request
Mar 28, 2023
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 <cmoesel@users.noreply.github.com>
mgramigna
pushed a commit
that referenced
this pull request
Mar 29, 2023
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 <cmoesel@users.noreply.github.com>
mgramigna
pushed a commit
that referenced
this pull request
Apr 21, 2023
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 <cmoesel@users.noreply.github.com>
cmoesel
added a commit
that referenced
this pull request
May 4, 2023
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 <cmoesel@users.noreply.github.com>
cmoesel
added a commit
that referenced
this pull request
May 4, 2023
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 <cmoesel@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request supersedes #278 as it has been rebased to the
nextbranch. The decision was made that rebasing to thenextbranch would be the appropriate way to promote to production.Closes #278
Pull requests into cql-execution require the following.
Submitter and reviewer should ✔ when done.
For items that are not-applicable, mark "N/A" and ✔.
CDS Connect and Bonnie are the main users of this repository.
It is strongly recommended to include a person from each of those projects as a reviewer.
Submitter:
npm run test:plusto run tests, lint, and prettier)cql4browsers.jsbuilt withnpm run build:browserifyif source changed.Reviewer:
Name: