diff --git a/changelog/@unreleased/pr-2433.v2.yml b/changelog/@unreleased/pr-2433.v2.yml new file mode 100644 index 000000000..0208d1090 --- /dev/null +++ b/changelog/@unreleased/pr-2433.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Reduce Optional allocations in SafetyEvaluator + links: + - https://github.com/palantir/conjure-java/pull/2433 diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/SafetyEvaluator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/SafetyEvaluator.java index 3313e2600..1f3ad20a0 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/SafetyEvaluator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/SafetyEvaluator.java @@ -16,6 +16,8 @@ package com.palantir.conjure.java.types; +import com.google.common.collect.ImmutableTable; +import com.google.common.collect.Table; import com.palantir.conjure.java.util.TypeFunctions; import com.palantir.conjure.spec.AliasDefinition; import com.palantir.conjure.spec.ArgumentDefinition; @@ -37,17 +39,23 @@ import com.palantir.conjure.spec.UnionDefinition; import com.palantir.logsafe.Preconditions; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.function.Supplier; +import java.util.stream.Stream; public final class SafetyEvaluator { + + // Make a constant to avoid allocating a new Optional every time we process a type definition for safety + private static final Optional OPTIONAL_OF_SAFE = Optional.of(LogSafety.SAFE); + /** * Enums contain an unknown variant, however we assume that the unknown variant is only used * for past and future values which are known at compile-time in that version. */ - public static final Optional ENUM_SAFETY = Optional.of(LogSafety.SAFE); + public static final Optional ENUM_SAFETY = OPTIONAL_OF_SAFE; /** * Unknown variant should be considered unsafe because we don't know what kind of data it may contain, * however this makes rollout much more challenging, so we will ratchet unknown safety once we have @@ -137,7 +145,7 @@ public Optional visitEnum(EnumDefinition _value) { @Override public Optional visitObject(ObjectDefinition value) { return with(value.getTypeName(), () -> { - Optional safety = Optional.of(LogSafety.SAFE); + Optional safety = OPTIONAL_OF_SAFE; for (FieldDefinition field : value.getFields()) { safety = combine(safety, getSafety(field.getType(), field.getSafety())); } @@ -164,7 +172,7 @@ public Optional visitUnknown(String unknownType) { private Optional with(TypeName typeName, Supplier> task) { if (!inProgress.add(typeName)) { // Given recursive evaluation, we return the least restrictive type: SAFE. - return Optional.of(LogSafety.SAFE); + return OPTIONAL_OF_SAFE; } Optional result = task.get(); if (!inProgress.remove(typeName)) { @@ -298,7 +306,25 @@ public Optional visitUnknown(String unknownValue) { } } - public static Optional combine(Optional one, Optional two) { + private static final Table, Optional, Optional> COMBINE_TABLE = + computeCombineTable(); + + private static Table, Optional, Optional> computeCombineTable() { + List> allValues = Stream.concat( + Stream.of(Optional.empty()), + LogSafety.values().stream().map(Optional::of)) + .toList(); + ImmutableTable.Builder, Optional, Optional> builder = + ImmutableTable.builder(); + for (Optional left : allValues) { + for (Optional right : allValues) { + builder.put(left, right, computeCombine(left, right)); + } + } + return builder.buildOrThrow(); + } + + private static Optional computeCombine(Optional one, Optional two) { if (one.isPresent() && two.isPresent()) { return Optional.of(combine(one.get(), two.get())); } @@ -307,7 +333,7 @@ public static Optional combine(Optional one, Optional !LogSafety.SAFE.equals(value)); } - public static LogSafety combine(LogSafety one, LogSafety two) { + private static LogSafety combine(LogSafety one, LogSafety two) { LogSafety.Value first = one.get(); LogSafety.Value second = two.get(); if (first == LogSafety.Value.UNKNOWN || second == LogSafety.Value.UNKNOWN) { @@ -322,6 +348,10 @@ public static LogSafety combine(LogSafety one, LogSafety two) { return one; } + public static Optional combine(Optional one, Optional two) { + return Preconditions.checkNotNull(COMBINE_TABLE.get(one, two), "Missing an entry in the combine table"); + } + public static boolean allows(Optional required, Optional given) { if (required.isEmpty() || given.isEmpty()) { // If there is no requirement, all inputs are allowed.