From d3dbb1e44abcda84e42aec0a3f47bb7b6ef313e1 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Thu, 20 Apr 2023 16:49:30 -0400 Subject: [PATCH 1/2] Opt out of canonicalization in Conjure JsonFactories We expect unlimited keyspace due to maps keyed by random IDs which are usually UUID-based. This interacts poorly with string canonicalization components in Jackson in ways that cause heavy heap churn. See the discussion here for the details: https://github.com/FasterXML/jackson-benchmarks/pull/6 The primary risk is that without canonicalization, we allocate reused strings much more heavily. Strings used to match setter methods should be very short lived and should not escape the parsing thread, so the JIT may be able to do terribly clever things to help us out. Note: it may be possible to canonicalize contextually in jackson 3, which would give us the best of both worlds! --- .../palantir/conjure/java/serialization/ObjectMappers.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/ObjectMappers.java b/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/ObjectMappers.java index 371df336d..8916acefd 100644 --- a/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/ObjectMappers.java +++ b/conjure-java-jackson-serialization/src/main/java/com/palantir/conjure/java/serialization/ObjectMappers.java @@ -245,6 +245,10 @@ public static CBORFactory cborFactory() { private static > B withDefaults(B builder) { return ReflectiveStreamReadConstraints.withDefaultConstraints(builder // Interning introduces excessive contention https://github.com/FasterXML/jackson-core/issues/946 - .disable(JsonFactory.Feature.INTERN_FIELD_NAMES)); + .disable(JsonFactory.Feature.INTERN_FIELD_NAMES) + // Canonicalization can be helpful to avoid string re-allocation, however we expect unbounded + // key space due to use of maps keyed by random identifiers, which cause heavy heap churn. + // See this discussion: https://github.com/FasterXML/jackson-benchmarks/pull/6 + .disable(JsonFactory.Feature.CANONICALIZE_FIELD_NAMES)); } } From 384f3e3495b05d7fd9440545cf3a51571dff68c7 Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Thu, 20 Apr 2023 20:57:16 +0000 Subject: [PATCH 2/2] Add generated changelog entries --- changelog/@unreleased/pr-2603.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/@unreleased/pr-2603.v2.yml diff --git a/changelog/@unreleased/pr-2603.v2.yml b/changelog/@unreleased/pr-2603.v2.yml new file mode 100644 index 000000000..4badba748 --- /dev/null +++ b/changelog/@unreleased/pr-2603.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Opt out of canonicalization in Conjure JsonFactories + links: + - https://github.com/palantir/conjure-java-runtime/pull/2603