From 31fa92a8ffa258fb2fd3dd20028ddb0431752cd4 Mon Sep 17 00:00:00 2001 From: Helen Yu Date: Mon, 13 Jul 2026 23:35:23 -0400 Subject: [PATCH 1/3] Memoize SafetyEvaluator type evaluations SafetyEvaluator re-evaluates the transitive safety of a referenced type at every reference site, so evaluation cost is proportional to the number of paths through the type graph rather than its size. On large, dense conjure definitions this dominates java codegen: a real-world 3.2MB IR spends ~350s in objects generation, with all sampled stacks inside SafetyEvaluator; the same IR generates in ~35s with this cache, with byte-identical output. Results are memoized per TypeName, respecting the recursive-type cycle guard: values computed after hitting the in-progress set are substituted with SAFE and therefore context-dependent, so only cycle-free subtree results and outermost (canonical) evaluations enter the cache. --- .../conjure/java/types/SafetyEvaluator.java | 29 +++++++- .../java/types/SafetyEvaluatorTest.java | 69 +++++++++++++++++++ 2 files changed, 95 insertions(+), 3 deletions(-) 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 1f3ad20a0..138ae39eb 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 @@ -38,6 +38,7 @@ import com.palantir.conjure.spec.TypeName; import com.palantir.conjure.spec.UnionDefinition; import com.palantir.logsafe.Preconditions; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -64,6 +65,7 @@ public final class SafetyEvaluator { public static final Optional UNKNOWN_UNION_VARINT_SAFETY = Optional.empty(); private final Map definitionMap; + private final Map> safetyCache = new HashMap<>(); public SafetyEvaluator(ConjureDefinition definition) { this(TypeFunctions.toTypesMap(definition)); @@ -75,12 +77,12 @@ public SafetyEvaluator(Map definitionMap) { public Optional evaluate(TypeDefinition def) { return Preconditions.checkNotNull(def, "TypeDefinition is required") - .accept(new TypeDefinitionSafetyVisitor(definitionMap, new HashSet<>())); + .accept(new TypeDefinitionSafetyVisitor(definitionMap, new HashSet<>(), safetyCache)); } public Optional evaluate(Type type) { return Preconditions.checkNotNull(type, "TypeDefinition is required") - .accept(new TypeDefinitionSafetyVisitor(definitionMap, new HashSet<>()).fieldVisitor); + .accept(new TypeDefinitionSafetyVisitor(definitionMap, new HashSet<>(), safetyCache).fieldVisitor); } public Optional evaluate(Type type, Optional declaredSafety) { @@ -126,9 +128,15 @@ public Optional getUsageTimeSafety(FieldDefinition field) { private static final class TypeDefinitionSafetyVisitor implements TypeDefinition.Visitor> { private final Set inProgress; private final Type.Visitor> fieldVisitor; + private final Map> cache; + private int cycleHits = 0; - private TypeDefinitionSafetyVisitor(Map definitionMap, Set inProgress) { + private TypeDefinitionSafetyVisitor( + Map definitionMap, + Set inProgress, + Map> cache) { this.inProgress = inProgress; + this.cache = cache; this.fieldVisitor = new FieldSafetyVisitor(definitionMap, this); } @@ -170,15 +178,30 @@ public Optional visitUnknown(String unknownType) { } private Optional with(TypeName typeName, Supplier> task) { + Optional cached = cache.get(typeName); + if (cached != null) { + return cached; + } + boolean outermost = inProgress.isEmpty(); if (!inProgress.add(typeName)) { + cycleHits++; // Given recursive evaluation, we return the least restrictive type: SAFE. return OPTIONAL_OF_SAFE; } + int cycleHitsBefore = cycleHits; Optional result = task.get(); if (!inProgress.remove(typeName)) { throw new IllegalStateException( "Failed to remove " + typeName + " from in-progress, something is very wrong!"); } + // Without memoization, evaluation cost is proportional to the number of paths through + // the type graph, which grows combinatorially on dense type graphs. Results computed + // without hitting the in-progress cycle guard are context-free and safe to memoize; + // outermost results are the canonical evaluation for a type even when a cycle guard + // fired within the subtree. + if (outermost || cycleHits == cycleHitsBefore) { + cache.put(typeName, result); + } return result; } diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java index 922ec9459..63b93679d 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java @@ -408,4 +408,73 @@ private static Stream getTypes(Type externalReference) { Arguments.of(Named.of("Alias", aliasType), conjureAliasDef), Arguments.of(Named.of("Union", unionType), conjureUnionDef)); } + + @Test + void testSharedEvaluatorMatchesFreshEvaluations() { + TypeDefinition firstObject = TypeDefinition.object(ObjectDefinition.builder() + .typeName(FOO) + .fields(FieldDefinition.builder() + .fieldName(FieldName.of("bar")) + .type(Type.reference(BAR)) + .build()) + .build()); + TypeDefinition secondObject = TypeDefinition.object(ObjectDefinition.builder() + .typeName(BAR) + .fields(FieldDefinition.builder() + .fieldName(FieldName.of("aliasRef")) + .type(Type.reference(UNSAFE_ALIAS_NAME)) + .build()) + .build()); + ConjureDefinition conjureDef = ConjureDefinition.builder() + .version(1) + .types(firstObject) + .types(secondObject) + .types(UNSAFE_ALIAS) + .build(); + ConjureDefinitionValidator.validateAll(conjureDef, SafetyDeclarationRequirements.ALLOWED); + SafetyEvaluator shared = new SafetyEvaluator(conjureDef); + assertThat(shared.evaluate(firstObject)).hasValue(LogSafety.UNSAFE); + // Repeated evaluations on the same instance take the memoized path and must agree + // with fresh evaluators. + assertThat(shared.evaluate(firstObject)).hasValue(LogSafety.UNSAFE); + assertThat(shared.evaluate(secondObject)) + .isEqualTo(new SafetyEvaluator(conjureDef).evaluate(secondObject)); + } + + @Test + void testRecursiveTypeSubtreeIsNotMemoizedWithCycleSubstitution() { + // Foo <-> Bar cycle where only Foo carries unsafe data. While evaluating Foo, the + // cycle guard substitutes SAFE for the in-progress Foo reference, making Bar appear + // SAFE within that traversal. A later standalone evaluation of Bar on the same + // evaluator must still see through the cycle to Foo's unsafe field rather than + // reusing the substituted value. + TypeDefinition firstObject = TypeDefinition.object(ObjectDefinition.builder() + .typeName(FOO) + .fields(FieldDefinition.builder() + .fieldName(FieldName.of("bar")) + .type(Type.reference(BAR)) + .build()) + .fields(FieldDefinition.builder() + .fieldName(FieldName.of("aliasRef")) + .type(Type.reference(UNSAFE_ALIAS_NAME)) + .build()) + .build()); + TypeDefinition secondObject = TypeDefinition.object(ObjectDefinition.builder() + .typeName(BAR) + .fields(FieldDefinition.builder() + .fieldName(FieldName.of("foo")) + .type(Type.reference(FOO)) + .build()) + .build()); + ConjureDefinition conjureDef = ConjureDefinition.builder() + .version(1) + .types(firstObject) + .types(secondObject) + .types(UNSAFE_ALIAS) + .build(); + SafetyEvaluator shared = new SafetyEvaluator(conjureDef); + assertThat(shared.evaluate(firstObject)).hasValue(LogSafety.UNSAFE); + assertThat(shared.evaluate(secondObject)).hasValue(LogSafety.UNSAFE); + assertThat(shared.evaluate(firstObject)).hasValue(LogSafety.UNSAFE); + } } From 569782a6a19cbebc8d4fed4e8c85402732490e50 Mon Sep 17 00:00:00 2001 From: Helen Yu Date: Mon, 13 Jul 2026 23:36:02 -0400 Subject: [PATCH 2/3] Add changelog entry --- changelog/@unreleased/pr-2976.v2.yml | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changelog/@unreleased/pr-2976.v2.yml diff --git a/changelog/@unreleased/pr-2976.v2.yml b/changelog/@unreleased/pr-2976.v2.yml new file mode 100644 index 000000000..87c0593d3 --- /dev/null +++ b/changelog/@unreleased/pr-2976.v2.yml @@ -0,0 +1,8 @@ +type: improvement +improvement: + description: SafetyEvaluator memoizes per-type safety results, making objects + codegen cost proportional to type-graph size rather than the number of paths + through it. Large, dense conjure definitions generate up to 10-25x faster + with byte-identical output. + links: + - https://github.com/palantir/conjure-java/pull/2976 From cedca0acfcda54024e006018fccb3470384c5422 Mon Sep 17 00:00:00 2001 From: Helen Yu Date: Mon, 13 Jul 2026 23:47:24 -0400 Subject: [PATCH 3/3] Apply spotless formatting --- .../com/palantir/conjure/java/types/SafetyEvaluatorTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java index 63b93679d..ab2af6248 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/types/SafetyEvaluatorTest.java @@ -437,8 +437,7 @@ void testSharedEvaluatorMatchesFreshEvaluations() { // Repeated evaluations on the same instance take the memoized path and must agree // with fresh evaluators. assertThat(shared.evaluate(firstObject)).hasValue(LogSafety.UNSAFE); - assertThat(shared.evaluate(secondObject)) - .isEqualTo(new SafetyEvaluator(conjureDef).evaluate(secondObject)); + assertThat(shared.evaluate(secondObject)).isEqualTo(new SafetyEvaluator(conjureDef).evaluate(secondObject)); } @Test