Skip to content

Commit 8686067

Browse files
authored
Reduce Optional allocations in SafetyEvaluator (#2433)
Reduce Optional allocations in SafetyEvaluator
1 parent 40c597e commit 8686067

File tree

2 files changed

+40
-5
lines changed

2 files changed

+40
-5
lines changed

changelog/@unreleased/pr-2433.v2.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
type: improvement
2+
improvement:
3+
description: Reduce Optional allocations in SafetyEvaluator
4+
links:
5+
- https://github.com/palantir/conjure-java/pull/2433

conjure-java-core/src/main/java/com/palantir/conjure/java/types/SafetyEvaluator.java

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package com.palantir.conjure.java.types;
1818

19+
import com.google.common.collect.ImmutableTable;
20+
import com.google.common.collect.Table;
1921
import com.palantir.conjure.java.util.TypeFunctions;
2022
import com.palantir.conjure.spec.AliasDefinition;
2123
import com.palantir.conjure.spec.ArgumentDefinition;
@@ -37,17 +39,23 @@
3739
import com.palantir.conjure.spec.UnionDefinition;
3840
import com.palantir.logsafe.Preconditions;
3941
import java.util.HashSet;
42+
import java.util.List;
4043
import java.util.Map;
4144
import java.util.Optional;
4245
import java.util.Set;
4346
import java.util.function.Supplier;
47+
import java.util.stream.Stream;
4448

4549
public final class SafetyEvaluator {
50+
51+
// Make a constant to avoid allocating a new Optional every time we process a type definition for safety
52+
private static final Optional<LogSafety> OPTIONAL_OF_SAFE = Optional.of(LogSafety.SAFE);
53+
4654
/**
4755
* Enums contain an unknown variant, however we assume that the unknown variant is only used
4856
* for past and future values which are known at compile-time in that version.
4957
*/
50-
public static final Optional<LogSafety> ENUM_SAFETY = Optional.of(LogSafety.SAFE);
58+
public static final Optional<LogSafety> ENUM_SAFETY = OPTIONAL_OF_SAFE;
5159
/**
5260
* Unknown variant should be considered unsafe because we don't know what kind of data it may contain,
5361
* however this makes rollout much more challenging, so we will ratchet unknown safety once we have
@@ -137,7 +145,7 @@ public Optional<LogSafety> visitEnum(EnumDefinition _value) {
137145
@Override
138146
public Optional<LogSafety> visitObject(ObjectDefinition value) {
139147
return with(value.getTypeName(), () -> {
140-
Optional<LogSafety> safety = Optional.of(LogSafety.SAFE);
148+
Optional<LogSafety> safety = OPTIONAL_OF_SAFE;
141149
for (FieldDefinition field : value.getFields()) {
142150
safety = combine(safety, getSafety(field.getType(), field.getSafety()));
143151
}
@@ -164,7 +172,7 @@ public Optional<LogSafety> visitUnknown(String unknownType) {
164172
private Optional<LogSafety> with(TypeName typeName, Supplier<Optional<LogSafety>> task) {
165173
if (!inProgress.add(typeName)) {
166174
// Given recursive evaluation, we return the least restrictive type: SAFE.
167-
return Optional.of(LogSafety.SAFE);
175+
return OPTIONAL_OF_SAFE;
168176
}
169177
Optional<LogSafety> result = task.get();
170178
if (!inProgress.remove(typeName)) {
@@ -298,7 +306,25 @@ public Optional<LogSafety> visitUnknown(String unknownValue) {
298306
}
299307
}
300308

301-
public static Optional<LogSafety> combine(Optional<LogSafety> one, Optional<LogSafety> two) {
309+
private static final Table<Optional<LogSafety>, Optional<LogSafety>, Optional<LogSafety>> COMBINE_TABLE =
310+
computeCombineTable();
311+
312+
private static Table<Optional<LogSafety>, Optional<LogSafety>, Optional<LogSafety>> computeCombineTable() {
313+
List<Optional<LogSafety>> allValues = Stream.concat(
314+
Stream.of(Optional.<LogSafety>empty()),
315+
LogSafety.values().stream().map(Optional::of))
316+
.toList();
317+
ImmutableTable.Builder<Optional<LogSafety>, Optional<LogSafety>, Optional<LogSafety>> builder =
318+
ImmutableTable.builder();
319+
for (Optional<LogSafety> left : allValues) {
320+
for (Optional<LogSafety> right : allValues) {
321+
builder.put(left, right, computeCombine(left, right));
322+
}
323+
}
324+
return builder.buildOrThrow();
325+
}
326+
327+
private static Optional<LogSafety> computeCombine(Optional<LogSafety> one, Optional<LogSafety> two) {
302328
if (one.isPresent() && two.isPresent()) {
303329
return Optional.of(combine(one.get(), two.get()));
304330
}
@@ -307,7 +333,7 @@ public static Optional<LogSafety> combine(Optional<LogSafety> one, Optional<LogS
307333
.filter(value -> !LogSafety.SAFE.equals(value));
308334
}
309335

310-
public static LogSafety combine(LogSafety one, LogSafety two) {
336+
private static LogSafety combine(LogSafety one, LogSafety two) {
311337
LogSafety.Value first = one.get();
312338
LogSafety.Value second = two.get();
313339
if (first == LogSafety.Value.UNKNOWN || second == LogSafety.Value.UNKNOWN) {
@@ -322,6 +348,10 @@ public static LogSafety combine(LogSafety one, LogSafety two) {
322348
return one;
323349
}
324350

351+
public static Optional<LogSafety> combine(Optional<LogSafety> one, Optional<LogSafety> two) {
352+
return Preconditions.checkNotNull(COMBINE_TABLE.get(one, two), "Missing an entry in the combine table");
353+
}
354+
325355
public static boolean allows(Optional<LogSafety> required, Optional<LogSafety> given) {
326356
if (required.isEmpty() || given.isEmpty()) {
327357
// If there is no requirement, all inputs are allowed.

0 commit comments

Comments
 (0)