Json factory methods are very inefficient

[barchetta]

The Json factory methods (createObjectBuilder​(), createArrayBuilder​(), etc) are very inefficient because they call JsonProvider.provider() every time and do not cache the result. Some benchmarking we did showed these methods to be an order of magnitude slower than using JsonBuilderFactory (which essentially reuses the same provider).

These Json factory methods are an attractive nuisance because they are very tempting to use (convenient) but result in terrible performance.

Is there any reason they do not cache and re-use the provider? That would fix this issue. If for some reason that's not possible then there should at least be a warning in the javadoc steering developers to use JsonBuilderFactory.

Workaround for Builders and Readers
Use JsonBuilderFactory:

private static final JsonBuilderFactory jsonFactory = Json.createBuilderFactory(null);

. . .

JsonObject jo = jsonFactory.createObjectBuilder()
                .add(. . .

Workaround for createValue()

It looks like Json.createValue() suffers the same problem. I suppose a workaround for that is:

    private static final JsonProvider provider = JsonProvider.provider();

   JsonValue value = provider.createValue(5);

[lukasj]

the reason here was problematic caching in server environment. EE compliant server must provide some implementation which is typically loaded by bootstrap/server root classloader but also allow usage of different implementation which may be part of some web application (thus loaded by webapp classloader). There is also related #26

[jjspiegel]

Not only are they slow, but they will also likely cause a global lock when the ServiceLoader is invoked. I have seen people get burned by this in both JSON-P and JAXP.

But I agree with Lukas - at this point, I don't see what can be done other than improving the javadoc to warn people of this.

[leadpony]

Hi everyone.
All of the API methods shoud be changed from class methods into instance methods of Json or Jsonp in JSON-P 2.0, like as Jsonb interface in JSON-B does.
Json#createPointer() is slow too and it is unavoidable in the current API design.

[jjspiegel]

I had an additional thought - what would be useful is if here:
https://github.com/javaee/jsonp/blob/master/api/src/main/java/javax/json/spi/JsonProvider.java#L90

There were an optional system property look up OVERRIDE_PROVIDER, or something like that, which statically loads a JsonProvider instance to be used universally. This would avoid the provider lookup when specified.

This would allow someone who knows they only have the RI in the classpath to fix this performance trap if found in production without having to write additional code to fix the problem.

This would also allow products with many developers to set this property in advance and guard against accidental future usage of these methods.

[lukasj]

@jjspiegel do you mean to define implementation discovery ie following way? :

Implementation discovery consists of following steps in the order specified (first successful resolution applies):

1. If the system property jakarta.json.spi.JsonProvider exists, then its value is assumed to be the provider factory class.
2. Provider of jakarta.json.spi.JsonProvider is loaded using the service-provider loading facilities, as defined by Java SE Platform, to attempt to locate and load an implementation of the service.
3. If all of the steps above fail, then the rest of the look up is unspecified.

[jjspiegel]

@lukasj yes. And if possible, the Class.forName() on the value would happen outside of the method (static block) to avoid frequent class loading.

[gsmet]

Hi. While I can understand the concerns about multi-deployment, I just wanted to add some numbers so that people can understand how massive the problem is and why it should be solved.

I did some JMH benchmarking this week-end as part of some work I was doing on Quarkus and here are the results:

Json.createValue("test"):

Iteration   1: 153.907 ops/ms
Iteration   2: 155.308 ops/ms
Iteration   3: 154.542 ops/ms
Iteration   4: 155.963 ops/ms
Iteration   5: 156.462 ops/ms

Using a static provider with JSON_PROVIDER.createValue("test"):

Iteration   1: 1112305.967 ops/ms
Iteration   2: 1114183.052 ops/ms
Iteration   3: 1113783.793 ops/ms
Iteration   4: 1111395.048 ops/ms
Iteration   5: 1112449.325 ops/ms

That's 7200+ times faster.

I think it's extremely dangerous to provide a cute shortcut (in the form of the Json class) that is so inefficient.

Now, not saying there's an easy way out given all this is API but it looks like something that needs to be addressed.

[jungm]

EE compliant server must provide some implementation which is typically loaded by bootstrap/server root classloader but also allow usage of different implementation which may be part of some web application (thus loaded by webapp classloader)

This got me thinking: Shouldn't it be possible to cache the instance per ClassLoader in a WeakHashMap maybe?
This way the expensive lookup only has to be done once per application/CL instead of on every invocation