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

Improve performance of json lookups in case of errors #886

Merged
merged 1 commit into from
Jun 4, 2023

Conversation

vdebergue
Copy link
Contributor

@vdebergue vdebergue commented May 31, 2023

Pull Request Checklist

  • Have you read through the contributor guidelines?
  • Have you squashed your commits?
  • Have you added copyright headers to new files?
  • Have you updated the documentation?
  • Have you added tests for any changed functionality?

Purpose

Improve performance of json lookup \ when encountering bad json.

Background Context

When a bad json was encountered, the json object was copied in the error message. This message could often be discarded, when using asOpt for instance.
The issue was especially present when parsing big json objects (> 10MB). As the program was busy serializing the json into a string.
Here we still give the user the available keys in the object to be helpful.

# BEFORE

[info] Benchmark                 (size)   Mode  Cnt      Score      Error   Units
[info] JsLookupBench.badLookup        1  thrpt   10    981.947 ±  443.761  ops/ms
[info] JsLookupBench.badLookup       20  thrpt   10    360.393 ±   39.503  ops/ms
[info] JsLookupBench.badLookup      100  thrpt   10     94.659 ±    9.231  ops/ms
[info] JsLookupBench.goodLookup       1  thrpt   10  32872.983 ± 2745.549  ops/ms
[info] JsLookupBench.goodLookup      20  thrpt   10  25447.611 ± 7564.005  ops/ms
[info] JsLookupBench.goodLookup     100  thrpt   10  30841.130 ±  984.572  ops/ms

# AFTER

[info] JsLookupBench.badLookup        1  thrpt   10   6949.590 ± 770.618  ops/ms
[info] JsLookupBench.badLookup       20  thrpt   10   1644.751 ±  12.360  ops/ms
[info] JsLookupBench.badLookup      100  thrpt   10    393.848 ±   5.391  ops/ms
[info] JsLookupBench.goodLookup       1  thrpt   10  35988.524 ± 645.783  ops/ms
[info] JsLookupBench.goodLookup      20  thrpt   10  33995.451 ± 504.901  ops/ms
[info] JsLookupBench.goodLookup     100  thrpt   10  31899.440 ± 477.421  ops/ms

References

Are there any relevant issues / PRs / mailing lists discussions?

Copy link
Member

@mkurz mkurz left a comment

Choose a reason for hiding this comment

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

LGTM

@mkurz mkurz merged commit d53f724 into playframework:main Jun 4, 2023
14 checks passed
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

2 participants