You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2022/04/21 19:48:26 UTC
[GitHub] [druid] gianm opened a new pull request, #12468: Reduce allocations due to Jackson serialization.
gianm opened a new pull request, #12468:
URL: https://github.com/apache/druid/pull/12468
This patch attacks two sources of allocations during Jackson
serialization:
1) ObjectMapper.writeValue and JsonGenerator.writeObject create a new
DefaultSerializerProvider instance for each call. It has lots of
fields and creates pressure on the garbage collector. So, this patch
adds helper functions in JacksonUtils that enable reuse of
SerializerProvider objects and updates various call sites to make
use of this.
2) GroupByQueryToolChest copies the ObjectMapper for every query to
install a special module that supports backwards compatibility with
map-based rows. This isn't needed if resultAsArray is set and
all servers are running Druid 0.16.0 or later. This release was a
while ago. So, this patch disables backwards compatibility by default,
which eliminates the need to copy the heavyweight ObjectMapper. The
patch also introduces a configuration option that allows admins to
explicitly enable backwards compatibility.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] FrankChen021 commented on pull request #12468: Reduce allocations due to Jackson serialization.
Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on PR #12468:
URL: https://github.com/apache/druid/pull/12468#issuecomment-1105940659
Should this be included in the upcoming 0.23?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhishekagarwal87 closed pull request #12468: Reduce allocations due to Jackson serialization.
Posted by GitBox <gi...@apache.org>.
abhishekagarwal87 closed pull request #12468: Reduce allocations due to Jackson serialization.
URL: https://github.com/apache/druid/pull/12468
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a diff in pull request #12468: Reduce allocations due to Jackson serialization.
Posted by GitBox <gi...@apache.org>.
gianm commented on code in PR #12468:
URL: https://github.com/apache/druid/pull/12468#discussion_r856483680
##########
core/src/main/java/org/apache/druid/java/util/common/jackson/JacksonUtils.java:
##########
@@ -51,6 +58,37 @@ public static <T> T readValue(ObjectMapper mapper, byte[] bytes, Class<T> valueC
}
}
+ /**
+ * Returns a serializer for a particular class. If you have a {@link SerializerProvider}, this is better than calling
+ * {@link JsonGenerator#writeObject(Object)} or {@link ObjectMapper#writeValue(JsonGenerator, Object)}, because it
+ * avoids re-creating the {@link SerializerProvider} for each serialized object.
+ */
+ public static JsonSerializer<Object> getSerializer(final SerializerProvider serializerProvider, final Class<?> clazz)
+ throws JsonMappingException
+ {
+ // cache = true, property = null because this is what DefaultSerializerProvider.serializeValue would do.
+ return serializerProvider.findTypedValueSerializer(clazz, true, null);
+ }
+
+ /**
+ * Serializes an object using a {@link JsonGenerator}. If you have a {@link SerializerProvider}, this is better than
+ * calling {@link JsonGenerator#writeObject(Object)}, because it avoids re-creating the {@link SerializerProvider}
+ * for each serialized object.
+ */
+ public static void writeObjectUsingSerializerProvider(
+ final JsonGenerator jsonGenerator,
Review Comment:
Good idea: I added them, and also updated the remaining call sites.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] lgtm-com[bot] commented on pull request #12468: Reduce allocations due to Jackson serialization.
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on PR #12468:
URL: https://github.com/apache/druid/pull/12468#issuecomment-1106814833
This pull request **introduces 1 alert** when merging c8128edd40569c75b8db72188fed98375b0e3a43 into b7621226d28493899c1331f7bf45469706c248c5 - [view on LGTM.com](https://lgtm.com/projects/g/apache/druid/rev/pr-257ba366b51236056d30a85dcac81094b997aca6)
**new alerts:**
* 1 for Dereferenced variable may be null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm merged pull request #12468: Reduce allocations due to Jackson serialization.
Posted by GitBox <gi...@apache.org>.
gianm merged PR #12468:
URL: https://github.com/apache/druid/pull/12468
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] lgtm-com[bot] commented on pull request #12468: Reduce allocations due to Jackson serialization.
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on PR #12468:
URL: https://github.com/apache/druid/pull/12468#issuecomment-1105760136
This pull request **introduces 1 alert** when merging 9a40e350a41414edbd39a17606bb643574a411db into f95447070ed2ccfa45c44075b2b5e94dfd175369 - [view on LGTM.com](https://lgtm.com/projects/g/apache/druid/rev/pr-03a2c496e00e4921864fb4979080446302b1c9c2)
**new alerts:**
* 1 for Dereferenced variable may be null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] FrankChen021 commented on a diff in pull request #12468: Reduce allocations due to Jackson serialization.
Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on code in PR #12468:
URL: https://github.com/apache/druid/pull/12468#discussion_r855935269
##########
core/src/main/java/org/apache/druid/java/util/common/jackson/JacksonUtils.java:
##########
@@ -51,6 +58,37 @@ public static <T> T readValue(ObjectMapper mapper, byte[] bytes, Class<T> valueC
}
}
+ /**
+ * Returns a serializer for a particular class. If you have a {@link SerializerProvider}, this is better than calling
+ * {@link JsonGenerator#writeObject(Object)} or {@link ObjectMapper#writeValue(JsonGenerator, Object)}, because it
+ * avoids re-creating the {@link SerializerProvider} for each serialized object.
+ */
+ public static JsonSerializer<Object> getSerializer(final SerializerProvider serializerProvider, final Class<?> clazz)
+ throws JsonMappingException
+ {
+ // cache = true, property = null because this is what DefaultSerializerProvider.serializeValue would do.
+ return serializerProvider.findTypedValueSerializer(clazz, true, null);
+ }
+
+ /**
+ * Serializes an object using a {@link JsonGenerator}. If you have a {@link SerializerProvider}, this is better than
+ * calling {@link JsonGenerator#writeObject(Object)}, because it avoids re-creating the {@link SerializerProvider}
+ * for each serialized object.
+ */
+ public static void writeObjectUsingSerializerProvider(
+ final JsonGenerator jsonGenerator,
Review Comment:
Since this helper API is used to replace`ObjectMapper.writeValue` and `JsonGenerator.writeObject`, can we prohibite these two methods in the druid-forbidden-apis.txt?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on pull request #12468: Reduce allocations due to Jackson serialization.
Posted by GitBox <gi...@apache.org>.
gianm commented on PR #12468:
URL: https://github.com/apache/druid/pull/12468#issuecomment-1110632477
> Should this be included in the upcoming 0.23?
IMO, it'd be OK to do it in 0.23, but it's not absolutely necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org