Skip to content
Permalink
Browse files
Avoid memory leaks in child
Avoid non needed work when no validation constraints are found
Small micro-benchmarks
Fix Java 11 compilation with Java FX as an external library
  • Loading branch information
jeanouii committed Sep 22, 2020
1 parent dd3f12e commit 718ff549e5965b56a4463cfc150233b150b57d92
Showing 11 changed files with 405 additions and 64 deletions.
@@ -44,6 +44,11 @@
<jdk>11</jdk>
</activation>
<dependencies>
<dependency>
<groupId>org.apache.geronimo.specs</groupId>
<artifactId>geronimo-activation_1.1_spec</artifactId>
<version>1.1</version>
</dependency>
<dependency>
<groupId>javax.xml.bind</groupId>
<artifactId>jaxb-api</artifactId>
@@ -262,6 +267,28 @@
<version>2.2.7</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.hibernate</groupId>
<artifactId>hibernate-validator</artifactId>
<version>6.1.5.Final</version>
</dependency>

<dependency>
<groupId>org.openjdk.jmh</groupId>
<artifactId>jmh-core</artifactId>
<version>1.25.2</version>
</dependency>
<dependency>
<groupId>org.openjdk.jmh</groupId>
<artifactId>jmh-generator-annprocess</artifactId>
<version>1.25.2</version>
</dependency>
<dependency>
<groupId>com.github.biboudis</groupId>
<artifactId>jmh-profilers</artifactId>
<version>0.1.4</version>
</dependency>
</dependencies>

<build>
@@ -299,6 +326,13 @@
<source>${project.basedir}/src/main/xsd/validation-mapping-2.0.xsd</source>
</sources>
</configuration>
<dependencies>
<dependency>
<groupId>org.apache.geronimo.specs</groupId>
<artifactId>geronimo-activation_1.1_spec</artifactId>
<version>1.1</version>
</dependency>
</dependencies>
</plugin>

<!-- create mainClass attribute -->
@@ -431,6 +465,14 @@ build.timestamp=${timestamp}
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<source>9</source>
<target>9</target>
</configuration>
</plugin>
</plugins>
</build>
</project>
@@ -72,7 +72,7 @@ <T> T create(String classname) {
return newInstance(loadClass(classname));
}

<T> Set<T> loadServices(Class<T> type) {
<T> Set<T> loadServices(Class<T> type) { // todo: enable somehow to cache shared classloader (think tomcat/tomee)
Validate.notNull(type);
final Set<URL> resources = new LinkedHashSet<>();
final String resourceName = META_INF_SERVICES + type.getName();
@@ -85,7 +85,7 @@ <T> Set<T> loadServices(Class<T> type) {
log.log(Level.SEVERE, "Error searching for resource(s) " + resourceName, e);
}
}
return resources.stream().map(this::read).flatMap(Collection::stream).<T> map(this::create)
return resources.stream().distinct().map(this::read).flatMap(Collection::stream).<T> map(this::create)
.collect(ToUnmodifiable.set());
}

