Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize cases with long potential simple_keys #555

Merged
merged 3 commits into from Jan 21, 2020
Merged

Conversation

cjcullen
Copy link

When we build up the simple_keys stack, we count on the (formerly named) staleness check to catch errors where a simple key is required but would be > 1024 chars or span lines. The previous simplification that searches the stack from the top can go 1024 keys deep before finding a "stale" key and stopping. I added a test that shows that this consumes ~3s per 1MB of document size.

I split that staleness check back out into a separate loop so that we don't unnecessarily re-process a bunch of keys just to get down to the ones that might have gone stale.

$ benchcmp old.txt new.txt
benchmark                                old ns/op      new ns/op     delta
Benchmark1000KB100Aliases-6              881167484      872120600     -1.03%
Benchmark1000KBDeeplyNestedSlices-6      48761251       5274819       -89.18%
Benchmark1000KBDeeplyNestedMaps-6        50438114       5292240       -89.51%
Benchmark1000KBDeeplyNestedIndents-6     4385726        4280545       -2.40%
Benchmark1000KB1000IndentLines-6         435702849      432047937     -0.84%
Benchmark1KBMaps-6                       574312         588420        +2.46%
Benchmark10KBMaps-6                      5727272        5895964       +2.95%
Benchmark100KBMaps-6                     52126341       52920733      +1.52%
Benchmark1000KBMaps-6                    484546095      474623653     -2.05%
BenchmarkDeepSlice-6                     455902294      397041567     -12.91%
BenchmarkDeepFlow-6                      399613786      407783513     +2.04%
Benchmark1000KBMaxDepthNested-6          2700904018     447527143     -83.43%

benchmark                                old allocs     new allocs     delta
Benchmark1000KB100Aliases-6              5832425        5832415        -0.00%
Benchmark1000KBDeeplyNestedSlices-6      9066           10081          +11.20%
Benchmark1000KBDeeplyNestedMaps-6        9071           10087          +11.20%
Benchmark1000KBDeeplyNestedIndents-6     10082          10082          +0.00%
Benchmark1000KB1000IndentLines-6         4093516        4093516        +0.00%
Benchmark1KBMaps-6                       3126           3126           +0.00%
Benchmark10KBMaps-6                      30780          30780          +0.00%
Benchmark100KBMaps-6                     307269         307269         +0.00%
Benchmark1000KBMaps-6                    3072079        3072079        +0.00%
BenchmarkDeepSlice-6                     2048121        2048112        -0.00%
BenchmarkDeepFlow-6                      1978104        1978103        -0.00%
Benchmark1000KBMaxDepthNested-6          4079998        4079989        -0.00%

benchmark                                old bytes     new bytes     delta
Benchmark1000KB100Aliases-6              393118344     392792852     -0.08%
Benchmark1000KBDeeplyNestedSlices-6      4689465       4576336       -2.41%
Benchmark1000KBDeeplyNestedMaps-6        4689831       4577450       -2.40%
Benchmark1000KBDeeplyNestedIndents-6     2969924       2969913       -0.00%
Benchmark1000KB1000IndentLines-6         143718512     143718512     +0.00%
Benchmark1KBMaps-6                       218984        218984        +0.00%
Benchmark10KBMaps-6                      2178155       2178154       -0.00%
Benchmark100KBMaps-6                     22002796      22002807      +0.00%
Benchmark1000KBMaps-6                    220560496     220560496     +0.00%
BenchmarkDeepSlice-6                     120114888     119789000     -0.27%
BenchmarkDeepFlow-6                      115058032     115056832     -0.00%
Benchmark1000KBMaxDepthNested-6          146582888     146257064     -0.22%

scannerc.go Outdated Show resolved Hide resolved
Copy link
Contributor

@niemeyer niemeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for keeping it up.

Here are some initial comments on the issue:

scannerc.go Outdated Show resolved Hide resolved
scannerc.go Outdated Show resolved Hide resolved
efficient lookup in yaml_parser_fetch_value().
@cjcullen
Copy link
Author

Thanks again for the first pass. Are you up for considering the addition of the single_key index? I'd like to have something to include in the next set of Kubernetes patch releases around the first week of January.

@cjcullen cjcullen requested a review from niemeyer January 7, 2020 22:23
@niemeyer niemeyer merged commit 53403b5 into go-yaml:v2 Jan 21, 2020
niemeyer added a commit that referenced this pull request Jan 21, 2020
Message from original commit (53403b5):

This change introduces an index to lookup token numbers referenced by simple_keys in O(1),
thus significantly reducing the performance impact of certain abusively constructed snippets.

When we build up the simple_keys stack, we count on the (formerly named) staleness check to
catch errors where a simple key is required but would be > 1024 chars or span lines. The previous
simplification that searches the stack from the top can go 1024 keys deep before finding a "stale"
key and stopping. I added a test that shows that this consumes ~3s per 1MB of document size.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants