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

Remove jakarta.json.Json usage for performance reasons #42773

Merged
merged 7 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions docs/src/main/asciidoc/_includes/json-provider-note.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
[WARNING]
====
If you use JSON-B and JSON-P, make sure you don't use the shortcut methods offered by `jakarta.json.Json` such as `Json.createValue(...)`.

Check warning on line 3 in docs/src/main/asciidoc/_includes/json-provider-note.adoc

View workflow job for this annotation

GitHub Actions / Linting with Vale

[vale] reported by reviewdog 🐶 [Quarkus.TermsWarnings] Consider using 'verify' rather than 'make sure' unless updating existing content that uses the term. Raw Output: {"message": "[Quarkus.TermsWarnings] Consider using 'verify' rather than 'make sure' unless updating existing content that uses the term.", "location": {"path": "docs/src/main/asciidoc/_includes/json-provider-note.adoc", "range": {"start": {"line": 3, "column": 31}}}, "severity": "WARNING"}

At the moment, any single call to these methods will https:/jakartaee/jsonp-api/issues/154[initialize a new `JsonProvider`], which is extremely slow.
Quarkus provides a shared `JsonProvider` instance via the `JsonProviderHolder` class of the `quarkus-jsonp` extension.

Check warning on line 6 in docs/src/main/asciidoc/_includes/json-provider-note.adoc

View workflow job for this annotation

GitHub Actions / Linting with Vale

[vale] reported by reviewdog 🐶 [Quarkus.TermsWarnings] Consider using 'through', 'by', 'from', 'on', or 'by using' rather than 'via' unless updating existing content that uses the term. Raw Output: {"message": "[Quarkus.TermsWarnings] Consider using 'through', 'by', 'from', 'on', or 'by using' rather than 'via' unless updating existing content that uses the term.", "location": {"path": "docs/src/main/asciidoc/_includes/json-provider-note.adoc", "range": {"start": {"line": 6, "column": 51}}}, "severity": "WARNING"}

You can import it as a static import to simplify your code:

Check warning on line 8 in docs/src/main/asciidoc/_includes/json-provider-note.adoc

View workflow job for this annotation

GitHub Actions / Linting with Vale

[vale] reported by reviewdog 🐶 [Quarkus.TermsSuggestions] Depending on the context, consider using 'because' or 'while' rather than 'as'. Raw Output: {"message": "[Quarkus.TermsSuggestions] Depending on the context, consider using 'because' or 'while' rather than 'as'.", "location": {"path": "docs/src/main/asciidoc/_includes/json-provider-note.adoc", "range": {"start": {"line": 8, "column": 19}}}, "severity": "INFO"}

[source,java]
----
import static io.quarkus.jsonp.JsonProviderHolder.jsonProvider;

[...]

public void method() {
jsonProvider().createValue("value");
}

[...]
----
====
4 changes: 2 additions & 2 deletions docs/src/main/asciidoc/rest-json.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,10 @@ and in particular adds the following dependency:
implementation("io.quarkus:quarkus-rest-jsonb")
----

include::{includes}/json-provider-note.adoc[]

[NOTE]
====
While named "reactive", Quarkus REST supports equally well both traditional blocking patterns and reactive patterns.

For more information about Quarkus REST, please refer to the xref:rest.adoc[dedicated guide].
====

Expand Down
2 changes: 2 additions & 0 deletions docs/src/main/asciidoc/smallrye-graphql-client.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,11 @@
implementation("io.quarkus:quarkus-smallrye-graphql-client")
----

include::{includes}/json-provider-note.adoc[]

== The application

The application we will build makes use of both types of GraphQL clients. In both cases,

Check warning on line 96 in docs/src/main/asciidoc/smallrye-graphql-client.adoc

View workflow job for this annotation

GitHub Actions / Linting with Vale

[vale] reported by reviewdog 🐶 [Quarkus.SentenceLength] Try to keep sentences to an average of 32 words or fewer. Raw Output: {"message": "[Quarkus.SentenceLength] Try to keep sentences to an average of 32 words or fewer.", "location": {"path": "docs/src/main/asciidoc/smallrye-graphql-client.adoc", "range": {"start": {"line": 96, "column": 64}}}, "severity": "INFO"}
they will connect to the Star Wars service at https://graphql.org/swapi-graphql[SWAPI] and
query it for a list of Star Wars films, and, for each film, the names of the planets which
appear in that film.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package io.quarkus.jsonp;

