Cache dictionary load and parsed-expression trees for repeated parses#4
Open
ghedwards wants to merge 1 commit into
Open
Cache dictionary load and parsed-expression trees for repeated parses#4ghedwards wants to merge 1 commit into
ghedwards wants to merge 1 commit into
Conversation
- DictionaryManager: trigger the default dictionary load in a static initializer, not just on first explicit call. This lets AOT tooling (e.g. GraalVM native-image's --initialize-at-build-time) bake the parsed dictionaries into the build-time heap snapshot instead of re-parsing them on every process start. initDictionaries() itself is unchanged and remains safe/idempotent to call directly. - CFMLParser.parseCFMLExpression: cache the ANTLR parse tree by source text, keyed on the exact expression string. Callers that re-parse textually-identical short expressions many times (e.g. CFLint scanning a file's <cfset>/<cfif> tags one at a time - common boilerplate like "var result = StructNew()" can repeat dozens of times in one file) skip the expensive lex/parse/ATN simulation on a cache hit. Deliberately does NOT cache/share the resulting CFExpression itself, since it has mutable state (CFParsedStatement.setParent()) that callers rely on per-use - every hit gets a fresh visit() over the cached (read-only) parse tree instead, so there's no risk of cross-contamination between call sites.
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.
Summary
DictionaryManager: moved the default dictionary load into astaticinitializer (in addition to the existing lazyinitDictionaries()entry point, which is otherwise unchanged and remains idempotent). Currently the dictionary XML is re-parsed from scratch on every process start; this change lets AOT tooling (GraalVM native-image's--initialize-at-build-time) bake the already-parsed dictionaries into the build-time heap snapshot instead. Verified: with this change +--initialize-at-build-time=cfml.dictionaryset on a downstream GraalVM native-image build, dictionary construction dropped to ~25ms with correct output, and the build itself succeeds cleanly.CFMLParser.parseCFMLExpression: added a parse-tree cache keyed by the exact source text. Callers that re-parse textually-identical short expressions many times over (verified against a real ~8,700-line.cfcvia CFLint: 2,619 calls to this method, only 1,786 distinct strings - e.g. boilerplate likevar result = StructNew()repeated 43 times) now skip the lex/parse/ATN-simulation step on a cache hit. Deliberately does not cache/share the resultingCFExpressionobject itself -CFParsedStatementhas mutable state (setParent()) that downstream callers read/rely on per-use (confirmed: CFLint'sImplicitScopeCheckerstores expression references for deferred end-of-scope processing), so sharing the object across call sites would risk cross-contamination. Every cache hit gets a freshvisit()over the cached (read-only) parse tree instead, avoiding that risk entirely.Test plan
mvn installoncfml.dictionary+cfml.parsing): 287 tests, 0 failures, 4 skipped (pre-existing skips, unrelated).TestDictionaryManager.testExternalDictionaryLocation(exercises the custom-DictionaryPreferencesoverride path) passes - confirms the static initializer doesn't interfere with explicitinitDictionaries(prefs)calls for non-default dictionaries.