tdunning / open-json

Open JSON - a truly open source JSON implementation
Apache License 2.0
18 stars 18 forks source link

Speed up serialization of String values in JSONObject #13

Closed sebastian-nagel closed 5 years ago

sebastian-nagel commented 5 years ago

When a JSONObject which holds String objects as values is serialized, for every object value is the class name is checked whether it contains "JSONFunc" (see #1 resp. 174ab93). Handling this common case beforehand speeds up the serialization of JSON objects holding simple string maps:

solomax commented 5 years ago

I thought to add pool of pre-allocated static buffers and measure the speed, but had no free time for it :(

this approach is being used here https://wiki.eclipse.org/Linux_Tools_Project/TMF/CTF_guide#JSON_CTF and works much faster

tdunning commented 5 years ago

I have no time to review this in detail. I have given @solomax permissions to deal with this pull request.

In general, I discourage the use of this package. Jackson is better in essentially every way. The point of this library was simply to help people get rid of the json.org dependency and the problematic license.

On the other hand, if a community appears out of the air who want to take this forward, I am happy for that to happen. I just can't contribute very much of my own time.

tdunning commented 5 years ago

Quick question for @sebastian-nagel:

Why are you using this package instead of something like Jackson?

(this is a serious question ... there is likely something I don't know about happening)

solomax commented 5 years ago

@sebastian-nagel could you please close this PR and create same PR for this repo https://github.com/openjson/openjson ?

sebastian-nagel commented 5 years ago

Why are you using this package instead of something like Jackson?

Issues regarding the license and performance: by switching to the Android classes the time spent for JSON serialization dropped by half. Using Jackson would require a lot more code changes and may introduce dependency issues. See the discussion in commoncrawl/ia-web-commons#16.

could you please close this PR and create same PR for this repo https://github.com/openjson/openjson ?

I'd be happy to do it. However, looks like it has been already fixed in 8db946a. This fits my observation that too much time is spent in Class.getSimpleName() (see profiler screenshot). The fix in 8db946a is more general and I cannot see any improvements if I apply my shortcut for String values on top of it and run the benchmarks.

Closing this PR. Thanks for pointing me to the openjson project!

open-json JSONStringer.value()

tdunning commented 5 years ago

Thanks for the contribution in any case!