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

Rewrite and restructure JSON parser #1343

Merged
merged 12 commits into from
Mar 12, 2021
Merged

Rewrite and restructure JSON parser #1343

merged 12 commits into from
Mar 12, 2021

Conversation

qwwdfsad
Copy link
Contributor

@qwwdfsad qwwdfsad commented Feb 17, 2021

Benchmark improvements for throughput (Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz, libraries-linux-perf-unit-877):

CitmBenchmark.decodeCitm                            diff   +23%
CoerceInputValuesBenchmark.testNonNullableCoercing  diff   +30%
JacksonComparisonBenchmark.kotlinFromString         diff   +31% 
TwitterBenchmark.parseTwitter                       diff   +55% 
TwitterFeedBenchmark.parseTwitter                   diff   +55%

* Get rid of spontaneous lookahead
* Spill fewer variables into object state
* Optimize token and whitespaces reading
* Separate fast and slow paths
* Add optimistic key consumption optimization to leverage indexOf intrinsic
* Improve exception messages in few places

All tests except lenient boolean should pass

Benchmark difference for throughput (Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz, libraries-linux-perf-unit-877):

CitmBenchmark.decodeCitm                                                diff   +8%
CoerceInputValuesBenchmark.testNonNullableCoercing  diff   +11%
CoerceInputValuesBenchmark.testNonNullableRegular    diff   +5%
CoerceInputValuesBenchmark.testNullableCoercing         diff   +7%
CoerceInputValuesBenchmark.testNullableRegular           diff   +13%
JacksonComparisonBenchmark.kotlinFromString             diff   +16% (noisy, JUT-dependable)
TwitterBenchmark.parseTwitter                                           diff   +26%
TwitterFeedBenchmark.parseTwitter                                  diff   +30%
@qwwdfsad qwwdfsad force-pushed the json-performance branch 4 times, most recently from 7117dcd to 269d3d3 Compare February 18, 2021 12:43
…here they are not really necessary

It gives solid 5-10% of throughpuut
@qwwdfsad qwwdfsad force-pushed the json-performance branch 6 times, most recently from 01b907b to aa4ae97 Compare February 18, 2021 18:23
@qwwdfsad qwwdfsad marked this pull request as ready for review February 18, 2021 18:27
@qwwdfsad qwwdfsad requested a review from sandwwraith February 18, 2021 18:27
    * Replace handrolled char-array with StringBuilder, tweak it here and there
    * Always use optimistic path for strings
    * Properly handle slow-path from the middle of a string
    * Better exception messages and hints
    * Minor improvements
    * More comments
@qwwdfsad qwwdfsad requested a review from sandwwraith February 26, 2021 16:57
@qwwdfsad qwwdfsad requested a review from sandwwraith March 4, 2021 12:18
Copy link
Member

@sandwwraith sandwwraith left a comment

Choose a reason for hiding this comment

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

Great job!

NB: it just came to my mind that JsonLexer is far more better name than JsonReader (since we already have JsonParser). If you feel the same, you can rename it as a part of this PR.

# Conflicts:
#	benchmark/src/jmh/kotlin/kotlinx/benchmarks/json/TwitterBenchmark.kt
@qwwdfsad qwwdfsad merged commit 628c973 into dev Mar 12, 2021
@qwwdfsad qwwdfsad deleted the json-performance branch March 12, 2021 09:12
@avently
Copy link

avently commented Jul 18, 2021

@qwwdfsad my app heavily utilize CPU - ~15% from all cores and more shows in Htop. Even worse on mobile devices.
99% of CPU tasks are reading from web socket and parsing via Kotlinx.Serialization.

After your two PRs CPU usage was reduced to ~10%. It's awesome!

Thank you for your findings!

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.

4 participants