Improve performance with large CodeSystems - #1637
Open
vadi2 wants to merge 4 commits into
Open
Conversation
setPropertyOnDefinitionInstance rebuilt the instance's StructureDefinition for every rule. A CodeSystem with thousands of concepts has tens of thousands of code caret rules, so that dominated the build. The exporters now pass in the StructureDefinition they already built, concepts are indexed by code instead of scanned, and filling a large array no longer restarts from index 0. Claude-Session: https://claude.ai/code/session_01AKNbxASp6vABedSLJMGW97
findAssignmentByPath scanned every rule on a definition, and the tank does that for every entity on every fish. Cache the result per rules array, invalidating when rules are added or removed or the array is replaced. Claude-Session: https://claude.ai/code/session_01AKNbxASp6vABedSLJMGW97
CodeSystemExporter rewrites a code caret rule's path in place, so a cached rule may stop matching its lookup. Re-check it on a hit and rescan if it no longer matches. Also clarify comments and add tests for the Instance branch and for reusing the StructureDefinition across extension caret rules. Claude-Session: https://claude.ai/code/session_01AKNbxASp6vABedSLJMGW97
Rename the cache for what it holds, return early on a cache hit, keep the original conceptIndices local in findConceptPath, and flatten the array fill branches. No behavior change. Claude-Session: https://claude.ai/code/session_01AKNbxASp6vABedSLJMGW97
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
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.
Description:
SUSHI slows down sharply as a CodeSystem grows. Profiling a 9,347-concept CodeSystem (about 56,000 code caret rules for designations and properties) showed two costs that grow with the number of rules, so this PR fixes both:
Rebuilding the StructureDefinition for every caret rule.
setPropertyOnDefinitionInstancecalledinstance.getOwnStructureDefinition(fisher)(a package lookup plusStructureDefinition.fromJSON) on each call. For a CodeSystem that is once per code caret rule, and it was 83% of the build. The function now takes the StructureDefinition as an optional final parameter (default unchanged), andCodeSystemExporterandValueSetExporterpass in the one they already built. Sharing one StructureDefinition across an entity's rules has precedent: the validation pass in these exporters already did it, andInstanceExporterreuses cached StructureDefinitions across all of an instance's rules. In the same code path,findConceptPathandsetConceptsindex concepts by code instead of scanning the concept list for every rule, andsetPropertyOnInstancefills a large array from its current end instead of from index 0 (the old loop re-read the array length each iteration, and each iteration pushed one element, so starting at the current length adds exactly the same elements).Scanning every rule to find a definition's id, url, name, or version.
findAssignmentByPathdid afindLastover all rules, andFSHTank.fishdoes that for every entity it checks on every fish. In an IG with several large CodeSystems, every lookup of a Reference or code system walked tens of thousands of rules, which was 63% of what remained after fix 1. The found rule is now cached per rules array. Rules arrays are only appended to or replaced wholesale (asapplyInsertRulesdoes), so the cache is invalidated when the array is replaced, when its length changes, or when its last rule changes. Because the rule rather than its value is cached, changes to a rule's value are still seen, and a cached rule is re-checked against the lookup on each hit becauseCodeSystemExporterrewrites a code caret rule'spathin place once it resolves the concept.Measurements (Node 22,
sushion the same machine):For comparison, the same IG with those 32 CodeSystems pre-rendered to JSON in
input/(the workaround the project uses today) builds in 31 s. Most of the remaining gap is ANTLR parsing of the large FSH files.Output is unchanged: the generated JSON for the 9,347-concept CodeSystem is byte-identical to master, and every other resource in the IG is identical to the pre-rendered build.
Developed with AI assistance (Claude Code). The diff, tests, and measurements were reviewed before submitting.
Testing Instructions:
Run
npm run check. New tests intest/fshtypes/FshCodeSystem.test.ts,test/fshtypes/Instance.test.tsandtest/import/FSHTank.test.tscover the cache being invalidated when rules are added, removed, replaced, their values change, or a code caret rule's path is resolved. New tests intest/export/CodeSystemExporter.test.tsandtest/export/ValueSetExporter.test.tspin the output when several caret rules target the same extension slice and a numerically indexed extension, since the shared StructureDefinition now carries the slices added by earlier rules.To see the performance difference, run
sushion a project containing a CodeSystem with a few thousand concepts that each have a designation or property.Related Issue:
None.
https://claude.ai/code/session_01AKNbxASp6vABedSLJMGW97