HV-2127 Do not create unnecessary lists when a simple ConstraintTree is used

marko-bekhta · marko-bekhta · commit 25729f089e37 · 2025-08-07T15:22:00.000+02:00
which would be the case with most constraints

Signed-off-by: marko-bekhta &lt;marko.prykladna@gmail.com&gt;
diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/constraintvalidation/ComposingConstraintTree.java b/engine/src/main/java/org/hibernate/validator/internal/engine/constraintvalidation/ComposingConstraintTree.java
@@ -14,7 +14,6 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
-import java.util.Optional;
 import java.util.stream.Collectors;
 
 import jakarta.validation.ConstraintValidator;
@@ -38,7 +37,7 @@
  * @author Guillaume Smet
  * @author Marko Bekhta
  */
-class ComposingConstraintTree<B extends Annotation> extends ConstraintTree<B> {
+final class ComposingConstraintTree<B extends Annotation> extends ConstraintTree<B> {
 
 	private static final Log LOG = LoggerFactory.make( MethodHandles.lookup() );
 
@@ -61,6 +60,23 @@ private <U extends Annotation> ConstraintTree<U> createConstraintTree(Constraint
 		}
 	}
 
+	@Override
+	public boolean validateConstraints(ValidationContext<?> validationContext, ValueContext<?, ?> valueContext) {
+		List<ConstraintValidatorContextImpl> violatedConstraintValidatorContexts = new ArrayList<>( 5 );
+		validateConstraints( validationContext, valueContext, violatedConstraintValidatorContexts );
+		if ( !violatedConstraintValidatorContexts.isEmpty() ) {
+			for ( ConstraintValidatorContextImpl constraintValidatorContext : violatedConstraintValidatorContexts ) {
+				for ( ConstraintViolationCreationContext constraintViolationCreationContext : constraintValidatorContext.getConstraintViolationCreationContexts() ) {
+					validationContext.addConstraintFailure(
+							valueContext, constraintViolationCreationContext, constraintValidatorContext.getConstraintDescriptor()
+					);
+				}
+			}
+			return false;
+		}
+		return true;
+	}
+
 	@Override
 	protected void validateConstraints(ValidationContext<?> validationContext,
 			ValueContext<?, ?> valueContext,
@@ -69,7 +85,7 @@ protected void validateConstraints(ValidationContext<?> validationContext,
 				validationContext, valueContext, violatedConstraintValidatorContexts
 		);
 
-		Optional<ConstraintValidatorContextImpl> violatedLocalConstraintValidatorContext;
+		ConstraintValidatorContextImpl violatedLocalConstraintValidatorContext;
 
 		// After all children are validated the actual ConstraintValidator of the constraint itself is executed
 		if ( mainConstraintNeedsEvaluation( validationContext, violatedConstraintValidatorContexts ) ) {
@@ -103,15 +119,15 @@ protected void validateConstraints(ValidationContext<?> validationContext,
 
 			// We re-evaluate the boolean composition by taking into consideration also the violations
 			// from the local constraintValidator
-			if ( !violatedLocalConstraintValidatorContext.isPresent() ) {
+			if ( violatedLocalConstraintValidatorContext == null ) {
 				compositionResult.setAtLeastOneTrue( true );
 			}
 			else {
 				compositionResult.setAllTrue( false );
 			}
 		}
 		else {
-			violatedLocalConstraintValidatorContext = Optional.empty();
+			violatedLocalConstraintValidatorContext = null;
 		}
 
 		if ( !passesCompositionTypeRequirement( violatedConstraintValidatorContexts, compositionResult ) ) {
@@ -157,7 +173,7 @@ private boolean mainConstraintNeedsEvaluation(ValidationContext<?> validationCon
 	private void prepareFinalConstraintViolations(ValidationContext<?> validationContext,
 			ValueContext<?, ?> valueContext,
 			Collection<ConstraintValidatorContextImpl> violatedConstraintValidatorContexts,
-			Optional<ConstraintValidatorContextImpl> localConstraintValidatorContext) {
+			ConstraintValidatorContextImpl localConstraintValidatorContext) {
 		if ( reportAsSingleViolation() ) {
 			// We clear the current violations list anyway
 			violatedConstraintValidatorContexts.clear();
@@ -166,7 +182,7 @@ private void prepareFinalConstraintViolations(ValidationContext<?> validationCon
 			// violations or not (or if there is no local ConstraintValidator at all).
 			// If not we create a violation
 			// using the error message in the annotation declaration at top level.
-			if ( !localConstraintValidatorContext.isPresent() ) {
+			if ( localConstraintValidatorContext == null ) {
 				violatedConstraintValidatorContexts.add(
 						validationContext.createConstraintValidatorContextFor(
 								descriptor, valueContext.getPropertyPath()
@@ -183,8 +199,8 @@ private void prepareFinalConstraintViolations(ValidationContext<?> validationCon
 		// as checked in test CustomErrorMessage.java
 		// If no violations have been reported from the local ConstraintValidator, or no such validator exists,
 		// then we just add an empty list.
-		if ( localConstraintValidatorContext.isPresent() ) {
-			violatedConstraintValidatorContexts.add( localConstraintValidatorContext.get() );
+		if ( localConstraintValidatorContext != null ) {
+			violatedConstraintValidatorContexts.add( localConstraintValidatorContext );
 		}
 	}
 
diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/constraintvalidation/ConstraintTree.java b/engine/src/main/java/org/hibernate/validator/internal/engine/constraintvalidation/ConstraintTree.java
@@ -7,9 +7,7 @@
 import java.lang.annotation.Annotation;
 import java.lang.invoke.MethodHandles;
 import java.lang.reflect.Type;
-import java.util.ArrayList;
 import java.util.Collection;
-import java.util.List;
 import java.util.Optional;
 
 import jakarta.validation.ConstraintDeclarationException;
@@ -33,7 +31,7 @@
  * @author Guillaume Smet
  * @author Marko Bekhta
  */
-public abstract class ConstraintTree<A extends Annotation> {
+public abstract sealed class ConstraintTree<A extends Annotation> permits SimpleConstraintTree, ComposingConstraintTree {
 
 	private static final Log LOG = LoggerFactory.make( MethodHandles.lookup() );
 
@@ -68,21 +66,7 @@ public static <U extends Annotation> ConstraintTree<U> of(ConstraintValidatorMan
 		}
 	}
 
-	public final boolean validateConstraints(ValidationContext<?> validationContext, ValueContext<?, ?> valueContext) {
-		List<ConstraintValidatorContextImpl> violatedConstraintValidatorContexts = new ArrayList<>( 5 );
-		validateConstraints( validationContext, valueContext, violatedConstraintValidatorContexts );
-		if ( !violatedConstraintValidatorContexts.isEmpty() ) {
-			for ( ConstraintValidatorContextImpl constraintValidatorContext : violatedConstraintValidatorContexts ) {
-				for ( ConstraintViolationCreationContext constraintViolationCreationContext : constraintValidatorContext.getConstraintViolationCreationContexts() ) {
-					validationContext.addConstraintFailure(
-							valueContext, constraintViolationCreationContext, constraintValidatorContext.getConstraintDescriptor()
-					);
-				}
-			}
-			return false;
-		}
-		return true;
-	}
+	public abstract boolean validateConstraints(ValidationContext<?> validationContext, ValueContext<?, ?> valueContext);
 
 	protected abstract void validateConstraints(ValidationContext<?> validationContext, ValueContext<?, ?> valueContext,
 			Collection<ConstraintValidatorContextImpl> violatedConstraintValidatorContexts);
@@ -168,7 +152,7 @@ private ValidationException getExceptionForNullValidator(Type validatedValueType
 	 * @return an {@link Optional#empty()} if there is no violation or a corresponding {@link ConstraintValidatorContextImpl}
 	 * 		otherwise.
 	 */
-	protected final <V> Optional<ConstraintValidatorContextImpl> validateSingleConstraint(
+	protected final <V> ConstraintValidatorContextImpl validateSingleConstraint(
 			ValueContext<?, ?> valueContext,
 			ConstraintValidatorContextImpl constraintValidatorContext,
 			ConstraintValidator<A, V> validator) {
@@ -187,9 +171,9 @@ protected final <V> Optional<ConstraintValidatorContextImpl> validateSingleConst
 		if ( !isValid ) {
 			//We do not add these violations yet, since we don't know how they are
 			//going to influence the final boolean evaluation
-			return Optional.of( constraintValidatorContext );
+			return constraintValidatorContext;
 		}
-		return Optional.empty();
+		return null;
 	}
 
 	@Override
diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/constraintvalidation/SimpleConstraintTree.java b/engine/src/main/java/org/hibernate/validator/internal/engine/constraintvalidation/SimpleConstraintTree.java
@@ -27,7 +27,7 @@
  * @author Guillaume Smet
  * @author Marko Bekhta
  */
-class SimpleConstraintTree<B extends Annotation> extends ConstraintTree<B> {
+final class SimpleConstraintTree<B extends Annotation> extends ConstraintTree<B> {
 
 	private static final Log LOG = LoggerFactory.make( MethodHandles.lookup() );
 
@@ -36,16 +36,43 @@ public SimpleConstraintTree(ConstraintValidatorManager constraintValidatorManage
 	}
 
 	@Override
-	protected void validateConstraints(ValidationContext<?> validationContext,
+	public boolean validateConstraints(ValidationContext<?> validationContext, ValueContext<?, ?> valueContext) {
+		ConstraintValidatorContextImpl constraintValidatorContext = doValidateConstraints( validationContext, valueContext );
+		if ( constraintValidatorContext != null ) {
+			for ( ConstraintViolationCreationContext constraintViolationCreationContext : constraintValidatorContext.getConstraintViolationCreationContexts() ) {
+				validationContext.addConstraintFailure(
+						valueContext, constraintViolationCreationContext, constraintValidatorContext.getConstraintDescriptor()
+				);
+			}
+			return false;
+		}
+		return true;
+	}
+
+	@Override
+	protected void validateConstraints(
+			ValidationContext<?> validationContext,
 			ValueContext<?, ?> valueContext,
-			Collection<ConstraintValidatorContextImpl> violatedConstraintValidatorContexts) {
+			Collection<ConstraintValidatorContextImpl> violatedConstraintValidatorContexts
+	) {
+		ConstraintValidatorContextImpl constraintValidatorContext = doValidateConstraints( validationContext, valueContext );
 
+		if ( constraintValidatorContext != null ) {
+			violatedConstraintValidatorContexts.add( constraintValidatorContext );
+		}
+	}
+
+	private ConstraintValidatorContextImpl doValidateConstraints(
+			ValidationContext<?> validationContext,
+			ValueContext<?, ?> valueContext
+	) {
 		if ( LOG.isTraceEnabled() ) {
 			if ( validationContext.isShowValidatedValuesInTraceLogs() ) {
 				LOG.tracef(
 						"Validating value %s against constraint defined by %s.",
 						valueContext.getCurrentValidatedValue(),
-						descriptor );
+						descriptor
+				);
 			}
 			else {
 				LOG.tracef( "Validating against constraint defined by %s.", descriptor );
@@ -61,8 +88,6 @@ protected void validateConstraints(ValidationContext<?> validationContext,
 		);
 
 		// validate
-		if ( validateSingleConstraint( valueContext, constraintValidatorContext, validator ).isPresent() ) {
-			violatedConstraintValidatorContexts.add( constraintValidatorContext );
-		}
+		return validateSingleConstraint( valueContext, constraintValidatorContext, validator );
 	}
 }
diff --git a/engine/src/main/java/org/hibernate/validator/internal/metadata/core/MetaConstraint.java b/engine/src/main/java/org/hibernate/validator/internal/metadata/core/MetaConstraint.java
@@ -128,9 +128,8 @@ public boolean validateConstraint(ValidationContext<?> validationContext, ValueC
 
 	private boolean doValidateConstraint(ValidationContext<?> executionContext, ValueContext<?, ?> valueContext) {
 		valueContext.setConstraintLocationKind( getConstraintLocationKind() );
-		boolean validationResult = constraintTree.validateConstraints( executionContext, valueContext );
 
-		return validationResult;
+		return constraintTree.validateConstraints( executionContext, valueContext );
 	}
 
 	public ConstraintLocation getLocation() {
diff --git a/performance/src/main/jakarta/org/hibernate/validator/performance/simple/SimpleSingleElementValidation.java b/performance/src/main/jakarta/org/hibernate/validator/performance/simple/SimpleSingleElementValidation.java
@@ -0,0 +1,85 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright Red Hat Inc. and Hibernate Authors
+ */
+package org.hibernate.validator.performance.simple;
+
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+import jakarta.validation.ConstraintViolation;
+import jakarta.validation.Validation;
+import jakarta.validation.Validator;
+import jakarta.validation.ValidatorFactory;
+import jakarta.validation.constraints.Min;
+import jakarta.validation.constraints.NotNull;
+import jakarta.validation.constraints.Size;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Threads;
+import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.infra.Blackhole;
+
+@State(Scope.Benchmark)
+@BenchmarkMode(Mode.Throughput)
+@OutputTimeUnit(TimeUnit.MILLISECONDS)
+@Fork(value = 1)
+@Threads(50)
+@Warmup(iterations = 10)
+@Measurement(iterations = 20)
+public class SimpleSingleElementValidation {
+
+	// The class to be validated
+	private static class User {
+		@NotNull
+		@Size(min = 3, max = 50)
+		private String name;
+
+		@Size(min = 3, max = 50)
+		@NotNull
+		private String email;
+
+		@Min(value = 20)
+		private int age;
+
+		public User(String name, String email, int age) {
+			this.name = name;
+			this.email = email;
+			this.age = age;
+		}
+	}
+
+	@State(Scope.Benchmark)
+	public static class BenchmarkState {
+		Validator validator;
+		User validUser;
+		User invalidUser;
+
+		@Setup
+		public void setup() {
+			ValidatorFactory factory = Validation.buildDefaultValidatorFactory();
+			this.validator = factory.getValidator();
+			this.validUser = new User( "John Doe", "john.doe@example.com", 25 );
+			this.invalidUser = new User( "Jo", "invalid-email", 19 );
+		}
+	}
+
+	@Benchmark
+	public void validObjectValidation(BenchmarkState state, Blackhole blackhole) {
+		Set<ConstraintViolation<User>> violations = state.validator.validate( state.validUser );
+		blackhole.consume( violations );
+	}
+
+	@Benchmark
+	public void invalidObjectValidation(BenchmarkState state, Blackhole blackhole) {
+		Set<ConstraintViolation<User>> violations = state.validator.validate( state.invalidUser );
+		blackhole.consume( violations );
+	}
+}

Original file line number	Diff line number	Diff line change
`@@ -128,9 +128,8 @@ public boolean validateConstraint(ValidationContext<?> validationContext, ValueC`
`128`	`128`
`129`	`129`	`private boolean doValidateConstraint(ValidationContext<?> executionContext, ValueContext<?, ?> valueContext) {`
`130`	`130`	`valueContext.setConstraintLocationKind( getConstraintLocationKind() );`
`131`		`- boolean validationResult = constraintTree.validateConstraints( executionContext, valueContext );`
`132`	`131`
`133`		`- return validationResult;`
	`132`	`+ return constraintTree.validateConstraints( executionContext, valueContext );`
`134`	`133`	`}`
`135`	`134`
`136`	`135`	`public ConstraintLocation getLocation() {`