Cut repeated lookups on the validation path

bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ConstraintD.java

@@ -82,6 +82,8 @@ private static <T> Set<T> set(Supplier<T[]> array) {
     private final List<Class<? extends ConstraintValidator<A, ?>>> constraintValidatorClasses;
     // racy single-check memoization: the annotation is immutable, so concurrent initializations agree
     private String messageTemplate;
+    /** Memo of this constraint's entry in the factory's validator cache. */
+    private volatile ConstraintValidator<?, ?> validator;
     private final Lazy<String> toString =
         new Lazy<>(() -> String.format("%s: %s", ConstraintD.class.getSimpleName(), getAnnotation()));
 
@@ -171,6 +173,24 @@ public <U> U unwrap(Class<U> type) throws ValidationException {
         }
     }
 
+    /**
+     * Get the initialized {@link ConstraintValidator} previously recorded with {@link #setValidator}.
+     *
+     * @return {@link ConstraintValidator} or {@code null}
+     */
+    public ConstraintValidator<?, ?> getValidator() {
+        return validator;
+    }
+
+    /**
+     * Record the initialized {@link ConstraintValidator} the factory caches for this constraint.
+     *
+     * @param validator
+     */
+    public void setValidator(ConstraintValidator<?, ?> validator) {
+        this.validator = validator;
+    }
+
     public Scope getScope() {
         return scope;
     }

bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/DescriptorManager.java

@@ -38,6 +38,7 @@
 import org.apache.bval.jsr.metadata.HierarchyBuilder;
 import org.apache.bval.jsr.metadata.MetadataBuilder;
 import org.apache.bval.jsr.metadata.ReflectionBuilder;