@@ -60,7 +60,6 @@ public static <E extends ExecutableDescriptor> boolean isConstrained(E descripto
private final ApacheValidatorFactory validatorFactory;
private final ConcurrentMap<Class<?>, BeanD<?>> beanDescriptors = new ConcurrentHashMap<>();
// synchronization unnecessary
private final Set<Class<?>> knownUnconstrainedTypes = new HashSet<>();
private final ReflectionBuilder reflectionBuilder;

public DescriptorManager(ApacheValidatorFactory validatorFactory) {
@@ -73,25 +72,13 @@ public <T> BeanDescriptor getBeanDescriptor(Class<T> beanClass) {
Validate.notNull(beanClass, IllegalArgumentException::new, "beanClass");

// cannot use computeIfAbsent due to recursion being the usual case:
if (beanDescriptors.containsKey(beanClass)) {
return beanDescriptors.get(beanClass);
final BeanD<?> existing = beanDescriptors.get(beanClass);
if (existing != null) {
return existing;
}
final boolean constrained = !knownUnconstrainedTypes.contains(beanClass);
final MetadataBuilder.ForBean<T> builder = constrained ? builder(beanClass) : EmptyBuilder.instance().forBean();
final BeanD<T> beanD = new BeanD<>(new MetadataReader(validatorFactory, beanClass).forBean(builder));

if (constrained) {
// if not previously known to be unconstrained, check:
if (beanD.isBeanConstrained() || !(beanD.getConstrainedConstructors().isEmpty()
&& beanD.getConstrainedMethods(MethodType.GETTER, MethodType.NON_GETTER).isEmpty())) {
@SuppressWarnings("unchecked")
final BeanD<T> result =
Optional.ofNullable((BeanD<T>) beanDescriptors.putIfAbsent(beanClass, beanD)).orElse(beanD);
return result;
}
}
knownUnconstrainedTypes.add(beanClass);
return beanD;
final BeanD<?> value = new BeanD<>(new MetadataReader(validatorFactory, beanClass).forBean(builder(beanClass)));
final BeanD<?> previous = beanDescriptors.putIfAbsent(beanClass, value);
return previous == null ? value : previous;
}

public void clear() {
@@ -24,8 +24,11 @@
import org.apache.bval.jsr.descriptor.BeanD;
import org.apache.bval.jsr.descriptor.ConstraintD;
import org.apache.bval.jsr.util.PathImpl;
import org.apache.bval.jsr.util.Proxies;
import org.apache.bval.util.Validate;

import java.util.Map;

public final class ValidateBean<T> extends ValidationJob<T> {

private final T bean;
@@ -35,6 +38,20 @@
this.bean = Validate.notNull(bean, IllegalArgumentException::new, "bean");
}

@Override
protected boolean hasWork() {
final Class<?> beanClass = bean.getClass();
final Map<Class<?>, Class<?>> classCache = validatorContext.getFactory().getUnwrappedClassCache();
Class<?> unwrappedClass = classCache.get(beanClass);
if (unwrappedClass == null) {
unwrappedClass = Proxies.classFor(beanClass);
classCache.putIfAbsent(beanClass, unwrappedClass);
}
return validatorContext.getFactory().getDescriptorManager()
.getBeanDescriptor(unwrappedClass)
.isBeanConstrained();
}

@Override
protected Frame<BeanD<T>> computeBaseFrame() {
return new BeanFrame<T>(new GraphContext(validatorContext, PathImpl.create(), bean));
@@ -185,7 +185,8 @@ protected Frame<?> computeBaseFrame() {

@Override
protected boolean hasWork() {
return describe() != null;
final ExecutableDescriptor descriptor = describe();
return descriptor != null && descriptor.hasConstrainedParameters();
}

protected abstract ExecutableDescriptor describe();
@@ -141,7 +141,8 @@ ConstraintViolationImpl<T> createViolation(String messageTemplate, String messag

@Override
protected boolean hasWork() {
return describe() != null;
final ExecutableDescriptor descriptor = describe();
return descriptor != null && descriptor.hasConstrainedReturnValue();
}

protected abstract ExecutableDescriptor describe();
@@ -561,7 +561,6 @@ public final Set<ConstraintViolation<T>> getResults() {
final Consumer<ConstraintViolation<T>> sink = results.consumer(Set::add);

completedValidations = new ConcurrentHashMap<>();

try {
baseFrame.process(groups.asStrategy(), sink);
} finally {
@@ -16,6 +16,21 @@
*/
package org.apache.bval.jsr.valueextraction;

import org.apache.bval.jsr.metadata.ContainerElementKey;
import org.apache.bval.util.Exceptions;
import org.apache.bval.util.ObjectUtils;
import org.apache.bval.util.StringUtils;
import org.apache.bval.util.Validate;
import org.apache.bval.util.reflection.Reflection;
import org.apache.bval.util.reflection.Reflection.Interfaces;
import org.apache.bval.util.reflection.TypeUtils;

import javax.validation.ConstraintDeclarationException;
import javax.validation.metadata.ValidateUnwrappedValue;
import javax.validation.valueextraction.UnwrapByDefault;
import javax.validation.valueextraction.ValueExtractor;
import javax.validation.valueextraction.ValueExtractorDeclarationException;
import javax.validation.valueextraction.ValueExtractorDefinitionException;
import java.io.IOException;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Type;
@@ -31,31 +46,14 @@
import java.util.Properties;
import java.util.Set;
import java.util.TreeMap;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.BooleanSupplier;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import javax.validation.ConstraintDeclarationException;
import javax.validation.metadata.ValidateUnwrappedValue;
import javax.validation.valueextraction.UnwrapByDefault;
import javax.validation.valueextraction.ValueExtractor;
import javax.validation.valueextraction.ValueExtractorDeclarationException;
import javax.validation.valueextraction.ValueExtractorDefinitionException;

import org.apache.bval.jsr.metadata.ContainerElementKey;
import org.apache.bval.util.Exceptions;
import org.apache.bval.util.Lazy;
import org.apache.bval.util.ObjectUtils;
import org.apache.bval.util.StringUtils;
import org.apache.bval.util.Validate;
import org.apache.bval.util.reflection.Reflection;
import org.apache.bval.util.reflection.Reflection.Interfaces;
import org.apache.bval.util.reflection.TypeUtils;

/**
* {@link ValueExtractor} collection of some level of a bean validation hierarchy.
*/
@@ -222,9 +220,8 @@ private static <T> Optional<T> maximallySpecific(Collection<T> candidates, Funct
}

private final ValueExtractors parent;
private final Lazy<Map<ContainerElementKey, ValueExtractor<?>>> valueExtractors = new Lazy<>(TreeMap::new);
private final Lazy<Set<ValueExtractors>> children = new Lazy<>(HashSet::new);
private final Lazy<Map<ContainerElementKey, ValueExtractor<?>>> searchCache = new Lazy<>(HashMap::new);
private final Map<ContainerElementKey, ValueExtractor<?>> valueExtractors = new ConcurrentHashMap<>();
private final Map<ContainerElementKey, ValueExtractor<?>> searchCache = new ConcurrentHashMap<>();
private final OnDuplicateContainerElementKey onDuplicateContainerElementKey;

public ValueExtractors() {
@@ -243,17 +240,16 @@ private ValueExtractors(ValueExtractors parent, OnDuplicateContainerElementKey o
private ValueExtractors(ValueExtractors parent, OnDuplicateContainerElementKey onDuplicateContainerElementKey,
Map<ContainerElementKey, ValueExtractor<?>> backingMap) {
this(parent, onDuplicateContainerElementKey);
this.valueExtractors.reset(backingMap);
this.valueExtractors.clear();
this.valueExtractors.putAll(backingMap);
}

public ValueExtractors createChild() {
return createChild(OnDuplicateContainerElementKey.EXCEPTION);
}

public ValueExtractors createChild(OnDuplicateContainerElementKey onDuplicateContainerElementKey) {
final ValueExtractors child = new ValueExtractors(this, onDuplicateContainerElementKey);
children.get().add(child);
return child;
return new ValueExtractors(this, onDuplicateContainerElementKey);
}

public void add(ValueExtractor<?> extractor) {
@@ -262,7 +258,7 @@ public void add(ValueExtractor<?> extractor) {
Exceptions.raise(IllegalStateException::new, "Computed null %s for %s",
ContainerElementKey.class.getSimpleName(), extractor);
}
final Map<ContainerElementKey, ValueExtractor<?>> m = valueExtractors.get();
final Map<ContainerElementKey, ValueExtractor<?>> m = valueExtractors;
if (onDuplicateContainerElementKey == OnDuplicateContainerElementKey.EXCEPTION) {
synchronized (this) {
if (m.containsKey(key)) {
@@ -274,19 +270,19 @@ public void add(ValueExtractor<?> extractor) {
} else {
m.put(key, extractor);
}
children.optional().ifPresent(s -> s.stream().forEach(ValueExtractors::clearCache));
searchCache.clear();
}

public Map<ContainerElementKey, ValueExtractor<?>> getValueExtractors() {
final Lazy<Map<ContainerElementKey, ValueExtractor<?>>> result = new Lazy<>(HashMap::new);
final Map<ContainerElementKey, ValueExtractor<?>> result = new HashMap<>();
populate(result);
return result.optional().orElseGet(Collections::emptyMap);
return result;
}

public ValueExtractor<?> find(ContainerElementKey key) {
final Optional<ValueExtractor<?>> cacheHit = searchCache.optional().map(m -> m.get(key));
if (cacheHit.isPresent()) {
return cacheHit.get();
final ValueExtractor<?> cacheHit = searchCache.get(key);
if (cacheHit != null) {
return cacheHit;
}
final Map<ContainerElementKey, ValueExtractor<?>> allValueExtractors = getValueExtractors();
if (allValueExtractors.containsKey(key)) {
@@ -299,7 +295,7 @@ public ValueExtractor<?> find(ContainerElementKey key) {
final Optional<ValueExtractor<?>> result =
maximallySpecific(candidates.keySet(), ve -> candidates.get(ve).getContainerClass());
if (result.isPresent()) {
searchCache.get().put(key, result.get());
searchCache.put(key, result.get());
return result.get();
}
throw Exceptions.create(ConstraintDeclarationException::new, "Could not determine %s for %s",
@@ -329,12 +325,11 @@ public Optional<UnwrappingInfo> findUnwrappingInfo(Class<?> containerClass,
return result;
}

private void populate(Supplier<Map<ContainerElementKey, ValueExtractor<?>>> target) {
Optional.ofNullable(parent).ifPresent(p -> p.populate(target));
valueExtractors.optional().ifPresent(m -> target.get().putAll(m));
private void populate(Map<ContainerElementKey, ValueExtractor<?>> target) {
if (parent != null) {
parent.populate(target);
}
target.putAll(valueExtractors);
}

private void clearCache() {
searchCache.optional().ifPresent(Map::clear);
}
}
@@ -43,6 +43,11 @@ private static class Parameterized extends EmulatedAnnotatedType<ParameterizedTy
public AnnotatedType[] getAnnotatedActualTypeArguments() {
return wrapArray(wrapped.getActualTypeArguments());
}

@Override
public AnnotatedType getAnnotatedOwnerType() {
return null;
}
}

private static class Variable extends EmulatedAnnotatedType<TypeVariable<?>> implements AnnotatedTypeVariable {
@@ -55,6 +60,11 @@ private static class Variable extends EmulatedAnnotatedType<TypeVariable<?>> imp
public AnnotatedType[] getAnnotatedBounds() {
return wrapped.getAnnotatedBounds();
}

@Override
public AnnotatedType getAnnotatedOwnerType() {
return null;
}
}

private static class Wildcard extends EmulatedAnnotatedType<WildcardType> implements AnnotatedWildcardType {
@@ -72,6 +82,11 @@ public AnnotatedType[] getAnnotatedLowerBounds() {
public AnnotatedType[] getAnnotatedUpperBounds() {
return wrapArray(wrapped.getUpperBounds());
}

@Override
public AnnotatedType getAnnotatedOwnerType() {
return null;
}
}

public static EmulatedAnnotatedType<?> wrap(Type type) {
@@ -178,7 +178,7 @@ public void testDescriptorCaching() {
// unconstrained
final BeanDescriptor objectDescriptor = validator.getConstraintsForClass(Object.class);
assertNotNull(objectDescriptor);
assertNotSame(objectDescriptor, validator.getConstraintsForClass(Object.class));
assertSame(objectDescriptor, validator.getConstraintsForClass(Object.class));
}

public static class Form {

0 comments on commit 718ff54

Please sign in to comment.