Avoid reparsing KeyValues lookahead tokens - #2090
Draft
Jason-Doyle wants to merge 1 commit into
Draft
Jason-Doyle wants to merge 1 commit into
Jason-Doyle wants to merge 1 commit into
Conversation
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
KeyValues::RecursiveLoadFromBufferreads one token ahead after each scalar value. It uses that token to check for a conditional tag.When the token is not a conditional, the current code rewinds the buffer. The next loop iteration reads the same token again.
This change carries that token into the next iteration. It also preserves whether the token was quoted. Conditional handling continues through the existing path.
Behaviour
The insertion path is unchanged.
CreateKeyUsingKnownLastChildstill appends each key to the same linked list.Value parsing is unchanged. Nested sections use the existing recursion path. There are no changes to public types or function signatures.
The baseline and changed parsers produced byte-identical serialized output for both validation corpora.
Measurement
The main workload was the current public TF2
items_game.txtschema.29F3ED79F01E063699876002C10442D8D9FAFBEE5B06F1A58E3F7DD4FA2301B5The schema contains no unquoted conditional tokens. The current parser therefore takes the read-and-rewind path after all 141,551 scalar values.
The test used Release x64 builds with MSVC 19.29. Each process completed three warm-up parses before measurement. Execution order alternated between the baseline and changed binaries.
The highest changed process median was lower than the lowest baseline process median. Execution order did not produce a material difference.
A separate mixed-value corpus exercised scalar parsing and nested structure. It included conditional tags and comments. The changed parser was faster in all 20 paired comparisons. Its median reduction was 21.01%.
Control workloads
A separate corpus contained 100,000 unique scalar keys. It had no duplicate key names. The changed parser was faster in all 20 paired comparisons, with a median reduction of 7.10%.
I also tested two repository files that contain conditional tags:
HudLayout.resMatchMakingDashboard.resBoth files produced byte-identical serialized output.
Two workloads exercised cases where the pending-token path did not provide its main benefit:
The slower pairs differed by at most 0.16%. An empty document produced no reliable difference because its confidence interval crossed zero.
The Release x64 stack frame grew by 16 bytes per active parser recursion level. The existing recursion limit bounds the maximum additional stack use at about 1.6 KB. Generated function code grew by 88 bytes. The
KeyValuesobject size is unchanged, and the change adds no heap allocation.Output validation
The serialized TF2 trees were byte-identical:
9290072EABFA724BF67CC6E5D858415B4793B44D65E57FA0F053382439AB46CBThe serialized mixed-corpus trees were also byte-identical:
B5D36CB1B6F655D9B441911CCF4650A9D35613A5C2F748381B54BBE412BFA4E1The benchmark harness is available with the raw timing results:
https://gist.github.com/Jason-Doyle/6f2906b0f00c1b7b965db07b946c3ae3
Limits
These measurements cover KeyValues parser time on one system. They do not establish the same reduction in total game startup time.