import jakarta.json.spi.JsonProvider;

/**
* A utility class that holds an instance of {@link JsonProvider}.
* Should be used instead of {@link jakarta.json.Json} for performance reasons.
*
* @see <a href="https:/jakartaee/jsonp-api/issues/154">Json factory methods are very inefficient</a>
*/
public final class JsonProviderHolder {

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

private JsonProviderHolder() {
}

public static JsonProvider jsonProvider() {
return JSON_PROVIDER;
}
}
Original file line number Diff line number Diff line change
@@ -1,22 +1,7 @@
/*
* Copyright 2017 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.quarkus.micrometer.runtime.registry.json;

import static io.quarkus.jsonp.JsonProviderHolder.jsonProvider;

import java.io.StringWriter;
import java.util.ArrayList;
import java.util.Collection;
Expand All @@ -30,7 +15,6 @@
import jakarta.json.JsonObjectBuilder;
import jakarta.json.JsonValue;
import jakarta.json.JsonWriter;
import jakarta.json.spi.JsonProvider;
import jakarta.json.stream.JsonGenerator;

import io.micrometer.core.instrument.Counter;
Expand All @@ -49,13 +33,12 @@
public class JsonExporter {

private static final Map<String, ?> JSON_CONFIG = Collections.singletonMap(JsonGenerator.PRETTY_PRINTING, true);
private static final JsonProvider JSON_PROVIDER = JsonProvider.provider();

public JsonExporter() {
}

public StringBuilder exportEverything(JsonMeterRegistry meterRegistry) {
JsonObjectBuilder root = JSON_PROVIDER.createObjectBuilder();
JsonObjectBuilder root = jsonProvider().createObjectBuilder();
List<Gauge> gauges = new ArrayList<>();
List<Counter> counters = new ArrayList<>();
List<TimeGauge> timeGauges = new ArrayList<>();
Expand Down Expand Up @@ -90,7 +73,7 @@ private Map<String, JsonValue> exportGauges(Collection<Gauge> gauges) {
for (Gauge g : gauges) {
double value = g.value();
if (Double.isFinite(value)) {
result.put(createExportKey(g.getId()), JSON_PROVIDER.createValue(value));
result.put(createExportKey(g.getId()), jsonProvider().createValue(value));
}
}
return result;
Expand All @@ -101,7 +84,7 @@ private Map<String, JsonValue> exportTimeGauges(Collection<TimeGauge> timeGauges
for (TimeGauge g : timeGauges) {
double value = g.value();
if (Double.isFinite(value)) {
result.put(createExportKey(g.getId()), JSON_PROVIDER.createValue(value));
result.put(createExportKey(g.getId()), jsonProvider().createValue(value));
}
}
return result;
Expand All @@ -110,20 +93,20 @@ private Map<String, JsonValue> exportTimeGauges(Collection<TimeGauge> timeGauges
private Map<String, JsonValue> exportCounters(Collection<Counter> counters) {
return counters.stream()
.collect(Collectors.toMap(counter -> createExportKey(counter.getId()),
counter -> JSON_PROVIDER.createValue(counter.count())));
counter -> jsonProvider().createValue(counter.count())));
}

private Map<String, JsonValue> exportFunctionCounters(Collection<FunctionCounter> counters) {
return counters.stream()
.collect(Collectors.toMap(counter -> createExportKey(counter.getId()),
counter -> JSON_PROVIDER.createValue(counter.count())));
counter -> jsonProvider().createValue(counter.count())));
}

private Map<String, JsonValue> exportTimers(Collection<Timer> timers) {
Map<String, List<Timer>> groups = timers.stream().collect(Collectors.groupingBy(timer -> timer.getId().getName()));
Map<String, JsonValue> result = new HashMap<>();
for (Map.Entry<String, List<Timer>> group : groups.entrySet()) {
JsonObjectBuilder builder = JSON_PROVIDER.createObjectBuilder();
JsonObjectBuilder builder = jsonProvider().createObjectBuilder();
for (Timer timer : group.getValue()) {
builder.add(createExportKey("count", timer.getId()), timer.count());
builder.add(createExportKey("elapsedTime", timer.getId()), timer.totalTime(timer.baseTimeUnit()));
Expand All @@ -138,7 +121,7 @@ private Map<String, JsonValue> exportLongTaskTimers(Collection<LongTaskTimer> ti
.collect(Collectors.groupingBy(timer -> timer.getId().getName()));
Map<String, JsonValue> result = new HashMap<>();
for (Map.Entry<String, List<LongTaskTimer>> group : groups.entrySet()) {
JsonObjectBuilder builder = JSON_PROVIDER.createObjectBuilder();
JsonObjectBuilder builder = jsonProvider().createObjectBuilder();
for (LongTaskTimer timer : group.getValue()) {
builder.add(createExportKey("activeTasks", timer.getId()), timer.activeTasks());
builder.add(createExportKey("duration", timer.getId()), timer.duration(timer.baseTimeUnit()));
Expand All @@ -155,7 +138,7 @@ private Map<String, JsonValue> exportFunctionTimers(Collection<FunctionTimer> ti
.collect(Collectors.groupingBy(timer -> timer.getId().getName()));
Map<String, JsonValue> result = new HashMap<>();
for (Map.Entry<String, List<FunctionTimer>> group : groups.entrySet()) {
JsonObjectBuilder builder = JSON_PROVIDER.createObjectBuilder();
JsonObjectBuilder builder = jsonProvider().createObjectBuilder();
for (FunctionTimer timer : group.getValue()) {
builder.add(createExportKey("count", timer.getId()), timer.count());
builder.add(createExportKey("elapsedTime", timer.getId()), timer.totalTime(timer.baseTimeUnit()));
Expand All @@ -170,7 +153,7 @@ private Map<String, JsonValue> exportDistributionSummaries(Collection<Distributi
.collect(Collectors.groupingBy(summary -> summary.getId().getName()));
Map<String, JsonValue> result = new HashMap<>();
for (Map.Entry<String, List<DistributionSummary>> group : groups.entrySet()) {
JsonObjectBuilder builder = JSON_PROVIDER.createObjectBuilder();
JsonObjectBuilder builder = jsonProvider().createObjectBuilder();
for (DistributionSummary summary : group.getValue()) {
HistogramSnapshot snapshot = summary.takeSnapshot();
if (summary instanceof JsonDistributionSummary) {
Expand Down Expand Up @@ -199,7 +182,7 @@ private Map<String, JsonValue> exportDistributionSummaries(Collection<Distributi

private StringBuilder stringify(JsonObject obj) {
StringWriter out = new StringWriter();
try (JsonWriter writer = JSON_PROVIDER.createWriterFactory(JSON_CONFIG).createWriter(out)) {
try (JsonWriter writer = jsonProvider().createWriterFactory(JSON_CONFIG).createWriter(out)) {
writer.writeObject(obj);
}
return new StringBuilder(out.toString());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package io.quarkus.smallrye.graphql.deployment;

import static io.quarkus.jsonp.JsonProviderHolder.jsonProvider;

import java.io.IOException;
import java.io.StringReader;
import java.io.StringWriter;
import java.util.Map;
import java.util.Properties;

import jakarta.json.Json;
import jakarta.json.JsonObject;
import jakarta.json.JsonObjectBuilder;
import jakarta.json.JsonReader;
Expand Down Expand Up @@ -63,14 +64,14 @@ protected String getPayload(String query, String variables) {

protected JsonObject createRequestBody(String graphQL, String variables) {
// Create the request
JsonObject vjo = Json.createObjectBuilder().build();
JsonObject vjo = jsonProvider().createObjectBuilder().build();
if (variables != null && !variables.isEmpty()) {
try (JsonReader jsonReader = Json.createReader(new StringReader(variables))) {
try (JsonReader jsonReader = jsonProvider().createReader(new StringReader(variables))) {
vjo = jsonReader.readObject();
}
}

JsonObjectBuilder job = Json.createObjectBuilder();
JsonObjectBuilder job = jsonProvider().createObjectBuilder();
if (graphQL != null && !graphQL.isEmpty()) {
job.add(QUERY, graphQL);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package io.quarkus.smallrye.graphql.deployment;

import static io.quarkus.jsonp.JsonProviderHolder.jsonProvider;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;

import jakarta.json.Json;
import jakarta.json.JsonObject;

import org.eclipse.microprofile.metrics.MetricID;
Expand Down Expand Up @@ -71,9 +71,9 @@ private JsonObject createRequestBody(String graphQL) {
private JsonObject createRequestBody(String graphQL, JsonObject variables) {
// Create the request
if (variables == null || variables.isEmpty()) {
variables = Json.createObjectBuilder().build();
variables = jsonProvider().createObjectBuilder().build();
}
return Json.createObjectBuilder().add("query", graphQL).add("variables", variables).build();
return jsonProvider().createObjectBuilder().add("query", graphQL).add("variables", variables).build();
}

}
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package io.quarkus.smallrye.graphql.runtime;

import static io.quarkus.jsonp.JsonProviderHolder.jsonProvider;

import java.io.StringReader;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

import jakarta.json.Json;
import jakarta.json.JsonObject;
import jakarta.json.JsonReader;
import jakarta.json.JsonReaderFactory;
Expand Down Expand Up @@ -38,7 +39,7 @@ public abstract class SmallRyeGraphQLAbstractHandler implements Handler<RoutingC

private volatile ExecutionService executionService;

protected static final JsonReaderFactory jsonReaderFactory = Json.createReaderFactory(null);
protected static final JsonReaderFactory jsonReaderFactory = jsonProvider().createReaderFactory(null);

public SmallRyeGraphQLAbstractHandler(
CurrentIdentityAssociation currentIdentityAssociation,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.quarkus.smallrye.graphql.runtime;

import static io.quarkus.jsonp.JsonProviderHolder.jsonProvider;

import java.io.IOException;
import java.io.StringReader;
import java.io.UnsupportedEncodingException;
Expand All @@ -8,7 +10,6 @@
import java.util.List;
import java.util.regex.Pattern;

import jakarta.json.Json;
import jakarta.json.JsonObject;
import jakarta.json.JsonObjectBuilder;
import jakarta.json.JsonReader;
Expand Down Expand Up @@ -106,7 +107,7 @@ private void handlePost(HttpServerResponse response, RoutingContext ctx, String
JsonObject jsonObjectFromQueryParameters = getJsonObjectFromQueryParameters(ctx);
JsonObject mergedJsonObject;
if (jsonObjectFromBody != null) {
mergedJsonObject = Json.createMergePatch(jsonObjectFromQueryParameters).apply(jsonObjectFromBody)
mergedJsonObject = jsonProvider().createMergePatch(jsonObjectFromQueryParameters).apply(jsonObjectFromBody)
.asJsonObject();
} else {
mergedJsonObject = jsonObjectFromQueryParameters;
Expand Down Expand Up @@ -153,7 +154,7 @@ private void handleInvalidAcceptRequest(HttpServerResponse response) {
}

private JsonObject getJsonObjectFromQueryParameters(RoutingContext ctx) throws UnsupportedEncodingException {
JsonObjectBuilder input = Json.createObjectBuilder();
JsonObjectBuilder input = jsonProvider().createObjectBuilder();
// Query
String query = stripNewlinesAndTabs(readQueryParameter(ctx, QUERY));
if (query != null && !query.isEmpty()) {
Expand Down Expand Up @@ -189,7 +190,7 @@ private JsonObject getJsonObjectFromBody(RoutingContext ctx) throws IOException

// If the content type is application/graphql, the query is in the body
if (contentType != null && contentType.startsWith(APPLICATION_GRAPHQL)) {
JsonObjectBuilder input = Json.createObjectBuilder();
JsonObjectBuilder input = jsonProvider().createObjectBuilder();
input.add(QUERY, body);
return input.build();
// Else we expect a Json in the content
Expand Down
Loading