You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bval.apache.org by rm...@apache.org on 2019/04/22 20:27:39 UTC

[bval] branch master updated: BVAL-172 dropping LRUCache since it is not thread safe and cache size is bounded by deployed classes anyway, to enhance if needed

This is an automated email from the ASF dual-hosted git repository.

rmannibucau pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bval.git


The following commit(s) were added to refs/heads/master by this push:
     new e5395c9  BVAL-172 dropping LRUCache since it is not thread safe and cache size is bounded by deployed classes anyway, to enhance if needed
e5395c9 is described below

commit e5395c9aa58c3edb1f9a7eed4e3b03146b3a633f
Author: Romain Manni-Bucau <rm...@apache.org>
AuthorDate: Mon Apr 22 22:27:31 2019 +0200

    BVAL-172 dropping LRUCache since it is not thread safe and cache size is bounded by deployed classes anyway, to enhance if needed
---
 .../bval/jsr/ApacheValidatorConfiguration.java     |  5 ---
 .../org/apache/bval/jsr/ConfigurationImpl.java     |  5 ---
 .../bval/jsr/descriptor/CascadableContainerD.java  | 14 ++++----
 .../bval/jsr/util/AnnotationProxyBuilder.java      |  9 +++--
 .../apache/bval/jsr/util/AnnotationsManager.java   | 25 +++++--------
 .../java/org/apache/bval/jsr/util/LRUCache.java    | 41 ----------------------
 6 files changed, 18 insertions(+), 81 deletions(-)

diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheValidatorConfiguration.java b/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheValidatorConfiguration.java
index 7c82c8b..c2167d6 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheValidatorConfiguration.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheValidatorConfiguration.java
@@ -46,11 +46,6 @@ public interface ApacheValidatorConfiguration extends Configuration<ApacheValida
         String VALIDATOR_FACTORY_CLASSNAME = "apache.bval.validator-factory-classname";
 
         /**
-         * Size to use for caching of constraint-related information. Default is {@code 50}.
-         */
-        String CONSTRAINTS_CACHE_SIZE = "apache.bval.constraints-cache-size";
-
-        /**
          * Specifies whether EL evaluation is permitted in non-default message
          * templates. By default this feature is disabled; if you enable it you
          * should ensure that no constraint validator builds violations using
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/ConfigurationImpl.java b/bval-jsr/src/main/java/org/apache/bval/jsr/ConfigurationImpl.java
index 4638722..0bc02aa 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/ConfigurationImpl.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/ConfigurationImpl.java
@@ -179,7 +179,6 @@ public class ConfigurationImpl implements ApacheValidatorConfiguration, Configur
             this.provider = aProvider;
             this.providerResolver = null;
         }
-        initializePropertyDefaults();
     }
 
     /**
@@ -466,10 +465,6 @@ public class ConfigurationImpl implements ApacheValidatorConfiguration, Configur
         }
     }
 
-    private void initializePropertyDefaults() {
-        properties.put(Properties.CONSTRAINTS_CACHE_SIZE, Integer.toString(50));
-    }
-
     private ValidationProvider<?> findProvider() {
         if (providerClass == null) {
             return providerResolver.getValidationProviders().get(0);
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/CascadableContainerD.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/CascadableContainerD.java
index ebb3c33..a9c5539 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/CascadableContainerD.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/CascadableContainerD.java
@@ -39,14 +39,12 @@ public abstract class CascadableContainerD<P extends ElementD<?, ?>, E extends A
 
     protected CascadableContainerD(MetadataReader.ForContainer<E> reader, P parent) {
         super(reader, parent);
-        synchronized (reader.meta.getHost()) { // cache is not thread safe for runtime perf so ensure we lock properly
-            cascaded = reader.isCascaded();
-            groupConversions = reader.getGroupConversions();
-            containerElementTypes = reader.getContainerElementTypes(this)
-                                          .stream()
-                                          .filter(DescriptorManager::isConstrained)
-                                          .collect(ToUnmodifiable.set());
-        }
+        cascaded = reader.isCascaded();
+        groupConversions = reader.getGroupConversions();
+        containerElementTypes = reader.getContainerElementTypes(this)
+                                      .stream()
+                                      .filter(DescriptorManager::isConstrained)
+                                      .collect(ToUnmodifiable.set());
     }
 
     @Override
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationProxyBuilder.java b/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationProxyBuilder.java
index cfa4c48..6d21098 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationProxyBuilder.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationProxyBuilder.java
@@ -24,6 +24,7 @@ import java.lang.reflect.Proxy;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
+import java.util.concurrent.ConcurrentMap;
 
 import javax.enterprise.util.AnnotationLiteral;
 import javax.validation.ConstraintTarget;
@@ -57,11 +58,9 @@ public final class AnnotationProxyBuilder<A extends Annotation> {
      * @param annotationType
      * @param cache
      */
