Improve performance with large CodeSystems

src/export/CodeSystemExporter.ts

@@ -53,6 +53,8 @@ export class CodeSystemExporter {
     if (concepts.length > 0) {
       codeSystem.concept = [];
       const existingConcepts = new Map<string, ConceptRule>();
+      // each list of concepts is indexed by code so that finding an ancestor does not require scanning the list
+      const conceptsByCode = new Map<CodeSystemConcept[], Map<string, CodeSystemConcept>>();
       concepts.forEach(concept => {
         const existingConcept = existingConcepts.get(concept.code);
         if (existingConcept) {
@@ -82,9 +84,7 @@ export class CodeSystemExporter {
             newConcept.definition = concept.definition;
           }
           for (const ancestorCode of concept.hierarchy) {
-            const ancestorConcept = conceptContainer.find(
-              ancestorConcept => ancestorConcept.code === ancestorCode
-            );
+            const ancestorConcept = conceptsByCode.get(conceptContainer)?.get(ancestorCode);
             if (ancestorConcept) {
               if (!ancestorConcept.concept) {
                 ancestorConcept.concept = [];
@@ -98,7 +98,13 @@ export class CodeSystemExporter {
               return;
             }
           }
+          let siblingsByCode = conceptsByCode.get(conceptContainer);
+          if (siblingsByCode == null) {
+            siblingsByCode = new Map();
+            conceptsByCode.set(conceptContainer, siblingsByCode);
+          }
           conceptContainer.push(newConcept);
+          siblingsByCode.set(newConcept.code, newConcept);
           existingConcepts.set(concept.code, concept);
         }
       });
@@ -116,9 +122,10 @@ export class CodeSystemExporter {
     // Because this.findConceptPath can potentially throw an error,
     // build a list of successful rules that will actually be applied.
     const successfulRules: CaretValueRule[] = [];
+    const conceptIndexCache = new Map<CodeSystemConcept[], Map<string, number>>();
     rules.forEach(rule => {
       try {
-        rule.path = this.findConceptPath(codeSystem, rule.pathArray);
+        rule.path = this.findConceptPath(codeSystem, rule.pathArray, conceptIndexCache);
         successfulRules.push(rule);
         if (rule.path) {
           rule.isCodeCaretRule = true;
@@ -275,7 +282,8 @@ export class CodeSystemExporter {
           rule.path.length > 1 ? `${rule.path}.${rule.caretPath}` : rule.caretPath,
           rule.value,
           this.fisher,
-          inlineResourceTypes
+          inlineResourceTypes,
+          codeSystemSD
         );
       } catch (err) {
         logger.error(err.message, rule.sourceInfo);
@@ -286,12 +294,41 @@ export class CodeSystemExporter {
     }
   }
 
-  private findConceptPath(codeSystem: CodeSystem, codePath: string[]): string {
+  /**
+   * Finds the FSH path to the concept identified by codePath. For example, if #a is the third top-level
+   * concept and #b is its first child, ['#a', '#b'] becomes concept[2].concept[0]. An empty codePath
+   * (a caret rule that is not on a concept) returns an empty path.
+   * @param {CodeSystem} codeSystem - The CodeSystem containing the concepts
+   * @param {string[]} codePath - The codes (with a leading #) leading to the concept
+   * @param {Map<CodeSystemConcept[], Map<string, number>>} conceptIndexCache - Cache of the index of the
+   *   first concept with each code in each concept list. Every code caret rule needs one of these lookups, so
+   *   the concept lists are indexed once rather than scanned for each rule. The cache is only valid while the
+   *   concept lists are not modified, so callers should use a new Map for each set of rules they resolve.
+   * @returns {string} the path to the concept
+   * @throws {CannotResolvePathError} when a code in codePath is not found
+   */
+  private findConceptPath(
+    codeSystem: CodeSystem,
+    codePath: string[],
+    conceptIndexCache: Map<CodeSystemConcept[], Map<string, number>>
+  ): string {
     const conceptIndices: number[] = [];
     let conceptList = codeSystem.concept ?? [];
     for (const codeStep of codePath) {
-      const stepIndex = conceptList.findIndex(concept => `#${concept.code}` === codeStep);
-      if (stepIndex === -1) {
+      let indexByCode = conceptIndexCache.get(conceptList);
+      if (indexByCode == null) {
+        indexByCode = new Map();
+        // the first concept with a given code wins, matching the findIndex this replaced
+        conceptList.forEach((concept, i) => {
+          const key = `#${concept.code}`;
+          if (!indexByCode.has(key)) {
+            indexByCode.set(key, i);
+          }
+        });
+        conceptIndexCache.set(conceptList, indexByCode);
+      }
+      const stepIndex = indexByCode.get(codeStep);
+      if (stepIndex == null) {
         throw new CannotResolvePathError(codePath.join(' '));
       }
       conceptIndices.push(stepIndex);

src/export/ValueSetExporter.ts

@@ -427,7 +427,8 @@ export class ValueSetExporter {
           rule.caretPath,
           rule.value,
           this.fisher,
-          inlineResourceTypes
+          inlineResourceTypes,
+          valueSetSD
         );
       } catch (err) {
         logger.error(err.message, rule.sourceInfo);
@@ -539,7 +540,7 @@ export class ValueSetExporter {
     );
 
     for (const [path, { rule }] of ruleMap) {
-      setPropertyOnDefinitionInstance(vs, path, rule.value, this.fisher);
+      setPropertyOnDefinitionInstance(vs, path, rule.value, this.fisher, [], valueSetSD);
     }
   }
 

src/fhirtypes/common.ts

@@ -94,15 +94,19 @@ export function splitOnPathPeriods(path: string): string[] {
  * @param {string} path - The path to assign a value at
  * @param {any} value - The value to assign
  * @param {Fishable} fisher - A fishable implementation for finding definitions and metadata
+ * @param {string[]} inlineResourceTypes - Types that will be used to replace Resource elements
+ * @param {StructureDefinition} instanceSD - The instance's own StructureDefinition, if the caller already has it.
+ *   CodeSystem and ValueSet rebuild theirs from JSON on every call to getOwnStructureDefinition, so callers
+ *   applying many rules to one such instance should pass it in.
  */
 export function setPropertyOnDefinitionInstance(
   instance: StructureDefinition | ElementDefinition | CodeSystem | ValueSet,
   path: string,
   value: any,
   fisher: Fishable,
-  inlineResourceTypes: string[] = []
+  inlineResourceTypes: string[] = [],
+  instanceSD: StructureDefinition = instance.getOwnStructureDefinition(fisher)
 ): void {
-  const instanceSD = instance.getOwnStructureDefinition(fisher);
   const { assignedValue, pathParts } = instanceSD.validateValueAtPath(
     path,
     value,
@@ -688,18 +692,11 @@ export function setPropertyOnInstance(
             index = sliceIndices[index];
           }
         }
-        // If the index doesn't exist in the array, add it and lesser indices
-        // Empty elements should be null, not undefined, according to https://www.hl7.org/fhir/json.html#primitive
-        for (let j = 0; j <= index; j++) {
-          if (j < current[key].length && j === index && current[key][index] == null) {
-            if (pathPart.primitive) {
-              // a value may already exist on one of the arrays, so only assign an empty object if it is nullish
-              current[pathPart.base][index] ??= {};
-              current[`_${pathPart.base}`][index] ??= {};
-            } else {
-              current[key][index] = {};
-            }
-          } else if (j >= current[key].length) {
+        if (index >= current[key].length) {
+          // Add only the missing elements: iterating from 0 on every call made filling a large array
+          // quadratic. Empty elements should be null, not undefined, according to
+          // https://www.hl7.org/fhir/json.html#primitive
+          for (let j = current[key].length; j <= index; j++) {
             if (sliceName) {
               // _sliceName is used to later differentiate which slice an element represents
               if (pathPart.primitive) {
@@ -724,6 +721,15 @@ export function setPropertyOnInstance(
               }
             }
           }
+        } else if (current[key][index] == null) {
+          // the index exists but is empty, so fill it in
+          if (pathPart.primitive) {
+            // a value may already exist on one of the arrays, so only assign an empty object if it is nullish
+            current[pathPart.base][index] ??= {};
+            current[`_${pathPart.base}`][index] ??= {};
+          } else {
+            current[key][index] = {};
+          }
         }
         // If it isn't the last element, move on, if it is, set the value
         if (i < pathParts.length - 1) {

src/fshtypes/common.ts

@@ -8,7 +8,7 @@ import { Mapping } from './Mapping';
 import { Profile } from './Profile';
 import { Resource } from './Resource';
 import { RuleSet } from './RuleSet';
-import { CaretValueRule, OnlyRuleType, AssignmentRule } from './rules';
+import { CaretValueRule, OnlyRuleType, AssignmentRule, Rule } from './rules';
 import { findLast } from 'lodash';
 
 export function typeString(types: OnlyRuleType[]): string {
@@ -52,6 +52,22 @@ export function fshifyString(input: string): string {
     .replace(/\t/g, '\\t');
 }
 
+// Looking up a rule by path (e.g., a definition's id, url, name, or version) scans every rule on the
+// definition, and the FSHTank does this for every entity it checks on every fish. For definitions with many
+// rules (e.g., large code systems) that scan dominated the build, so the rule found by each lookup is cached
+// against the definition's rules array. Rules arrays are only appended to or replaced wholesale (as
+// applyInsertRules does), so the cache is checked against the array's length and last rule. The rule rather
+// than its value is cached, so a change to the rule's value is still seen, and a cached rule is re-checked
+// against the lookup in case its path was changed in place (as CodeSystemExporter does to code caret rules).
+// Replacing a rule in the middle of the array, or changing a rule so that it newly matches a lookup, would
+// not be seen; no code does either.
+type RuleLookupCache = {
+  length: number;
+  lastRule: Rule;
+  rulesByLookup: Map<string, AssignmentRule | CaretValueRule>;
+};
+const ruleLookupCache = new WeakMap<Rule[], RuleLookupCache>();
+
 export function findAssignmentByPath(
   fshDefinition:
     | Profile
@@ -67,20 +83,33 @@ export function findAssignmentByPath(
   caretRulePath: string,
   caretRuleCaretPath: string
 ) {
+  const rules: Rule[] = fshDefinition.rules;
+  const lastRule = rules[rules.length - 1];
+  let cache = ruleLookupCache.get(rules);
+  if (cache == null || cache.length !== rules.length || cache.lastRule !== lastRule) {
+    cache = { length: rules.length, lastRule, rulesByLookup: new Map() };
+    ruleLookupCache.set(rules, cache);
+  }
+  let key: string;
+  let matches: (rule: Rule) => boolean;
   if (fshDefinition instanceof Instance || fshDefinition instanceof Invariant) {
-    return findLast(
-      fshDefinition.rules,
-      rule => rule instanceof AssignmentRule && rule.path === assignmentRulePath
-    ) as AssignmentRule;
+    key = `assignment|${assignmentRulePath}`;
+    matches = rule => rule instanceof AssignmentRule && rule.path === assignmentRulePath;
   } else {
-    return findLast(
-      fshDefinition.rules,
-      rule =>
-        rule instanceof CaretValueRule &&
-        rule.path === caretRulePath &&
-        rule.caretPath === caretRuleCaretPath
-    ) as CaretValueRule;
+    key = `caret|${caretRulePath}|${caretRuleCaretPath}`;
+    matches = rule =>
+      rule instanceof CaretValueRule &&
+      rule.path === caretRulePath &&
+      rule.caretPath === caretRuleCaretPath;
+  }
+  const cachedRule = cache.rulesByLookup.get(key);
+  // a cached rule is re-checked in case its path changed in place; a cached miss is trusted as-is
+  if (cache.rulesByLookup.has(key) && (cachedRule == null || matches(cachedRule))) {
+    return cachedRule;
   }
+  const foundRule = findLast(rules, matches) as AssignmentRule | CaretValueRule;
+  cache.rulesByLookup.set(key, foundRule);
+  return foundRule;
 }
 
 /**

test/export/CodeSystemExporter.test.ts

@@ -1211,6 +1211,87 @@ describe('CodeSystemExporter', () => {
     });
   });
 
+  it('should apply several caret rules to the same extension slice and to a numerically indexed extension', () => {
+    // CodeSystem: ExtensionSystem
+    // * ^extension[structuredefinition-fmm].valueInteger = 1
+    // * ^extension[structuredefinition-fmm].id = "fmm"
+    // * ^extension[1].url = "http://example.org/StructureDefinition/plain"
+    // * ^extension[1].valueString = "plain"
+    // * #bar "Bar"
+    // * #bar ^extension[structuredefinition-fmm].valueInteger = 2
+    // * #bar ^extension[structuredefinition-fmm].id = "concept-fmm"
+    // * #bar ^extension[1].url = "http://example.org/StructureDefinition/plain"
+    // * #bar ^extension[1].valueString = "concept plain"
+    const codeSystem = new FshCodeSystem('ExtensionSystem');
+    const fmmRule = new CaretValueRule('');
+    fmmRule.caretPath = 'extension[structuredefinition-fmm].valueInteger';
+    fmmRule.value = 1;
+    const fmmIdRule = new CaretValueRule('');
+    fmmIdRule.caretPath = 'extension[structuredefinition-fmm].id';
+    fmmIdRule.value = 'fmm';
+    const plainUrlRule = new CaretValueRule('');
+    plainUrlRule.caretPath = 'extension[1].url';
+    plainUrlRule.value = 'http://example.org/StructureDefinition/plain';
+    const plainValueRule = new CaretValueRule('');
+    plainValueRule.caretPath = 'extension[1].valueString';
+    plainValueRule.value = 'plain';
+    const conceptRule = new ConceptRule('bar', 'Bar');
+    const conceptFmmRule = new CaretValueRule('');
+    conceptFmmRule.pathArray = ['#bar'];
+    conceptFmmRule.caretPath = 'extension[structuredefinition-fmm].valueInteger';
+    conceptFmmRule.value = 2;
+    const conceptFmmIdRule = new CaretValueRule('');
+    conceptFmmIdRule.pathArray = ['#bar'];
+    conceptFmmIdRule.caretPath = 'extension[structuredefinition-fmm].id';
+    conceptFmmIdRule.value = 'concept-fmm';
+    const conceptPlainUrlRule = new CaretValueRule('');
+    conceptPlainUrlRule.pathArray = ['#bar'];
+    conceptPlainUrlRule.caretPath = 'extension[1].url';
+    conceptPlainUrlRule.value = 'http://example.org/StructureDefinition/plain';
+    const conceptPlainValueRule = new CaretValueRule('');
+    conceptPlainValueRule.pathArray = ['#bar'];
+    conceptPlainValueRule.caretPath = 'extension[1].valueString';
+    conceptPlainValueRule.value = 'concept plain';
+    codeSystem.rules.push(
+      fmmRule,
+      fmmIdRule,
+      plainUrlRule,
+      plainValueRule,
+      conceptRule,
+      conceptFmmRule,
+      conceptFmmIdRule,
+      conceptPlainUrlRule,
+      conceptPlainValueRule
+    );
+    doc.codeSystems.set(codeSystem.name, codeSystem);
+    const exported = exporter.export().codeSystems;
+    expect(exported.length).toBe(1);
+    expect(exported[0].extension).toEqual([
+      {
+        id: 'fmm',
+        url: 'http://hl7.org/fhir/StructureDefinition/structuredefinition-fmm',
+        valueInteger: 1
+      },
+      {
+        url: 'http://example.org/StructureDefinition/plain',
+        valueString: 'plain'
+      }
+    ]);
+    expect(exported[0].concept[0].extension).toEqual([
+      {
+        id: 'concept-fmm',
+        url: 'http://hl7.org/fhir/StructureDefinition/structuredefinition-fmm',
+        valueInteger: 2
+      },
+      {
+        url: 'http://example.org/StructureDefinition/plain',
+        valueString: 'concept plain'
+      }
+    ]);
+    expect(loggerSpy.getAllMessages('error')).toHaveLength(0);
+    expect(loggerSpy.getAllMessages('warn')).toHaveLength(0);
+  });
+
   it('should output an error when a choice element has values assigned to more than one choice type', () => {
     const codeSystem = new FshCodeSystem('MultiChoiceSystem')
       .withFile('MultipleChoice.fsh')

test/export/ValueSetExporter.test.ts

@@ -2659,6 +2659,44 @@ describe('ValueSetExporter', () => {
     expect(loggerSpy.getAllMessages('warn')).toHaveLength(0);
   });
 
+  it('should apply several caret rules to the same extension slice and to a numerically indexed extension', () => {
+    // ValueSet: ExtensionVS
+    // * ^extension[structuredefinition-fmm].valueInteger = 1
+    // * ^extension[structuredefinition-fmm].id = "fmm"
+    // * ^extension[1].url = "http://example.org/StructureDefinition/plain"
+    // * ^extension[1].valueString = "plain"
+    const valueSet = new FshValueSet('ExtensionVS');
+    const fmmRule = new CaretValueRule('');
+    fmmRule.caretPath = 'extension[structuredefinition-fmm].valueInteger';
+    fmmRule.value = 1;
+    const fmmIdRule = new CaretValueRule('');
+    fmmIdRule.caretPath = 'extension[structuredefinition-fmm].id';
+    fmmIdRule.value = 'fmm';
+    const plainUrlRule = new CaretValueRule('');
+    plainUrlRule.caretPath = 'extension[1].url';
+    plainUrlRule.value = 'http://example.org/StructureDefinition/plain';
+    const plainValueRule = new CaretValueRule('');
+    plainValueRule.caretPath = 'extension[1].valueString';
+    plainValueRule.value = 'plain';
+    valueSet.rules.push(fmmRule, fmmIdRule, plainUrlRule, plainValueRule);
+    doc.valueSets.set(valueSet.name, valueSet);
+    const exported = exporter.export().valueSets;
+    expect(exported.length).toBe(1);
+    expect(exported[0].extension).toEqual([
+      {
+        id: 'fmm',
+        url: 'http://hl7.org/fhir/StructureDefinition/structuredefinition-fmm',
+        valueInteger: 1
+      },
+      {
+        url: 'http://example.org/StructureDefinition/plain',
+        valueString: 'plain'
+      }
+    ]);
+    expect(loggerSpy.getAllMessages('error')).toHaveLength(0);
+    expect(loggerSpy.getAllMessages('warn')).toHaveLength(0);
+  });
+
   it('should apply a CaretValueRule that assigns an inline Instance', () => {
     // ValueSet: BreakfastVS
     // Title: "Breakfast Values"