From be9c71bd67d86e48a09e1ac42942909261f9cc3f Mon Sep 17 00:00:00 2001 From: Robert A Dingwell Date: Thu, 10 Oct 2024 15:22:13 -0400 Subject: [PATCH 1/8] implmenting caching of matching FunctionDefs in FunctionRef to improve performance. --- src/elm/reusable.ts | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/src/elm/reusable.ts b/src/elm/reusable.ts index 2dfe6b01d..0609f3df5 100644 --- a/src/elm/reusable.ts +++ b/src/elm/reusable.ts @@ -59,27 +59,28 @@ export class FunctionDef extends Expression { export class FunctionRef extends Expression { name: string; library: string; + functionDefs: FunctionDef[] | null; constructor(json: any) { super(json); this.name = json.name; this.library = json.libraryName; + this.functionDefs = null; } - async exec(ctx: Context) { + getFunctionDefs(ctx: Context, args: any) { + if (this.functionDefs != null) { + // cache hit + return this.functionDefs; + } let functionDefs, child_ctx; if (this.library) { const lib = ctx.get(this.library); functionDefs = lib ? lib.getFunction(this.name) : undefined; - const libCtx = ctx.getLibraryContext(this.library); - child_ctx = libCtx ? libCtx.childContext() : undefined; } else { functionDefs = ctx.get(this.name); - child_ctx = ctx.childContext(); } - const args = await this.execArgs(ctx); - // Filter out functions w/ wrong number of arguments. functionDefs = functionDefs.filter((f: any) => f.parameters.length === args.length); // If there is still > 1 matching function, filter by argument types if (functionDefs.length > 1) { @@ -101,17 +102,33 @@ export class FunctionRef extends Expression { return match; }); } + this.functionDefs = functionDefs; + return functionDefs; + } + + async exec(ctx: Context) { + const args = await this.execArgs(ctx); + // Filter out functions w/ wrong number of arguments. + const fDefs = this.getFunctionDefs(ctx, args); // If there is still > 1 matching function, calculate a score based on quality of matches - if (functionDefs.length > 1) { + if (fDefs.length > 1) { // TODO } - if (functionDefs.length === 0) { + if (fDefs.length === 0) { throw new Error('no function with matching signature could be found'); } + + let child_ctx; + if (this.library) { + const libCtx = ctx.getLibraryContext(this.library); + child_ctx = libCtx ? libCtx.childContext() : undefined; + } else { + child_ctx = ctx.childContext(); + } // By this point, we should have only one function, but until implementation is completed, // use the last one (no matter how many still remain) - const functionDef = functionDefs[functionDefs.length - 1]; + const functionDef = fDefs[fDefs.length - 1]; for (let i = 0; i < functionDef.parameters.length; i++) { child_ctx.set(functionDef.parameters[i].name, args[i]); } From 2406a8fe02077a4494a123afc1046eae0942f48a Mon Sep 17 00:00:00 2001 From: Robert A Dingwell Date: Thu, 10 Oct 2024 15:54:08 -0400 Subject: [PATCH 2/8] removing unused variable --- examples/browser/cql4browsers.js | 35 +++++++++++++++++++++++--------- src/elm/reusable.ts | 2 +- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/examples/browser/cql4browsers.js b/examples/browser/cql4browsers.js index b969ee813..5ac7fdc30 100644 --- a/examples/browser/cql4browsers.js +++ b/examples/browser/cql4browsers.js @@ -6811,21 +6811,21 @@ class FunctionRef extends expression_1.Expression { super(json); this.name = json.name; this.library = json.libraryName; + this.functionDefs = null; } - async exec(ctx) { - let functionDefs, child_ctx; + getFunctionDefs(ctx, args) { + if (this.functionDefs != null) { + // cache hit + return this.functionDefs; + } + let functionDefs; if (this.library) { const lib = ctx.get(this.library); functionDefs = lib ? lib.getFunction(this.name) : undefined; - const libCtx = ctx.getLibraryContext(this.library); - child_ctx = libCtx ? libCtx.childContext() : undefined; } else { functionDefs = ctx.get(this.name); - child_ctx = ctx.childContext(); } - const args = await this.execArgs(ctx); - // Filter out functions w/ wrong number of arguments. functionDefs = functionDefs.filter((f) => f.parameters.length === args.length); // If there is still > 1 matching function, filter by argument types if (functionDefs.length > 1) { @@ -6847,16 +6847,31 @@ class FunctionRef extends expression_1.Expression { return match; }); } + this.functionDefs = functionDefs; + return functionDefs; + } + async exec(ctx) { + const args = await this.execArgs(ctx); + // Filter out functions w/ wrong number of arguments. + const fDefs = this.getFunctionDefs(ctx, args); // If there is still > 1 matching function, calculate a score based on quality of matches - if (functionDefs.length > 1) { + if (fDefs.length > 1) { // TODO } - if (functionDefs.length === 0) { + if (fDefs.length === 0) { throw new Error('no function with matching signature could be found'); } + let child_ctx; + if (this.library) { + const libCtx = ctx.getLibraryContext(this.library); + child_ctx = libCtx ? libCtx.childContext() : undefined; + } + else { + child_ctx = ctx.childContext(); + } // By this point, we should have only one function, but until implementation is completed, // use the last one (no matter how many still remain) - const functionDef = functionDefs[functionDefs.length - 1]; + const functionDef = fDefs[fDefs.length - 1]; for (let i = 0; i < functionDef.parameters.length; i++) { child_ctx.set(functionDef.parameters[i].name, args[i]); } diff --git a/src/elm/reusable.ts b/src/elm/reusable.ts index 0609f3df5..485fcc94d 100644 --- a/src/elm/reusable.ts +++ b/src/elm/reusable.ts @@ -73,7 +73,7 @@ export class FunctionRef extends Expression { // cache hit return this.functionDefs; } - let functionDefs, child_ctx; + let functionDefs; if (this.library) { const lib = ctx.get(this.library); functionDefs = lib ? lib.getFunction(this.name) : undefined; From fe017148a85ce033afb737e873a8034bfbfc76bd Mon Sep 17 00:00:00 2001 From: Robert A Dingwell Date: Thu, 10 Oct 2024 15:56:37 -0400 Subject: [PATCH 3/8] adding a comment about moving some code --- src/elm/reusable.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/elm/reusable.ts b/src/elm/reusable.ts index 485fcc94d..4de6c66c7 100644 --- a/src/elm/reusable.ts +++ b/src/elm/reusable.ts @@ -118,7 +118,8 @@ export class FunctionRef extends Expression { if (fDefs.length === 0) { throw new Error('no function with matching signature could be found'); } - + // Moved context creation below the functionDef checks because it's not needed if + // there are no matching function defs let child_ctx; if (this.library) { const libCtx = ctx.getLibraryContext(this.library); From 4566a616bf3ee291f120dfc29dd6c47e2c46f2ff Mon Sep 17 00:00:00 2001 From: Robert A Dingwell Date: Fri, 11 Oct 2024 16:45:29 -0400 Subject: [PATCH 4/8] prettier fix --- examples/browser/cql4browsers.js | 2 ++ src/elm/reusable.ts | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/examples/browser/cql4browsers.js b/examples/browser/cql4browsers.js index 5ac7fdc30..d4a7efb82 100644 --- a/examples/browser/cql4browsers.js +++ b/examples/browser/cql4browsers.js @@ -6861,6 +6861,8 @@ class FunctionRef extends expression_1.Expression { if (fDefs.length === 0) { throw new Error('no function with matching signature could be found'); } + // Moved context creation below the functionDef checks because it's not needed if + // there are no matching function defs let child_ctx; if (this.library) { const libCtx = ctx.getLibraryContext(this.library); diff --git a/src/elm/reusable.ts b/src/elm/reusable.ts index 4de6c66c7..2df40ff00 100644 --- a/src/elm/reusable.ts +++ b/src/elm/reusable.ts @@ -118,7 +118,7 @@ export class FunctionRef extends Expression { if (fDefs.length === 0) { throw new Error('no function with matching signature could be found'); } - // Moved context creation below the functionDef checks because it's not needed if + // Moved context creation below the functionDef checks because it's not needed if // there are no matching function defs let child_ctx; if (this.library) { From 1a70d0513bb8ae784240a36368b097f11df18766 Mon Sep 17 00:00:00 2001 From: Robert A Dingwell Date: Fri, 11 Oct 2024 16:45:58 -0400 Subject: [PATCH 5/8] prettier fix --- examples/browser/cql4browsers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/browser/cql4browsers.js b/examples/browser/cql4browsers.js index d4a7efb82..c42e4f8f5 100644 --- a/examples/browser/cql4browsers.js +++ b/examples/browser/cql4browsers.js @@ -6861,7 +6861,7 @@ class FunctionRef extends expression_1.Expression { if (fDefs.length === 0) { throw new Error('no function with matching signature could be found'); } - // Moved context creation below the functionDef checks because it's not needed if + // Moved context creation below the functionDef checks because it's not needed if // there are no matching function defs let child_ctx; if (this.library) { From 394b55bad65bd498c6a4c701f89e15994f4582ed Mon Sep 17 00:00:00 2001 From: Robert A Dingwell Date: Fri, 11 Oct 2024 16:46:51 -0400 Subject: [PATCH 6/8] npm audit fix --- package-lock.json | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/package-lock.json b/package-lock.json index c947f90f6..860e2b1d0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1988,11 +1988,10 @@ "license": "ISC" }, "node_modules/elliptic": { - "version": "6.5.5", - "resolved": "https://registry.npmjs.org/elliptic/-/elliptic-6.5.5.tgz", - "integrity": "sha512-7EjbcmUm17NQFu4Pmgmq2olYMj8nwMnpcddByChSUjArp8F5DQWcIcpriwO4ZToLNAJig0yiyjswfyGNje/ixw==", + "version": "6.5.7", + "resolved": "https://registry.npmjs.org/elliptic/-/elliptic-6.5.7.tgz", + "integrity": "sha512-ESVCtTwiA+XhY3wyh24QqRGBoP3rEdDUl3EDUUo9tft074fi19IrdpH7hLCMMP3CIj7jb3W96rn8lt/BqIlt5Q==", "dev": true, - "license": "MIT", "dependencies": { "bn.js": "^4.11.9", "brorand": "^1.1.0", @@ -3306,12 +3305,13 @@ } }, "node_modules/micromatch": { - "version": "4.0.4", + "version": "4.0.8", + "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.8.tgz", + "integrity": "sha512-PXwfBhYu0hBCPw8Dn0E+WDYb7af3dSLVWKi3HGv84IdF4TyFoC0ysxFd0Goxw7nSv4T/PzEJQxsYsEiFCKo2BA==", "dev": true, - "license": "MIT", "dependencies": { - "braces": "^3.0.1", - "picomatch": "^2.2.3" + "braces": "^3.0.3", + "picomatch": "^2.3.1" }, "engines": { "node": ">=8.6" @@ -3951,9 +3951,10 @@ } }, "node_modules/path-to-regexp": { - "version": "1.8.0", + "version": "1.9.0", + "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-1.9.0.tgz", + "integrity": "sha512-xIp7/apCFJuUHdDLWe8O1HIkb0kQrOMb/0u6FXQjemHn/ii5LrIzU6bdECnsiTF/GjZkMEKg1xdiZwNqDYlZ6g==", "dev": true, - "license": "MIT", "dependencies": { "isarray": "0.0.1" } @@ -6593,9 +6594,9 @@ "dev": true }, "elliptic": { - "version": "6.5.5", - "resolved": "https://registry.npmjs.org/elliptic/-/elliptic-6.5.5.tgz", - "integrity": "sha512-7EjbcmUm17NQFu4Pmgmq2olYMj8nwMnpcddByChSUjArp8F5DQWcIcpriwO4ZToLNAJig0yiyjswfyGNje/ixw==", + "version": "6.5.7", + "resolved": "https://registry.npmjs.org/elliptic/-/elliptic-6.5.7.tgz", + "integrity": "sha512-ESVCtTwiA+XhY3wyh24QqRGBoP3rEdDUl3EDUUo9tft074fi19IrdpH7hLCMMP3CIj7jb3W96rn8lt/BqIlt5Q==", "dev": true, "requires": { "bn.js": "^4.11.9", @@ -7438,11 +7439,13 @@ "dev": true }, "micromatch": { - "version": "4.0.4", + "version": "4.0.8", + "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.8.tgz", + "integrity": "sha512-PXwfBhYu0hBCPw8Dn0E+WDYb7af3dSLVWKi3HGv84IdF4TyFoC0ysxFd0Goxw7nSv4T/PzEJQxsYsEiFCKo2BA==", "dev": true, "requires": { - "braces": "^3.0.1", - "picomatch": "^2.2.3" + "braces": "^3.0.3", + "picomatch": "^2.3.1" } }, "miller-rabin": { @@ -7888,7 +7891,9 @@ "dev": true }, "path-to-regexp": { - "version": "1.8.0", + "version": "1.9.0", + "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-1.9.0.tgz", + "integrity": "sha512-xIp7/apCFJuUHdDLWe8O1HIkb0kQrOMb/0u6FXQjemHn/ii5LrIzU6bdECnsiTF/GjZkMEKg1xdiZwNqDYlZ6g==", "dev": true, "requires": { "isarray": "0.0.1" From 522ca7d625feb8cf149eaf21dcc560e925982ff2 Mon Sep 17 00:00:00 2001 From: Robert A Dingwell Date: Fri, 11 Oct 2024 17:07:02 -0400 Subject: [PATCH 7/8] Moved new function below existing one to make diff easier to read --- examples/browser/cql4browsers.js | 58 +++++++++++++++--------------- src/elm/reusable.ts | 60 ++++++++++++++++---------------- 2 files changed, 59 insertions(+), 59 deletions(-) diff --git a/examples/browser/cql4browsers.js b/examples/browser/cql4browsers.js index c42e4f8f5..3aac85498 100644 --- a/examples/browser/cql4browsers.js +++ b/examples/browser/cql4browsers.js @@ -6813,6 +6813,35 @@ class FunctionRef extends expression_1.Expression { this.library = json.libraryName; this.functionDefs = null; } + async exec(ctx) { + const args = await this.execArgs(ctx); + // Filter out functions w/ wrong number of arguments. + const fDefs = this.getFunctionDefs(ctx, args); + // If there is still > 1 matching function, calculate a score based on quality of matches + if (fDefs.length > 1) { + // TODO + } + if (fDefs.length === 0) { + throw new Error('no function with matching signature could be found'); + } + // Moved context creation below the functionDef checks because it's not needed if + // there are no matching function defs + let child_ctx; + if (this.library) { + const libCtx = ctx.getLibraryContext(this.library); + child_ctx = libCtx ? libCtx.childContext() : undefined; + } + else { + child_ctx = ctx.childContext(); + } + // By this point, we should have only one function, but until implementation is completed, + // use the last one (no matter how many still remain) + const functionDef = fDefs[fDefs.length - 1]; + for (let i = 0; i < functionDef.parameters.length; i++) { + child_ctx.set(functionDef.parameters[i].name, args[i]); + } + return functionDef.expression.execute(child_ctx); + } getFunctionDefs(ctx, args) { if (this.functionDefs != null) { // cache hit @@ -6850,35 +6879,6 @@ class FunctionRef extends expression_1.Expression { this.functionDefs = functionDefs; return functionDefs; } - async exec(ctx) { - const args = await this.execArgs(ctx); - // Filter out functions w/ wrong number of arguments. - const fDefs = this.getFunctionDefs(ctx, args); - // If there is still > 1 matching function, calculate a score based on quality of matches - if (fDefs.length > 1) { - // TODO - } - if (fDefs.length === 0) { - throw new Error('no function with matching signature could be found'); - } - // Moved context creation below the functionDef checks because it's not needed if - // there are no matching function defs - let child_ctx; - if (this.library) { - const libCtx = ctx.getLibraryContext(this.library); - child_ctx = libCtx ? libCtx.childContext() : undefined; - } - else { - child_ctx = ctx.childContext(); - } - // By this point, we should have only one function, but until implementation is completed, - // use the last one (no matter how many still remain) - const functionDef = fDefs[fDefs.length - 1]; - for (let i = 0; i < functionDef.parameters.length; i++) { - child_ctx.set(functionDef.parameters[i].name, args[i]); - } - return functionDef.expression.execute(child_ctx); - } } exports.FunctionRef = FunctionRef; class OperandRef extends expression_1.Expression { diff --git a/src/elm/reusable.ts b/src/elm/reusable.ts index 2df40ff00..578008271 100644 --- a/src/elm/reusable.ts +++ b/src/elm/reusable.ts @@ -68,6 +68,36 @@ export class FunctionRef extends Expression { this.functionDefs = null; } + async exec(ctx: Context) { + const args = await this.execArgs(ctx); + // Filter out functions w/ wrong number of arguments. + const fDefs = this.getFunctionDefs(ctx, args); + // If there is still > 1 matching function, calculate a score based on quality of matches + if (fDefs.length > 1) { + // TODO + } + + if (fDefs.length === 0) { + throw new Error('no function with matching signature could be found'); + } + // Moved context creation below the functionDef checks because it's not needed if + // there are no matching function defs + let child_ctx; + if (this.library) { + const libCtx = ctx.getLibraryContext(this.library); + child_ctx = libCtx ? libCtx.childContext() : undefined; + } else { + child_ctx = ctx.childContext(); + } + // By this point, we should have only one function, but until implementation is completed, + // use the last one (no matter how many still remain) + const functionDef = fDefs[fDefs.length - 1]; + for (let i = 0; i < functionDef.parameters.length; i++) { + child_ctx.set(functionDef.parameters[i].name, args[i]); + } + return functionDef.expression.execute(child_ctx); + } + getFunctionDefs(ctx: Context, args: any) { if (this.functionDefs != null) { // cache hit @@ -105,36 +135,6 @@ export class FunctionRef extends Expression { this.functionDefs = functionDefs; return functionDefs; } - - async exec(ctx: Context) { - const args = await this.execArgs(ctx); - // Filter out functions w/ wrong number of arguments. - const fDefs = this.getFunctionDefs(ctx, args); - // If there is still > 1 matching function, calculate a score based on quality of matches - if (fDefs.length > 1) { - // TODO - } - - if (fDefs.length === 0) { - throw new Error('no function with matching signature could be found'); - } - // Moved context creation below the functionDef checks because it's not needed if - // there are no matching function defs - let child_ctx; - if (this.library) { - const libCtx = ctx.getLibraryContext(this.library); - child_ctx = libCtx ? libCtx.childContext() : undefined; - } else { - child_ctx = ctx.childContext(); - } - // By this point, we should have only one function, but until implementation is completed, - // use the last one (no matter how many still remain) - const functionDef = fDefs[fDefs.length - 1]; - for (let i = 0; i < functionDef.parameters.length; i++) { - child_ctx.set(functionDef.parameters[i].name, args[i]); - } - return functionDef.expression.execute(child_ctx); - } } export class OperandRef extends Expression { From 5d0babb412c3430fd0a165c00399161fdd78ac8c Mon Sep 17 00:00:00 2001 From: Chris Moesel Date: Mon, 21 Oct 2024 15:57:33 -0400 Subject: [PATCH 8/8] Use CodeCov GitHub Action in CI instead of npx codecov The npx codecov script is deprecated, so use the GitHub action instead. --- .github/workflows/ci-workflow.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci-workflow.yml b/.github/workflows/ci-workflow.yml index b81690d7a..171f7cb60 100644 --- a/.github/workflows/ci-workflow.yml +++ b/.github/workflows/ci-workflow.yml @@ -53,6 +53,10 @@ jobs: - 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 + - run: npx nyc --reporter=lcov npm test env: CI: true + - name: Upload coverage to Codecov + uses: codecov/codecov-action@v4 + with: + token: ${{ secrets.CODECOV_TOKEN }}