+import org.apache.bval.jsr.util.Proxies;
 import org.apache.bval.util.Validate;
 
 public class DescriptorManager {
@@ -59,8 +60,8 @@ public static <E extends ExecutableDescriptor> boolean isConstrained(E descripto
 
     private final ApacheValidatorFactory validatorFactory;
     private final ConcurrentMap<Class<?>, BeanD<?>> beanDescriptors = new ConcurrentHashMap<>();
-    /** Same meaning as {@link BeanDescriptor#isBeanConstrained()} — avoids loading metadata on repeat {@code hasWork} checks. */
-    private final ConcurrentMap<Class<?>, Boolean> beanConstrainedByType = new ConcurrentHashMap<>();
+    /** Keyed by the runtime class of validated objects, which may be a proxy of the described class. */
+    private final ConcurrentMap<Class<?>, BeanD<?>> beanDescriptorsByRuntimeClass = new ConcurrentHashMap<>();
     // synchronization unnecessary
     private final ReflectionBuilder reflectionBuilder;
 
@@ -70,32 +71,40 @@ public DescriptorManager(ApacheValidatorFactory validatorFactory) {
         this.reflectionBuilder = new ReflectionBuilder(validatorFactory);
     }
 
-    /**
-     * @return cached result of {@link BeanDescriptor#isBeanConstrained()} for this class, or {@code null} if unknown
-     */
-    public Boolean getCachedBeanConstrained(Class<?> beanClass) {
-        return beanConstrainedByType.get(beanClass);
-    }
-
     public <T> BeanDescriptor getBeanDescriptor(Class<T> beanClass) {
         Validate.notNull(beanClass, IllegalArgumentException::new, "beanClass");
 
         // cannot use computeIfAbsent due to recursion being the usual case:
         final BeanD<?> existing = beanDescriptors.get(beanClass);
         if (existing != null) {
-            beanConstrainedByType.put(beanClass, existing.isBeanConstrained());
             return existing;
         }
         final BeanD<?> value = new BeanD<>(new MetadataReader(validatorFactory, beanClass).forBean(builder(beanClass)));
         final BeanD<?> previous = beanDescriptors.putIfAbsent(beanClass, value);
-        final BeanD<?> result = previous == null ? value : previous;
-        beanConstrainedByType.put(beanClass, result.isBeanConstrained());
+        return previous == null ? value : previous;
+    }
+
+    /**
+     * Get the descriptor for an object of runtime type {@code runtimeClass}, which is first unwrapped if it is a
+     * proxy class.
+     *
+     * @param runtimeClass
+     * @return {@link BeanD}
+     * @see Proxies#classFor(Class)
+     */
+    public BeanD<?> getBeanDescriptorForRuntimeClass(Class<?> runtimeClass) {
+        final BeanD<?> existing = beanDescriptorsByRuntimeClass.get(runtimeClass);
+        if (existing != null) {
+            return existing;
+        }
+        final BeanD<?> result = (BeanD<?>) getBeanDescriptor(Proxies.classFor(runtimeClass));
+        beanDescriptorsByRuntimeClass.putIfAbsent(runtimeClass, result);
         return result;
     }
 
     public void clear() {
         beanDescriptors.clear();
-        beanConstrainedByType.clear();
+        beanDescriptorsByRuntimeClass.clear();
     }
 
     private <T> MetadataBuilder.ForBean<T> builder(Class<T> beanClass) {

bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/PropertyD.java

@@ -16,6 +16,11 @@
  */
 package org.apache.bval.jsr.descriptor;
 
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodHandles.Lookup;
+import java.lang.invoke.MethodType;
+import java.lang.reflect.AccessibleObject;
 import java.lang.reflect.AnnotatedElement;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
@@ -48,13 +53,28 @@ public String getPropertyName() {
 
         @Override
         public Object getValue(Object parent) throws Exception {
+            final MethodHandle getter = getter();
+            if (getter != null) {
+                try {
+                    return (Object) getter.invokeExact(parent);
+                } catch (Exception | Error e) {
+                    throw e;
+                } catch (Throwable t) {
+                    throw new IllegalArgumentException(t);
+                }
+            }
             Reflection.makeAccessible(getTarget());
             try {
                 return getTarget().get(parent);
             } catch (IllegalAccessException e) {
                 throw new IllegalArgumentException(e);
             }
         }
+
+        @Override
+        MethodHandle unreflect(Lookup lookup) throws IllegalAccessException {
+            return lookup.unreflectGetter(getTarget());
+        }
     }
 
     static class ForMethod extends PropertyD<Method> {
@@ -70,15 +90,40 @@ public String getPropertyName() {
 
         @Override
         public Object getValue(Object parent) throws Exception {
+            final MethodHandle getter = getter();
+            if (getter != null) {
+                try {
+                    return (Object) getter.invokeExact(parent);
+                } catch (Throwable t) {
+                    // same wrapping as Method#invoke followed by the catch below
+                    throw new IllegalArgumentException(new InvocationTargetException(t));
+                }
+            }
             Reflection.makeAccessible(getTarget());
             try {
                 return getTarget().invoke(parent);
             } catch (IllegalAccessException | InvocationTargetException e) {
                 throw new IllegalArgumentException(e);
             }
         }
+
+        @Override
+        MethodHandle unreflect(Lookup lookup) throws IllegalAccessException {
+            return lookup.unreflect(getTarget());
+        }
     }
 
+    private static final MethodType GETTER_TYPE = MethodType.methodType(Object.class, Object.class);
+
+    /**
+     * Reading through a cached {@link MethodHandle} avoids the slow path that {@link Field#get(Object)} and
+     * {@link Method#invoke(Object, Object...)} take when the reflective object is not a JIT constant. Holds
+     * {@link #NO_GETTER} when no handle can be created, in which case reflection is used.
+     */
+    private volatile MethodHandle getter;
+
+    private static final MethodHandle NO_GETTER = MethodHandles.constant(Object.class, null);
+
     protected PropertyD(MetadataReader.ForContainer<E> reader, BeanD<?> parent) {
         super(reader, parent);
     }
@@ -96,4 +141,23 @@ public final Stream<GraphContext> read(GraphContext context) {
     }
 
     public abstract Object getValue(Object parent) throws Exception;
+
+    abstract MethodHandle unreflect(Lookup lookup) throws IllegalAccessException;
+
+    /**
+     * Get a {@code (Object)Object} handle reading this property, or {@code null} if none could be created.
+     */
+    final MethodHandle getter() {
+        MethodHandle result = getter;
+        if (result == null) {
+            try {
+                Reflection.makeAccessible((AccessibleObject) getTarget());
+                result = unreflect(MethodHandles.lookup()).asType(GETTER_TYPE);
+            } catch (RuntimeException | IllegalAccessException e) {
+                result = NO_GETTER;
+            }
+            getter = result;
+        }
+        return result == NO_GETTER ? null : result;
+    }
 }

bval-jsr/src/main/java/org/apache/bval/jsr/groups/GroupStrategy.java

@@ -36,6 +36,7 @@
 public interface GroupStrategy {
     public static class Simple implements GroupStrategy {
         private final Set<Group> groups;
+        private int hashCode;
 
         private Simple(Set<Group> groups) {
             this.groups = groups;
@@ -57,7 +58,12 @@ public GroupStrategy redefining(Map<Group, ? extends GroupStrategy> redefinition
 
         @Override
         public int hashCode() {
-            return groups.hashCode();
+            int result = hashCode;
+            if (result == 0) {
+                result = groups.hashCode();
+                hashCode = result;
+            }
+            return result;
         }
 
         @Override
@@ -75,6 +81,7 @@ public String toString() {
     public static class Composite implements GroupStrategy {
         private final Set<? extends GroupStrategy> components;
         protected final boolean ordered;
+        private int hashCode;
 
         public Composite(Collection<? extends GroupStrategy> components, boolean ordered) {
             this.components = new LinkedHashSet<>(components);
@@ -117,7 +124,12 @@ public boolean applyTo(Predicate<GroupStrategy> operation) {
 
         @Override
         public int hashCode() {
-            return Objects.hash(components, ordered);
+            int result = hashCode;
+            if (result == 0) {
+                result = Objects.hash(components, ordered);
+                hashCode = result;
+            }
+            return result;
         }
 
         @Override

bval-jsr/src/main/java/org/apache/bval/jsr/groups/Groups.java

@@ -38,6 +38,7 @@
 public class Groups {
     private final Set<Group> groups = new LinkedHashSet<>();
     private final Set<Group.Sequence> sequences = new LinkedHashSet<>();
+    private volatile GroupStrategy strategy;
 
     /**
      * Get the Groups.
@@ -65,6 +66,7 @@ public Collection<Group.Sequence> getSequences() {
      * @return success
      */
     boolean insertGroup(Group group) {
+        strategy = null;
         return groups.add(group);
     }
 
@@ -76,6 +78,7 @@ boolean insertGroup(Group group) {
      * @return success
      */
     boolean insertSequence(Collection<Group> groups) {
+        strategy = null;
         return !(groups == null || groups.isEmpty()) && sequences.add(Group.sequence(groups));
     }
 
@@ -120,9 +123,14 @@ private void ensureExpandable(List<Group> groupList, List<Group> defaultGroupLis
     }
 
     public GroupStrategy asStrategy() {
-        final List<GroupStrategy> components = new ArrayList<>();
-        components.addAll(groups);
-        components.addAll(sequences);
-        return GroupStrategy.composite(components);
+        GroupStrategy result = strategy;
+        if (result == null) {
+            final List<GroupStrategy> components = new ArrayList<>();
+            components.addAll(groups);
+            components.addAll(sequences);
+            result = GroupStrategy.composite(components);
+            strategy = result;
+        }
+        return result;
     }
 }

bval-jsr/src/main/java/org/apache/bval/jsr/groups/GroupsComputer.java

@@ -61,6 +61,9 @@ public class GroupsComputer {
     /** caching resolved groups in a thread-safe map. */
     private final Map<Class<?>, List<Group>> resolvedSequences = new ConcurrentHashMap<>();
 
+    /** Results of {@link #computeGroups(Class...)} for a single group, the common case for explicit groups. */
+    private final Map<Class<?>, Groups> singleGroups = new ConcurrentHashMap<>();
+
     /**
      * Compute groups from an array of group classes.
      * 
@@ -74,6 +77,15 @@ public final Groups computeGroups(Class<?>... groups) {
         if (groups.length == 0 || (groups.length == 1 && groups[0] == Default.class)) {
             return DEFAULT_GROUPS;
         }
+        if (groups.length == 1 && groups[0] != null) {
+            final Groups cached = singleGroups.get(groups[0]);
+            if (cached != null) {
+                return cached;
+            }
+            final Groups result = computeGroups(Arrays.asList(groups));
+            singleGroups.putIfAbsent(groups[0], result);
+            return result;
+        }
         return computeGroups(Arrays.asList(groups));
     }
 

bval-jsr/src/main/java/org/apache/bval/jsr/job/ConstraintValidatorContextImpl.java

@@ -18,8 +18,8 @@
  */
 package org.apache.bval.jsr.job;
 
-import java.util.HashSet;
-import java.util.Set;
+import java.util.ArrayList;
+import java.util.List;
 
 import jakarta.validation.ClockProvider;
 import jakarta.validation.ConstraintValidatorContext;
@@ -40,7 +40,6 @@
 import org.apache.bval.jsr.util.NodeImpl;
 import org.apache.bval.jsr.util.PathImpl;
 import org.apache.bval.util.Exceptions;
-import org.apache.bval.util.Lazy;
 import org.apache.bval.util.Validate;
 
 public class ConstraintValidatorContextImpl<T> implements ConstraintValidatorContext, ApacheMessageContext {
@@ -136,7 +135,8 @@ private PathImpl extensiblePath() {
 
     private final ValidationJob<T>.Frame<?> frame;
     private final ConstraintD<?> constraint;
-    private final Lazy<Set<ConstraintViolation<T>>> violations = new Lazy<>(HashSet::new);
+    // typically holds a single violation; a list with a containment check keeps set semantics cheaply
+    private List<ConstraintViolation<T>> violations;
     private boolean defaultConstraintViolationDisabled;
 
     ConstraintValidatorContextImpl(ValidationJob<T>.Frame<?> frame, ConstraintD<?> constraint) {
@@ -178,14 +178,14 @@ ValidationJob<T>.Frame<?> getFrame() {
         return frame;
     }
 
-    Set<ConstraintViolation<T>> getRequiredViolations() {
-        if (!violations.optional().isPresent()) {
+    List<ConstraintViolation<T>> getRequiredViolations() {
+        if (violations == null) {
             if (defaultConstraintViolationDisabled) {
                 Exceptions.raise(ValidationException::new, "Expected custom constraint violation(s)");
             }
             addError(getDefaultConstraintMessageTemplate(), frame.context.getPath());
         }
-        return violations.get();
+        return violations;
     }
 
     @Override
@@ -200,7 +200,14 @@ public Object getValidatedValue() {
 
     @SuppressWarnings({ "unchecked", "rawtypes" })
     private void addError(String messageTemplate, PathImpl propertyPath) {
-        violations.get().add(((ValidationJob) frame.getJob()).createViolation(messageTemplate, this, propertyPath));
+        if (violations == null) {
+            violations = new ArrayList<>(2);
+        }
+        final ConstraintViolation<T> violation =
+            ((ValidationJob) frame.getJob()).createViolation(messageTemplate, this, propertyPath);
+        if (!violations.contains(violation)) {
+            violations.add(violation);
+        }
     }
 
     @Override