-    AnnotationProxyBuilder(final Class<A> annotationType, Map<Class<?>, Method[]> cache) {
+    AnnotationProxyBuilder(final Class<A> annotationType, ConcurrentMap<Class<?>, Method[]> cache) {
         this.type = Validate.notNull(annotationType, "annotationType");
-        synchronized (annotationType) { // cache is not thread safe generally
-            this.methods = Validate.notNull(cache, "cache").computeIfAbsent(annotationType, Reflection::getDeclaredMethods);
-        }
+        this.methods = Validate.notNull(cache, "cache").computeIfAbsent(annotationType, Reflection::getDeclaredMethods);
     }
 
     /**
@@ -73,7 +72,7 @@ public final class AnnotationProxyBuilder<A extends Annotation> {
      * @param cache
      */
     @SuppressWarnings("unchecked")
-    AnnotationProxyBuilder(A annot, Map<Class<?>, Method[]> cache) {
+    AnnotationProxyBuilder(A annot, ConcurrentMap<Class<?>, Method[]> cache) {
         this((Class<A>) annot.annotationType(), cache);
         elements.putAll(AnnotationsManager.readAttributes(annot));
     }
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationsManager.java b/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationsManager.java
index dd9c28b..39ca237 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationsManager.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationsManager.java
@@ -36,6 +36,8 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Function;
 import java.util.stream.Collectors;
@@ -51,7 +53,6 @@ import javax.validation.ValidationException;
 import javax.validation.constraintvalidation.ValidationTarget;
 
 import org.apache.bval.jsr.ApacheValidatorFactory;
-import org.apache.bval.jsr.ConfigurationImpl;
 import org.apache.bval.jsr.ConstraintAnnotationAttributes;
 import org.apache.bval.jsr.ConstraintAnnotationAttributes.Worker;
 import org.apache.bval.jsr.ConstraintCached.ConstraintValidatorInfo;
@@ -307,24 +308,14 @@ public class AnnotationsManager {
     }
 
     private final ApacheValidatorFactory validatorFactory;
-    private final LRUCache<Class<? extends Annotation>, Composition> compositions;
-    private final LRUCache<Class<? extends Annotation>, Method[]> constraintAttributes;
+    private final ConcurrentMap<Class<?>, Composition> compositions;
+    private final ConcurrentMap<Class<?>, Method[]> constraintAttributes;
 
     public AnnotationsManager(ApacheValidatorFactory validatorFactory) {
         super();
         this.validatorFactory = Validate.notNull(validatorFactory);
-        final String cacheSize =
-            validatorFactory.getProperties().get(ConfigurationImpl.Properties.CONSTRAINTS_CACHE_SIZE);
-        final int sz;
-        try {
-            sz = Integer.parseInt(cacheSize);
-        } catch (NumberFormatException e) {
-            throw Exceptions.create(IllegalStateException::new, e,
-                "Cannot parse value %s for configuration property %s", cacheSize,
-                ConfigurationImpl.Properties.CONSTRAINTS_CACHE_SIZE);
-        }
-        compositions = new LRUCache<>(sz);
-        constraintAttributes = new LRUCache<>(sz);
+        compositions = new ConcurrentHashMap<>();
+        constraintAttributes = new ConcurrentHashMap<>();
     }
 
     public void validateConstraintDefinition(Class<? extends Annotation> type) {
@@ -418,12 +409,12 @@ public class AnnotationsManager {
 
     @SuppressWarnings({ "unchecked", "rawtypes" })
     public <A extends Annotation> AnnotationProxyBuilder<A> buildProxyFor(Class<A> type) {
-        return new AnnotationProxyBuilder<>(type, (Map) constraintAttributes);
+        return new AnnotationProxyBuilder<>(type, constraintAttributes);
     }
 
     @SuppressWarnings({ "unchecked", "rawtypes" })
     public <A extends Annotation> AnnotationProxyBuilder<A> buildProxyFor(A instance) {
-        return new AnnotationProxyBuilder<>(instance, (Map) constraintAttributes);
+        return new AnnotationProxyBuilder<>(instance, constraintAttributes);
     }
 
     private Composition getComposition(Class<? extends Annotation> annotationType) {
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/util/LRUCache.java b/bval-jsr/src/main/java/org/apache/bval/jsr/util/LRUCache.java
deleted file mode 100644
index 112ddf2..0000000
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/util/LRUCache.java
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.bval.jsr.util;
-
-import java.util.LinkedHashMap;
-import java.util.Map;
-
-public class LRUCache<K, V> extends LinkedHashMap<K, V> {
-    private static final long serialVersionUID = 1L;
-
-    private final int maximumCapacity;
-
-    public LRUCache(int maximumCapacity) {
-        super(16, 0.75f, true);
-        if (maximumCapacity < 1) {
-            throw new IllegalArgumentException("maximumCapacity must be > 0");
-        }
-        this.maximumCapacity = maximumCapacity;
-    }
-
-    @Override
-    protected boolean removeEldestEntry(Map.Entry<K, V> eldest) { // todo: synchronize?
-        return super.removeEldestEntry(eldest) || size() >= maximumCapacity;
-    }
-}