From b6c8e498a521d56593fb6b2d55cf3b51591462b6 Mon Sep 17 00:00:00 2001 From: XenoAmess Date: Sun, 31 May 2020 15:12:24 +0800 Subject: [PATCH 1/8] add tests to show how slow it is. please run it using Jprofiler or other tools to see cpu call tree. --- .../performance/WeakFastHashMapTest.java | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 src/test/java/org/apache/commons/beanutils2/performance/WeakFastHashMapTest.java diff --git a/src/test/java/org/apache/commons/beanutils2/performance/WeakFastHashMapTest.java b/src/test/java/org/apache/commons/beanutils2/performance/WeakFastHashMapTest.java new file mode 100644 index 000000000..aeabee33c --- /dev/null +++ b/src/test/java/org/apache/commons/beanutils2/performance/WeakFastHashMapTest.java @@ -0,0 +1,93 @@ +/* + * 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.commons.beanutils2.performance; + +import org.apache.commons.beanutils2.WeakFastHashMap; + +import java.util.Objects; +import java.util.Random; +import java.util.TreeSet; +import java.util.concurrent.ConcurrentHashMap; + +public class WeakFastHashMapTest { + static final TreeSet treeSet = new TreeSet<>(); + static final Random random = new Random(); + static final WeakFastHashMap weakFastHashMap = new WeakFastHashMap<>(); + static final ConcurrentHashMap concurrentHashMap = new ConcurrentHashMap<>(); + + static final int INIT_SIZE = 50000; + static final double WRITE_CHANCE = 0.01; + static final double READ_NON_EXIST_CHANCE = 0.30; + + public static void main(String[] args) { + for (int i = 0; i < INIT_SIZE; i++) { + writeRandom(); + } + System.out.println("init over"); + + weakFastHashMap.setFast(true); + + for (int i = 0; ; i++) { + if (i % 100000 == 0) { + System.out.println("running> i : " + i + " size: " + treeSet.size()); + } + double ifWrite = random.nextDouble(); + if (ifWrite < WRITE_CHANCE) { + writeRandom(); + } else { + double ifNonExist = random.nextDouble(); + if (ifNonExist < READ_NON_EXIST_CHANCE) { + readNonExist(); + } else { + readExist(); + } + } + } + } + + public static void writeRandom() { + Integer nowKey = random.nextInt() * random.nextInt(); + Integer nowValue = random.nextInt() * random.nextInt(); + treeSet.add(nowKey); + weakFastHashMap.put(nowKey, nowValue); + concurrentHashMap.put(nowKey, nowValue); + } + + public static void readExist() { + Integer nowKey = null; + while (nowKey == null) { + nowKey = treeSet.lower(random.nextInt() * random.nextInt()); + } + read(nowKey); + } + + public static void readNonExist() { + Integer nowKey = random.nextInt() * random.nextInt(); + while (treeSet.contains(nowKey)) { + nowKey = random.nextInt() * random.nextInt(); + } + read(nowKey); + } + + public static void read(Integer nowKey) { + Integer value1 = weakFastHashMap.get(nowKey); + Integer value2 = concurrentHashMap.get(nowKey); + if (!Objects.equals(value1, value2)) { + System.out.println("not equal! nowKey : " + nowKey + " value1 : " + value1 + " value2 : " + value2); + } + } +} From 2c3e1085cb3a6defc1674ec96a0e5edde2e49b4d Mon Sep 17 00:00:00 2001 From: XenoAmess Date: Sun, 31 May 2020 15:54:18 +0800 Subject: [PATCH 2/8] try remove the use of WeakFastHashMap. --- .../org/apache/commons/beanutils2/BeanUtils.java | 4 ++-- .../apache/commons/beanutils2/ConvertUtilsBean.java | 9 +++++---- .../apache/commons/beanutils2/PropertyUtilsBean.java | 12 ++++++------ 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/apache/commons/beanutils2/BeanUtils.java b/src/main/java/org/apache/commons/beanutils2/BeanUtils.java index 0a1698e9d..73fb377dc 100644 --- a/src/main/java/org/apache/commons/beanutils2/BeanUtils.java +++ b/src/main/java/org/apache/commons/beanutils2/BeanUtils.java @@ -20,7 +20,7 @@ import java.lang.reflect.InvocationTargetException; import java.util.Map; - +import java.util.concurrent.ConcurrentHashMap; /** @@ -436,6 +436,6 @@ public static boolean initCause(final Throwable throwable, final Throwable cause * @since 1.8.0 */ public static Map createCache() { - return new WeakFastHashMap<>(); + return new ConcurrentHashMap<>(); } } diff --git a/src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java b/src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java index c793a9e8c..acf3ea6d2 100644 --- a/src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java +++ b/src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java @@ -27,6 +27,7 @@ import java.sql.Timestamp; import java.util.Calendar; import java.util.Collection; +import java.util.concurrent.ConcurrentHashMap; import org.apache.commons.beanutils2.converters.ArrayConverter; import org.apache.commons.beanutils2.converters.BigDecimalConverter; @@ -144,8 +145,8 @@ protected static ConvertUtilsBean getInstance() { * The set of {@link Converter}s that can be used to convert Strings * into objects of a specified Class, keyed by the destination Class. */ - private final WeakFastHashMap, Converter> converters = - new WeakFastHashMap<>(); + private final ConcurrentHashMap, Converter> converters = + new ConcurrentHashMap<>(); /** * The Log instance for this class. @@ -156,9 +157,9 @@ protected static ConvertUtilsBean getInstance() { /** Construct a bean with standard converters registered */ public ConvertUtilsBean() { - converters.setFast(false); +// converters.setFast(false); deregister(); - converters.setFast(true); +// converters.setFast(true); } // --------------------------------------------------------- Public Methods diff --git a/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java b/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java index dbc3c3670..748115d2e 100644 --- a/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java +++ b/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java @@ -110,8 +110,8 @@ protected static PropertyUtilsBean getInstance() { * The cache of PropertyDescriptor arrays for beans we have already * introspected, keyed by the java.lang.Class of this object. */ - private WeakFastHashMap, BeanIntrospectionData> descriptorsCache = null; - private WeakFastHashMap, Map> mappedDescriptorsCache = null; + private ConcurrentHashMap, BeanIntrospectionData> descriptorsCache = null; + private ConcurrentHashMap, Map> mappedDescriptorsCache = null; /** An empty object array */ private static final Object[] EMPTY_OBJECT_ARRAY = new Object[0]; @@ -126,10 +126,10 @@ protected static PropertyUtilsBean getInstance() { /** Base constructor */ public PropertyUtilsBean() { - descriptorsCache = new WeakFastHashMap<>(); - descriptorsCache.setFast(true); - mappedDescriptorsCache = new WeakFastHashMap<>(); - mappedDescriptorsCache.setFast(true); + descriptorsCache = new ConcurrentHashMap<>(); +// descriptorsCache.setFast(true); + mappedDescriptorsCache = new ConcurrentHashMap<>(); +// mappedDescriptorsCache.setFast(true); introspectors = new CopyOnWriteArrayList<>(); resetBeanIntrospectors(); } From 54711ac635dabffe7a37d8927f3aaa1a9bd5b80d Mon Sep 17 00:00:00 2001 From: XenoAmess Date: Sun, 31 May 2020 20:23:24 +0800 Subject: [PATCH 3/8] change to org.apache.commons.logging.impl.WeakHashtable --- .../apache/commons/beanutils2/ConvertUtilsBean.java | 6 ++++-- .../apache/commons/beanutils2/PropertyUtilsBean.java | 9 +++++---- .../beanutils2/performance/WeakFastHashMapTest.java | 10 ++++++---- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java b/src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java index 0ed2c5ee2..ca0977083 100644 --- a/src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java +++ b/src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java @@ -25,6 +25,7 @@ import java.net.URL; import java.nio.file.Path; import java.sql.Timestamp; +import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.time.Duration; import java.time.LocalDate; @@ -83,6 +84,7 @@ import org.apache.commons.beanutils2.converters.ZonedDateTimeConverter; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.commons.logging.impl.WeakHashtable; /** *

Utility methods for converting String scalar values to objects of the @@ -191,8 +193,8 @@ protected static ConvertUtilsBean getInstance() { * The set of {@link Converter}s that can be used to convert Strings * into objects of a specified Class, keyed by the destination Class. */ - private final ConcurrentHashMap, Converter> converters = - new ConcurrentHashMap<>(); + private final Map, Converter> converters = + new WeakHashtable(); /** * The {@code Log} instance for this class. diff --git a/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java b/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java index 45eae85c2..506ae2adf 100644 --- a/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java +++ b/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java @@ -34,6 +34,7 @@ import org.apache.commons.beanutils2.expression.Resolver; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.commons.logging.impl.WeakHashtable; /** * Utility methods for using Java Reflection APIs to facilitate generic @@ -108,8 +109,8 @@ protected static PropertyUtilsBean getInstance() { * The cache of PropertyDescriptor arrays for beans we have already * introspected, keyed by the java.lang.Class of this object. */ - private ConcurrentHashMap, BeanIntrospectionData> descriptorsCache = null; - private ConcurrentHashMap, Map> mappedDescriptorsCache = null; + private Map, BeanIntrospectionData> descriptorsCache = null; + private Map, Map> mappedDescriptorsCache = null; /** An empty object array */ private static final Object[] EMPTY_OBJECT_ARRAY = new Object[0]; @@ -124,9 +125,9 @@ protected static PropertyUtilsBean getInstance() { /** Base constructor */ public PropertyUtilsBean() { - descriptorsCache = new ConcurrentHashMap<>(); + descriptorsCache = new WeakHashtable(); // descriptorsCache.setFast(true); - mappedDescriptorsCache = new ConcurrentHashMap<>(); + mappedDescriptorsCache = new WeakHashtable(); // mappedDescriptorsCache.setFast(true); introspectors = new CopyOnWriteArrayList<>(); resetBeanIntrospectors(); diff --git a/src/test/java/org/apache/commons/beanutils2/performance/WeakFastHashMapTest.java b/src/test/java/org/apache/commons/beanutils2/performance/WeakFastHashMapTest.java index aeabee33c..5f21c17e9 100644 --- a/src/test/java/org/apache/commons/beanutils2/performance/WeakFastHashMapTest.java +++ b/src/test/java/org/apache/commons/beanutils2/performance/WeakFastHashMapTest.java @@ -17,17 +17,19 @@ package org.apache.commons.beanutils2.performance; import org.apache.commons.beanutils2.WeakFastHashMap; +import org.apache.commons.logging.impl.WeakHashtable; +import java.util.Map; import java.util.Objects; import java.util.Random; import java.util.TreeSet; -import java.util.concurrent.ConcurrentHashMap; + public class WeakFastHashMapTest { static final TreeSet treeSet = new TreeSet<>(); static final Random random = new Random(); static final WeakFastHashMap weakFastHashMap = new WeakFastHashMap<>(); - static final ConcurrentHashMap concurrentHashMap = new ConcurrentHashMap<>(); + static final Map weakHashtable = new WeakHashtable(); static final int INIT_SIZE = 50000; static final double WRITE_CHANCE = 0.01; @@ -64,7 +66,7 @@ public static void writeRandom() { Integer nowValue = random.nextInt() * random.nextInt(); treeSet.add(nowKey); weakFastHashMap.put(nowKey, nowValue); - concurrentHashMap.put(nowKey, nowValue); + weakHashtable.put(nowKey, nowValue); } public static void readExist() { @@ -85,7 +87,7 @@ public static void readNonExist() { public static void read(Integer nowKey) { Integer value1 = weakFastHashMap.get(nowKey); - Integer value2 = concurrentHashMap.get(nowKey); + Integer value2 = weakHashtable.get(nowKey); if (!Objects.equals(value1, value2)) { System.out.println("not equal! nowKey : " + nowKey + " value1 : " + value1 + " value2 : " + value2); } From 807209e65c47ac914e9b882e4e09d0f69b21f08f Mon Sep 17 00:00:00 2001 From: XenoAmess Date: Sun, 31 May 2020 22:05:14 +0800 Subject: [PATCH 4/8] change to some other ConcurrentWeakHashMap --- .../beanutils2/ConcurrentWeakHashMap.java | 383 +++++++++ .../commons/beanutils2/ConvertUtilsBean.java | 4 +- .../beanutils2/OriginalWeakFastHashMap.java | 746 ++++++++++++++++++ .../commons/beanutils2/PropertyUtilsBean.java | 6 +- .../performance/WeakFastHashMapTest.java | 5 +- 5 files changed, 1137 insertions(+), 7 deletions(-) create mode 100644 src/main/java/org/apache/commons/beanutils2/ConcurrentWeakHashMap.java create mode 100644 src/main/java/org/apache/commons/beanutils2/OriginalWeakFastHashMap.java diff --git a/src/main/java/org/apache/commons/beanutils2/ConcurrentWeakHashMap.java b/src/main/java/org/apache/commons/beanutils2/ConcurrentWeakHashMap.java new file mode 100644 index 000000000..ed78a1b84 --- /dev/null +++ b/src/main/java/org/apache/commons/beanutils2/ConcurrentWeakHashMap.java @@ -0,0 +1,383 @@ +/* + * 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.commons.beanutils2; + +import java.util.AbstractMap; +import java.util.Iterator; +import java.util.NoSuchElementException; +import java.util.Set; +import java.util.WeakHashMap; +import java.util.concurrent.ConcurrentMap; + +/** + * org.apache.commons.beanutils2.ConcurrentWeakHashMap from https://issues.apache.org/jira/browse/HARMONY-6434?jql=text%20~%20%22concurrent%20weak%20hash%20map%22 + * donated by James Gan + */ +public class ConcurrentWeakHashMap extends AbstractMap implements + ConcurrentMap { + /* ---------------- Constants -------------- */ + /** + * The default initial capacity for this table, used when not otherwise + * specified in a constructor. + */ + static final int DEFAULT_INITIAL_CAPACITY = 16; + + /** + * The default load factor for this table, used when not otherwise specified + * in a constructor. + */ + static final float DEFAULT_LOAD_FACTOR = 0.75f; + + /** + * The default concurrency level for this table, used when not otherwise + * specified in a constructor. + */ + static final int DEFAULT_CONCURRENCY_LEVEL = 16; + + /** + * The maximum capacity, used if a higher value is implicitly specified by + * either of the constructors with arguments. MUST be a power of two <= + * 1<<30 to ensure that entries are indexable using ints. + */ + static final int MAXIMUM_CAPACITY = 1 << 30; + + /** + * The maximum number of segments to allow; used to bound constructor + * arguments. + */ + static final int MAX_SEGMENTS = 1 << 16; // slightly conservative + + /* ---------------- Fields -------------- */ + + /** + * Mask value for indexing into segments. The upper bits of a key's hash + * code are used to choose the segment. + */ + final int segmentMask; + + /** + * Shift value for indexing within segments. + */ + final int segmentShift; + + final WeakHashMap[] segments; + + transient Set keySet; + + /** + * Generate hash code for input object in order to map it to different + * segments. + */ + private static int hash(Object o) { + if (o == null) + return 0; + int hashCode = o.hashCode(); + hashCode ^= (hashCode << 7); + hashCode ^= (hashCode >>> 3); + hashCode ^= (hashCode << 27); + hashCode ^= (hashCode >>> 15); + return hashCode; + } + + /** + * Returns the segment that should be used for key with given hash + * + * @param hash + * the hash code for the key + * @return the segment + */ + final private WeakHashMap segmentFor(int hash) { + return segments[(hash >>> segmentShift) & segmentMask]; + } + + /** + * Constructs a new, empty HashMap with the specified initial capacity and + * the specified load factor. + * + * @param initialCapacity + * the initial capacity of the HashMap. + * @param loadFactor + * a number between 0.0 and 1.0. + * @param concurrencyLevel + * the estimated number of concurrently updating threads. The + * implementation performs internal sizing to try to accommodate + * this many threads. + * @throws IllegalArgumentException + * if neither keys nor values use hard references, if the + * initial capacity is less than or equal to zero, or if the + * load factor is less than or equal to zero + */ + @SuppressWarnings("unchecked") + public ConcurrentWeakHashMap(int initialCapacity, float loadFactor, + int concurrencyLevel) { + if (initialCapacity < 0) { + throw new IllegalArgumentException("Illegal Initial Capacity: " + + initialCapacity); + } + + if ((loadFactor >= 1) || (loadFactor <= 0)) { + throw new IllegalArgumentException("Illegal Load factor: " + + loadFactor); + } + + if (concurrencyLevel <= 1) + throw new IllegalArgumentException("Illegal concurrencyLevel: " + + concurrencyLevel); + + if (concurrencyLevel > MAX_SEGMENTS) + concurrencyLevel = MAX_SEGMENTS; + + // Find power-of-two sizes best matching arguments + int sshift = 0; + int ssize = 1; + while (ssize < concurrencyLevel) { + ++sshift; + ssize <<= 1; + } + segmentShift = 32 - sshift; + segmentMask = ssize - 1; + + this.segments = new WeakHashMap[ssize]; + + if (initialCapacity > MAXIMUM_CAPACITY) + initialCapacity = MAXIMUM_CAPACITY; + int c = initialCapacity / ssize; + if (c * ssize < initialCapacity) + ++c; + int cap = 1; + while (cap < c) + cap <<= 1; + + for (int i = 0; i < this.segments.length; ++i) + this.segments[i] = new WeakHashMap(cap, loadFactor); + } + + /** + * Constructs a new, empty HashMap with the specified initial capacity and + * default load factor. + * + * @param initialCapacity + * the initial capacity of the HashMap. + */ + public ConcurrentWeakHashMap(int initialCapacity) { + this(initialCapacity, DEFAULT_LOAD_FACTOR, DEFAULT_CONCURRENCY_LEVEL); + } + + /** + * Constructs a new, empty HashMap with a default capacity and load factor. + */ + public ConcurrentWeakHashMap() { + this(32, DEFAULT_LOAD_FACTOR, DEFAULT_CONCURRENCY_LEVEL); + } + + @Override + public V put(K key, V value) { + int hash = hash(key); + WeakHashMap whm = segmentFor(hash); + synchronized (whm) { + return whm.put(key, value); + } + } + + /** + * Returns the value to which this HashMap maps the specified key. Returns + * null if the HashMap contains no mapping for this key. + * + * @param key + * key whose associated value is to be returned. + */ + @Override + public V get(Object key) { + int hash = hash(key); + WeakHashMap whm = segmentFor(hash); + //synchronized (whm) { + return whm.get(key); + //} + } + + @Override + public V remove(Object key) { + int hash = hash(key); + WeakHashMap whm = segmentFor(hash); + synchronized (whm) { + return whm.remove(key); + } + } + + @Override + public boolean remove(Object key, Object value) { + int hash = hash(key); + WeakHashMap whm = segmentFor(hash); + synchronized (whm) { + if (whm.containsKey(key) && (whm.get(key).equals(value))) { + whm.remove(key); + return true; + } + } + return false; + } + + /** + * This function is not accurate under concurrent environment. + * + */ + public int size() { + int sum = 0; + for (int i = 0; i < segments.length; i++) { + sum += segments[i].size(); + } + + return sum; + } + + public boolean containsKey(Object key) { + int hash = hash(key); + WeakHashMap whm = segmentFor(hash); + synchronized (whm) { + return whm.containsKey(key); + } + } + + /** + * Clear all mappings from all segment, leaving the org.apache.commons.beanutils2.ConcurrentWeakHashMap + * empty. + */ + public void clear() { + for (int i = 0; i < segments.length; i++) { + WeakHashMap whm = segments[i]; + synchronized (whm) { + whm.clear(); + } + } + } + + @SuppressWarnings("unchecked") + public Set keySet() { + if (keySet == null) { + keySet = new java.util.AbstractSet() { + public Iterator iterator() { + return new HashIterator(); + } + + public int size() { + return ConcurrentWeakHashMap.this.size(); + } + + public boolean contains(Object o) { + return containsKey(o); + } + + public boolean remove(Object o) { + return ConcurrentWeakHashMap.this.remove(o) != null; + } + + public void clear() { + ConcurrentWeakHashMap.this.clear(); + } + }; + } + return keySet; + } + + class HashIterator implements Iterator { + int currSegmentIndex = 0; + Iterator internal_iter = segments[0].keySet().iterator(); + + HashIterator() { + } + + final boolean advance() { + currSegmentIndex++; + if (currSegmentIndex < segments.length) { + internal_iter = segments[currSegmentIndex].keySet().iterator(); + return true; + } else + return false; + } + + public boolean hasNext() { + if (currSegmentIndex >= segments.length) + return false; + else { + boolean ret = internal_iter.hasNext(); + while (!ret && advance()) { + ret = internal_iter.hasNext(); + } + return ret; + } + } + + public K next() { + try { + return internal_iter.next(); + } catch (NoSuchElementException e) { + if (advance()) + return internal_iter.next(); + else + throw new NoSuchElementException(); + } + } + + public void remove() { + ConcurrentWeakHashMap.this.remove(next()); + } + } + + @Override + public V putIfAbsent(K key, V value) { + int hash = hash(key); + WeakHashMap whm = segmentFor(hash); + synchronized (whm) { + if (!whm.containsKey(key)) { + return whm.put(key, value); + } else + return whm.get(key); + } + } + + @Override + public boolean replace(K key, V oldValue, V newValue) { + int hash = hash(key); + WeakHashMap whm = segmentFor(hash); + synchronized (whm) { + if (whm.containsKey(key) && (whm.get(key).equals(oldValue))) { + whm.put(key, newValue); + return true; + } + } + return false; + } + + @Override + public V replace(K key, V value) { + int hash = hash(key); + WeakHashMap whm = segmentFor(hash); + synchronized (whm) { + if (whm.containsKey(key)) { + return whm.put(key, value); + } + } + return null; + } + + @Override + public Set> entrySet() { + // TODO: + return null; + } +} \ No newline at end of file diff --git a/src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java b/src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java index ca0977083..8cf6352b7 100644 --- a/src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java +++ b/src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java @@ -84,7 +84,7 @@ import org.apache.commons.beanutils2.converters.ZonedDateTimeConverter; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.apache.commons.logging.impl.WeakHashtable; + /** *

Utility methods for converting String scalar values to objects of the @@ -194,7 +194,7 @@ protected static ConvertUtilsBean getInstance() { * into objects of a specified Class, keyed by the destination Class. */ private final Map, Converter> converters = - new WeakHashtable(); + new ConcurrentWeakHashMap(); /** * The {@code Log} instance for this class. diff --git a/src/main/java/org/apache/commons/beanutils2/OriginalWeakFastHashMap.java b/src/main/java/org/apache/commons/beanutils2/OriginalWeakFastHashMap.java new file mode 100644 index 000000000..827ff41c0 --- /dev/null +++ b/src/main/java/org/apache/commons/beanutils2/OriginalWeakFastHashMap.java @@ -0,0 +1,746 @@ +/* + * 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.commons.beanutils2; + +import java.util.*; + +/** + *

A customized implementation of {@code java.util.HashMap} designed + * to operate in a multi-threaded environment where the large majority of + * method calls are read-only, instead of structural changes. When operating + * in "fast" mode, read calls are non-synchronized and write calls perform the + * following steps:

+ *
    + *
  • Clone the existing collection + *
  • Perform the modification on the clone + *
  • Replace the existing collection with the (modified) clone + *
+ *

When first created, objects of this class default to "slow" mode, where + * all accesses of any type are synchronized but no cloning takes place. This + * is appropriate for initially populating the collection, followed by a switch + * to "fast" mode (by calling {@code setFast(true)}) after initialization + * is complete.

+ * + *

NOTE: If you are creating and accessing a + * {@code HashMap} only within a single thread, you should use + * {@code java.util.HashMap} directly (with no synchronization), for + * maximum performance.

+ * + *

NOTE: This class is not cross-platform. + * Using it may cause unexpected failures on some architectures. + * It suffers from the same problems as the double-checked locking idiom. + * In particular, the instruction that clones the internal collection and the + * instruction that sets the internal reference to the clone can be executed + * or perceived out-of-order. This means that any read operation might fail + * unexpectedly, as it may be reading the state of the internal collection + * before the internal collection is fully formed. + * For more information on the double-checked locking idiom, see the + * + * Double-Checked Locking Idiom Is Broken Declaration.

+ * + * @since Commons Collections 1.0 + */ +public class OriginalWeakFastHashMap extends HashMap { + + private static final long serialVersionUID = 1L; + + /** + * The underlying map we are managing. + */ + private volatile Map map = null; + + /** + * Are we currently operating in "fast" mode? + */ + private boolean fast = false; + + // Constructors + + + /** + * Construct an empty map. + */ + public OriginalWeakFastHashMap() { + super(); + this.map = createMap(); + } + + /** + * Construct an empty map with the specified capacity. + * + * @param capacity the initial capacity of the empty map + */ + public OriginalWeakFastHashMap(final int capacity) { + super(); + this.map = createMap(capacity); + } + + /** + * Construct an empty map with the specified capacity and load factor. + * + * @param capacity the initial capacity of the empty map + * @param factor the load factor of the new map + */ + public OriginalWeakFastHashMap(final int capacity, final float factor) { + super(); + this.map = createMap(capacity, factor); + } + + /** + * Construct a new map with the same mappings as the specified map. + * + * @param map the map whose mappings are to be copied + */ + public OriginalWeakFastHashMap(final Map map) { + super(); + this.map = createMap(map); + } + + // Property access + + + /** + * Returns true if this map is operating in fast mode. + * + * @return true if this map is operating in fast mode + */ + public boolean getFast() { + return this.fast; + } + + /** + * Sets whether this map is operating in fast mode. + * + * @param fast true if this map should operate in fast mode + */ + public void setFast(final boolean fast) { + this.fast = fast; + } + + // Map access + + // These methods can forward straight to the wrapped Map in 'fast' mode. + // (because they are query methods) + + /** + * Return the value to which this map maps the specified key. Returns + * {@code null} if the map contains no mapping for this key, or if + * there is a mapping with a value of {@code null}. Use the + * {@code containsKey()} method to disambiguate these cases. + * + * @param key the key whose value is to be returned + * @return the value mapped to that key, or null + */ + @Override + public V get(final Object key) { + if (fast) { + return map.get(key); + } + synchronized (map) { + return map.get(key); + } + } + + /** + * Return the number of key-value mappings in this map. + * + * @return the current size of the map + */ + @Override + public int size() { + if (fast) { + return map.size(); + } + synchronized (map) { + return map.size(); + } + } + + /** + * Return {@code true} if this map contains no mappings. + * + * @return is the map currently empty + */ + @Override + public boolean isEmpty() { + if (fast) { + return map.isEmpty(); + } + synchronized (map) { + return map.isEmpty(); + } + } + + /** + * Return {@code true} if this map contains a mapping for the + * specified key. + * + * @param key the key to be searched for + * @return true if the map contains the key + */ + @Override + public boolean containsKey(final Object key) { + if (fast) { + return map.containsKey(key); + } + synchronized (map) { + return map.containsKey(key); + } + } + + /** + * Return {@code true} if this map contains one or more keys mapping + * to the specified value. + * + * @param value the value to be searched for + * @return true if the map contains the value + */ + @Override + public boolean containsValue(final Object value) { + if (fast) { + return map.containsValue(value); + } + synchronized (map) { + return map.containsValue(value); + } + } + + // Map modification + + // These methods perform special behavior in 'fast' mode. + // The map is cloned, updated and then assigned back. + // See the comments at the top as to why this won't always work. + + /** + * Associate the specified value with the specified key in this map. + * If the map previously contained a mapping for this key, the old + * value is replaced and returned. + * + * @param key the key with which the value is to be associated + * @param value the value to be associated with this key + * @return the value previously mapped to the key, or null + */ + @Override + public V put(final K key, final V value) { + if (fast) { + synchronized (this) { + final Map temp = cloneMap(map); + final V result = temp.put(key, value); + map = temp; + return result; + } + } + synchronized (map) { + return map.put(key, value); + } + } + + /** + * Copy all of the mappings from the specified map to this one, replacing + * any mappings with the same keys. + * + * @param in the map whose mappings are to be copied + */ + @Override + public void putAll(final Map in) { + if (fast) { + synchronized (this) { + final Map temp = cloneMap(map); + temp.putAll(in); + map = temp; + } + } else { + synchronized (map) { + map.putAll(in); + } + } + } + + /** + * Remove any mapping for this key, and return any previously + * mapped value. + * + * @param key the key whose mapping is to be removed + * @return the value removed, or null + */ + @Override + public V remove(final Object key) { + if (fast) { + synchronized (this) { + final Map temp = cloneMap(map); + final V result = temp.remove(key); + map = temp; + return result; + } + } + synchronized (map) { + return map.remove(key); + } + } + + /** + * Remove all mappings from this map. + */ + @Override + public void clear() { + if (fast) { + synchronized (this) { + map = createMap(); + } + } else { + synchronized (map) { + map.clear(); + } + } + } + + // Basic object methods + + + /** + * Compare the specified object with this list for equality. This + * implementation uses exactly the code that is used to define the + * list equals function in the documentation for the + * {@code Map.equals} method. + * + * @param o the object to be compared to this list + * @return true if the two maps are equal + */ + @Override + public boolean equals(final Object o) { + // Simple tests that require no synchronization + if (o == this) { + return true; + } else if (!(o instanceof Map)) { + return false; + } + final Map mo = (Map) o; + + // Compare the two maps for equality + if (fast) { + if (mo.size() != map.size()) { + return false; + } + for (final Entry e : map.entrySet()) { + final K key = e.getKey(); + final V value = e.getValue(); + if (value == null) { + if (!(mo.get(key) == null && mo.containsKey(key))) { + return false; + } + } else { + if (!value.equals(mo.get(key))) { + return false; + } + } + } + return true; + + } + synchronized (map) { + if (mo.size() != map.size()) { + return false; + } + for (final Entry e : map.entrySet()) { + final K key = e.getKey(); + final V value = e.getValue(); + if (value == null) { + if (!(mo.get(key) == null && mo.containsKey(key))) { + return false; + } + } else { + if (!value.equals(mo.get(key))) { + return false; + } + } + } + return true; + } + } + + /** + * Return the hash code value for this map. This implementation uses + * exactly the code that is used to define the list hash function in the + * documentation for the {@code Map.hashCode} method. + * + * @return suitable integer hash code + */ + @Override + public int hashCode() { + if (fast) { + int h = 0; + for (final Entry e : map.entrySet()) { + h += e.hashCode(); + } + return h; + } + synchronized (map) { + int h = 0; + for (final Entry e : map.entrySet()) { + h += e.hashCode(); + } + return h; + } + } + + /** + * Return a shallow copy of this {@code FastHashMap} instance. + * The keys and values themselves are not copied. + * + * @return a clone of this map + */ + @Override + public Object clone() { + OriginalWeakFastHashMap results = null; + if (fast) { + results = new OriginalWeakFastHashMap<>(map); + } else { + synchronized (map) { + results = new OriginalWeakFastHashMap<>(map); + } + } + results.setFast(getFast()); + return results; + } + + // Map views + + + /** + * Return a collection view of the mappings contained in this map. Each + * element in the returned collection is a {@code Map.Entry}. + * @return the set of map Map entries + */ + @Override + public Set> entrySet() { + return new EntrySet(); + } + + /** + * Return a set view of the keys contained in this map. + * @return the set of the Map's keys + */ + @Override + public Set keySet() { + return new KeySet(); + } + + /** + * Return a collection view of the values contained in this map. + * @return the set of the Map's values + */ + @Override + public Collection values() { + return new Values(); + } + + // Abstractions on Map creations (for subclasses such as WeakFastHashMap) + + + protected Map createMap() { + return new WeakHashMap<>(); + } + + protected Map createMap(final int capacity) { + return new WeakHashMap<>(capacity); + } + + protected Map createMap(final int capacity, final float factor) { + return new WeakHashMap<>(capacity, factor); + } + + protected Map createMap(final Map map) { + return new WeakHashMap<>(map); + } + + protected Map cloneMap(final Map map) { + return createMap(map); + } + + // Map view inner classes + + + /** + * Abstract collection implementation shared by keySet(), values() and entrySet(). + * + * @param the element type + */ + private abstract class CollectionView implements Collection { + + public CollectionView() { + } + + protected abstract Collection get(Map map); + protected abstract E iteratorNext(Entry entry); + + @Override + public void clear() { + if (fast) { + synchronized (OriginalWeakFastHashMap.this) { + map = createMap(); + } + } else { + synchronized (map) { + get(map).clear(); + } + } + } + + @Override + public boolean remove(final Object o) { + if (fast) { + synchronized (OriginalWeakFastHashMap.this) { + final Map temp = cloneMap(map); + final boolean r = get(temp).remove(o); + map = temp; + return r; + } + } + synchronized (map) { + return get(map).remove(o); + } + } + + @Override + public boolean removeAll(final Collection o) { + if (fast) { + synchronized (OriginalWeakFastHashMap.this) { + final Map temp = cloneMap(map); + final boolean r = get(temp).removeAll(o); + map = temp; + return r; + } + } + synchronized (map) { + return get(map).removeAll(o); + } + } + + @Override + public boolean retainAll(final Collection o) { + if (fast) { + synchronized (OriginalWeakFastHashMap.this) { + final Map temp = cloneMap(map); + final boolean r = get(temp).retainAll(o); + map = temp; + return r; + } + } + synchronized (map) { + return get(map).retainAll(o); + } + } + + @Override + public int size() { + if (fast) { + return get(map).size(); + } + synchronized (map) { + return get(map).size(); + } + } + + @Override + public boolean isEmpty() { + if (fast) { + return get(map).isEmpty(); + } + synchronized (map) { + return get(map).isEmpty(); + } + } + + @Override + public boolean contains(final Object o) { + if (fast) { + return get(map).contains(o); + } + synchronized (map) { + return get(map).contains(o); + } + } + + @Override + public boolean containsAll(final Collection o) { + if (fast) { + return get(map).containsAll(o); + } + synchronized (map) { + return get(map).containsAll(o); + } + } + + @Override + public T[] toArray(final T[] o) { + if (fast) { + return get(map).toArray(o); + } + synchronized (map) { + return get(map).toArray(o); + } + } + + @Override + public Object[] toArray() { + if (fast) { + return get(map).toArray(); + } + synchronized (map) { + return get(map).toArray(); + } + } + + @Override + public boolean equals(final Object o) { + if (o == this) { + return true; + } + if (fast) { + return get(map).equals(o); + } + synchronized (map) { + return get(map).equals(o); + } + } + + @Override + public int hashCode() { + if (fast) { + return get(map).hashCode(); + } + synchronized (map) { + return get(map).hashCode(); + } + } + + @Override + public boolean add(final E o) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean addAll(final Collection c) { + throw new UnsupportedOperationException(); + } + + @Override + public Iterator iterator() { + return new CollectionViewIterator(); + } + + private class CollectionViewIterator implements Iterator { + + private Map expected; + private Entry lastReturned = null; + private final Iterator> iterator; + + public CollectionViewIterator() { + this.expected = map; + this.iterator = expected.entrySet().iterator(); + } + + @Override + public boolean hasNext() { + if (expected != map) { + throw new ConcurrentModificationException(); + } + return iterator.hasNext(); + } + + @Override + public E next() { + if (expected != map) { + throw new ConcurrentModificationException(); + } + lastReturned = iterator.next(); + return iteratorNext(lastReturned); + } + + @Override + public void remove() { + if (lastReturned == null) { + throw new IllegalStateException(); + } + if (fast) { + synchronized (OriginalWeakFastHashMap.this) { + if (expected != map) { + throw new ConcurrentModificationException(); + } + OriginalWeakFastHashMap.this.remove(lastReturned.getKey()); + lastReturned = null; + expected = map; + } + } else { + iterator.remove(); + lastReturned = null; + } + } + } + } + + /** + * Set implementation over the keys of the FastHashMap + */ + private class KeySet extends CollectionView implements Set { + + @Override + protected Collection get(final Map map) { + return map.keySet(); + } + + @Override + protected K iteratorNext(final Entry entry) { + return entry.getKey(); + } + + } + + /** + * Collection implementation over the values of the FastHashMap + */ + private class Values extends CollectionView { + + @Override + protected Collection get(final Map map) { + return map.values(); + } + + @Override + protected V iteratorNext(final Entry entry) { + return entry.getValue(); + } + } + + /** + * Set implementation over the entries of the FastHashMap + */ + private class EntrySet extends CollectionView> implements Set> { + + @Override + protected Collection> get(final Map map) { + return map.entrySet(); + } + + @Override + protected Entry iteratorNext(final Entry entry) { + return entry; + } + + } + +} diff --git a/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java b/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java index 506ae2adf..aa171a33e 100644 --- a/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java +++ b/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java @@ -34,7 +34,7 @@ import org.apache.commons.beanutils2.expression.Resolver; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.apache.commons.logging.impl.WeakHashtable; + /** * Utility methods for using Java Reflection APIs to facilitate generic @@ -125,9 +125,9 @@ protected static PropertyUtilsBean getInstance() { /** Base constructor */ public PropertyUtilsBean() { - descriptorsCache = new WeakHashtable(); + descriptorsCache = new ConcurrentWeakHashMap(); // descriptorsCache.setFast(true); - mappedDescriptorsCache = new WeakHashtable(); + mappedDescriptorsCache = new ConcurrentWeakHashMap(); // mappedDescriptorsCache.setFast(true); introspectors = new CopyOnWriteArrayList<>(); resetBeanIntrospectors(); diff --git a/src/test/java/org/apache/commons/beanutils2/performance/WeakFastHashMapTest.java b/src/test/java/org/apache/commons/beanutils2/performance/WeakFastHashMapTest.java index 5f21c17e9..d26154fde 100644 --- a/src/test/java/org/apache/commons/beanutils2/performance/WeakFastHashMapTest.java +++ b/src/test/java/org/apache/commons/beanutils2/performance/WeakFastHashMapTest.java @@ -16,8 +16,9 @@ */ package org.apache.commons.beanutils2.performance; +import org.apache.commons.beanutils2.ConcurrentWeakHashMap; import org.apache.commons.beanutils2.WeakFastHashMap; -import org.apache.commons.logging.impl.WeakHashtable; + import java.util.Map; import java.util.Objects; @@ -29,7 +30,7 @@ public class WeakFastHashMapTest { static final TreeSet treeSet = new TreeSet<>(); static final Random random = new Random(); static final WeakFastHashMap weakFastHashMap = new WeakFastHashMap<>(); - static final Map weakHashtable = new WeakHashtable(); + static final Map weakHashtable = new ConcurrentWeakHashMap(); static final int INIT_SIZE = 50000; static final double WRITE_CHANCE = 0.01; From f9f877312d4b67f578b0835f2a6629b3590bd84d Mon Sep 17 00:00:00 2001 From: XenoAmess Date: Mon, 1 Jun 2020 04:00:11 +0800 Subject: [PATCH 5/8] fix --- src/main/java/org/apache/commons/beanutils2/BeanUtils.java | 4 ++-- .../java/org/apache/commons/beanutils2/ConvertUtilsBean.java | 1 - .../java/org/apache/commons/beanutils2/PropertyUtilsBean.java | 1 - 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/apache/commons/beanutils2/BeanUtils.java b/src/main/java/org/apache/commons/beanutils2/BeanUtils.java index c286292e1..173041a56 100644 --- a/src/main/java/org/apache/commons/beanutils2/BeanUtils.java +++ b/src/main/java/org/apache/commons/beanutils2/BeanUtils.java @@ -20,7 +20,7 @@ import java.lang.reflect.InvocationTargetException; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; + /** @@ -436,6 +436,6 @@ public static boolean initCause(final Throwable throwable, final Throwable cause * @since 1.8.0 */ public static Map createCache() { - return new ConcurrentHashMap<>(); + return new ConcurrentWeakHashMap<>(); } } diff --git a/src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java b/src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java index 8cf6352b7..ffc5a5b50 100644 --- a/src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java +++ b/src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java @@ -26,7 +26,6 @@ import java.nio.file.Path; import java.sql.Timestamp; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; import java.time.Duration; import java.time.LocalDate; import java.time.LocalDateTime; diff --git a/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java b/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java index aa171a33e..55d53da59 100644 --- a/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java +++ b/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java @@ -35,7 +35,6 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; - /** * Utility methods for using Java Reflection APIs to facilitate generic * property getter and setter operations on Java objects. Much of this From b26d0f4f6fd71a6fa340793819bfffbd7b53a0d4 Mon Sep 17 00:00:00 2001 From: XenoAmess Date: Mon, 1 Jun 2020 04:01:21 +0800 Subject: [PATCH 6/8] trytofix --- .../beanutils2/ConcurrentWeakHashMap.java | 117 +++++++++++++----- 1 file changed, 84 insertions(+), 33 deletions(-) diff --git a/src/main/java/org/apache/commons/beanutils2/ConcurrentWeakHashMap.java b/src/main/java/org/apache/commons/beanutils2/ConcurrentWeakHashMap.java index ed78a1b84..c1750ddc9 100644 --- a/src/main/java/org/apache/commons/beanutils2/ConcurrentWeakHashMap.java +++ b/src/main/java/org/apache/commons/beanutils2/ConcurrentWeakHashMap.java @@ -17,16 +17,13 @@ package org.apache.commons.beanutils2; -import java.util.AbstractMap; -import java.util.Iterator; -import java.util.NoSuchElementException; -import java.util.Set; -import java.util.WeakHashMap; +import java.util.*; import java.util.concurrent.ConcurrentMap; /** - * org.apache.commons.beanutils2.ConcurrentWeakHashMap from https://issues.apache.org/jira/browse/HARMONY-6434?jql=text%20~%20%22concurrent%20weak%20hash%20map%22 - * donated by James Gan + * org.apache.commons.beanutils2.ConcurrentWeakHashMap + * from https://issues.apache.org/jira/browse/HARMONY-6434 + * http://amino-cbbs.sourceforge.net/ */ public class ConcurrentWeakHashMap extends AbstractMap implements ConcurrentMap { @@ -84,8 +81,9 @@ public class ConcurrentWeakHashMap extends AbstractMap implements * segments. */ private static int hash(Object o) { - if (o == null) + if (o == null) { return 0; + } int hashCode = o.hashCode(); hashCode ^= (hashCode << 7); hashCode ^= (hashCode >>> 3); @@ -135,12 +133,14 @@ public ConcurrentWeakHashMap(int initialCapacity, float loadFactor, + loadFactor); } - if (concurrencyLevel <= 1) + if (concurrencyLevel <= 1) { throw new IllegalArgumentException("Illegal concurrencyLevel: " + concurrencyLevel); + } - if (concurrencyLevel > MAX_SEGMENTS) + if (concurrencyLevel > MAX_SEGMENTS) { concurrencyLevel = MAX_SEGMENTS; + } // Find power-of-two sizes best matching arguments int sshift = 0; @@ -154,17 +154,21 @@ public ConcurrentWeakHashMap(int initialCapacity, float loadFactor, this.segments = new WeakHashMap[ssize]; - if (initialCapacity > MAXIMUM_CAPACITY) + if (initialCapacity > MAXIMUM_CAPACITY) { initialCapacity = MAXIMUM_CAPACITY; + } int c = initialCapacity / ssize; - if (c * ssize < initialCapacity) + if (c * ssize < initialCapacity) { ++c; + } int cap = 1; - while (cap < c) + while (cap < c) { cap <<= 1; + } - for (int i = 0; i < this.segments.length; ++i) + for (int i = 0; i < this.segments.length; ++i) { this.segments[i] = new WeakHashMap(cap, loadFactor); + } } /** @@ -190,7 +194,12 @@ public V put(K key, V value) { int hash = hash(key); WeakHashMap whm = segmentFor(hash); synchronized (whm) { - return whm.put(key, value); + V res; + synchronized (segments) { + segments[(hash >>> segmentShift) & segmentMask] = new WeakHashMap<>(whm); + res = segments[(hash >>> segmentShift) & segmentMask].put(key, value); + } + return res; } } @@ -215,7 +224,12 @@ public V remove(Object key) { int hash = hash(key); WeakHashMap whm = segmentFor(hash); synchronized (whm) { - return whm.remove(key); + V res; + synchronized (segments) { + segments[(hash >>> segmentShift) & segmentMask] = new WeakHashMap<>(whm); + res = segments[(hash >>> segmentShift) & segmentMask].remove(key); + } + return res; } } @@ -225,7 +239,10 @@ public boolean remove(Object key, Object value) { WeakHashMap whm = segmentFor(hash); synchronized (whm) { if (whm.containsKey(key) && (whm.get(key).equals(value))) { - whm.remove(key); + synchronized (segments) { + segments[(hash >>> segmentShift) & segmentMask] = new WeakHashMap<>(whm); + segments[(hash >>> segmentShift) & segmentMask].remove(key); + } return true; } } @@ -236,15 +253,16 @@ public boolean remove(Object key, Object value) { * This function is not accurate under concurrent environment. * */ + @Override public int size() { int sum = 0; for (int i = 0; i < segments.length; i++) { sum += segments[i].size(); } - return sum; } + @Override public boolean containsKey(Object key) { int hash = hash(key); WeakHashMap whm = segmentFor(hash); @@ -257,35 +275,44 @@ public boolean containsKey(Object key) { * Clear all mappings from all segment, leaving the org.apache.commons.beanutils2.ConcurrentWeakHashMap * empty. */ + @Override public void clear() { - for (int i = 0; i < segments.length; i++) { - WeakHashMap whm = segments[i]; - synchronized (whm) { - whm.clear(); + synchronized (segments) { + for (int i = 0; i < segments.length; i++) { + WeakHashMap whm = segments[i]; + synchronized (whm) { + segments[i] = new WeakHashMap<>(); + } } } } + @Override @SuppressWarnings("unchecked") public Set keySet() { if (keySet == null) { keySet = new java.util.AbstractSet() { + @Override public Iterator iterator() { return new HashIterator(); } + @Override public int size() { return ConcurrentWeakHashMap.this.size(); } + @Override public boolean contains(Object o) { return containsKey(o); } + @Override public boolean remove(Object o) { return ConcurrentWeakHashMap.this.remove(o) != null; } + @Override public void clear() { ConcurrentWeakHashMap.this.clear(); } @@ -306,14 +333,16 @@ final boolean advance() { if (currSegmentIndex < segments.length) { internal_iter = segments[currSegmentIndex].keySet().iterator(); return true; - } else + } else { return false; + } } + @Override public boolean hasNext() { - if (currSegmentIndex >= segments.length) + if (currSegmentIndex >= segments.length) { return false; - else { + } else { boolean ret = internal_iter.hasNext(); while (!ret && advance()) { ret = internal_iter.hasNext(); @@ -322,17 +351,20 @@ public boolean hasNext() { } } + @Override public K next() { try { return internal_iter.next(); } catch (NoSuchElementException e) { - if (advance()) + if (advance()) { return internal_iter.next(); - else + } else { throw new NoSuchElementException(); + } } } + @Override public void remove() { ConcurrentWeakHashMap.this.remove(next()); } @@ -344,9 +376,15 @@ public V putIfAbsent(K key, V value) { WeakHashMap whm = segmentFor(hash); synchronized (whm) { if (!whm.containsKey(key)) { - return whm.put(key, value); - } else + V res; + synchronized (segments) { + segments[(hash >>> segmentShift) & segmentMask] = new WeakHashMap<>(whm); + res = segments[(hash >>> segmentShift) & segmentMask].put(key, value); + } + return res; + } else { return whm.get(key); + } } } @@ -356,7 +394,10 @@ public boolean replace(K key, V oldValue, V newValue) { WeakHashMap whm = segmentFor(hash); synchronized (whm) { if (whm.containsKey(key) && (whm.get(key).equals(oldValue))) { - whm.put(key, newValue); + synchronized (segments) { + segments[(hash >>> segmentShift) & segmentMask] = new WeakHashMap<>(whm); + segments[(hash >>> segmentShift) & segmentMask].put(key, newValue); + } return true; } } @@ -369,7 +410,12 @@ public V replace(K key, V value) { WeakHashMap whm = segmentFor(hash); synchronized (whm) { if (whm.containsKey(key)) { - return whm.put(key, value); + V res; + synchronized (segments) { + segments[(hash >>> segmentShift) & segmentMask] = new WeakHashMap<>(whm); + res = segments[(hash >>> segmentShift) & segmentMask].put(key, value); + } + return res; } } return null; @@ -377,7 +423,12 @@ public V replace(K key, V value) { @Override public Set> entrySet() { - // TODO: - return null; + Set> res = new HashSet<>(); + for (WeakHashMap whm : segments) { + synchronized (whm) { + res.addAll(whm.entrySet()); + } + } + return res; } } \ No newline at end of file From b1f3e057f8921b6f45c859204904ee90e574d827 Mon Sep 17 00:00:00 2001 From: XenoAmess Date: Mon, 1 Jun 2020 04:01:40 +0800 Subject: [PATCH 7/8] Revert "trytofix" This reverts commit b26d0f4f --- .../beanutils2/ConcurrentWeakHashMap.java | 117 +++++------------- 1 file changed, 33 insertions(+), 84 deletions(-) diff --git a/src/main/java/org/apache/commons/beanutils2/ConcurrentWeakHashMap.java b/src/main/java/org/apache/commons/beanutils2/ConcurrentWeakHashMap.java index c1750ddc9..ed78a1b84 100644 --- a/src/main/java/org/apache/commons/beanutils2/ConcurrentWeakHashMap.java +++ b/src/main/java/org/apache/commons/beanutils2/ConcurrentWeakHashMap.java @@ -17,13 +17,16 @@ package org.apache.commons.beanutils2; -import java.util.*; +import java.util.AbstractMap; +import java.util.Iterator; +import java.util.NoSuchElementException; +import java.util.Set; +import java.util.WeakHashMap; import java.util.concurrent.ConcurrentMap; /** - * org.apache.commons.beanutils2.ConcurrentWeakHashMap - * from https://issues.apache.org/jira/browse/HARMONY-6434 - * http://amino-cbbs.sourceforge.net/ + * org.apache.commons.beanutils2.ConcurrentWeakHashMap from https://issues.apache.org/jira/browse/HARMONY-6434?jql=text%20~%20%22concurrent%20weak%20hash%20map%22 + * donated by James Gan */ public class ConcurrentWeakHashMap extends AbstractMap implements ConcurrentMap { @@ -81,9 +84,8 @@ public class ConcurrentWeakHashMap extends AbstractMap implements * segments. */ private static int hash(Object o) { - if (o == null) { + if (o == null) return 0; - } int hashCode = o.hashCode(); hashCode ^= (hashCode << 7); hashCode ^= (hashCode >>> 3); @@ -133,14 +135,12 @@ public ConcurrentWeakHashMap(int initialCapacity, float loadFactor, + loadFactor); } - if (concurrencyLevel <= 1) { + if (concurrencyLevel <= 1) throw new IllegalArgumentException("Illegal concurrencyLevel: " + concurrencyLevel); - } - if (concurrencyLevel > MAX_SEGMENTS) { + if (concurrencyLevel > MAX_SEGMENTS) concurrencyLevel = MAX_SEGMENTS; - } // Find power-of-two sizes best matching arguments int sshift = 0; @@ -154,21 +154,17 @@ public ConcurrentWeakHashMap(int initialCapacity, float loadFactor, this.segments = new WeakHashMap[ssize]; - if (initialCapacity > MAXIMUM_CAPACITY) { + if (initialCapacity > MAXIMUM_CAPACITY) initialCapacity = MAXIMUM_CAPACITY; - } int c = initialCapacity / ssize; - if (c * ssize < initialCapacity) { + if (c * ssize < initialCapacity) ++c; - } int cap = 1; - while (cap < c) { + while (cap < c) cap <<= 1; - } - for (int i = 0; i < this.segments.length; ++i) { + for (int i = 0; i < this.segments.length; ++i) this.segments[i] = new WeakHashMap(cap, loadFactor); - } } /** @@ -194,12 +190,7 @@ public V put(K key, V value) { int hash = hash(key); WeakHashMap whm = segmentFor(hash); synchronized (whm) { - V res; - synchronized (segments) { - segments[(hash >>> segmentShift) & segmentMask] = new WeakHashMap<>(whm); - res = segments[(hash >>> segmentShift) & segmentMask].put(key, value); - } - return res; + return whm.put(key, value); } } @@ -224,12 +215,7 @@ public V remove(Object key) { int hash = hash(key); WeakHashMap whm = segmentFor(hash); synchronized (whm) { - V res; - synchronized (segments) { - segments[(hash >>> segmentShift) & segmentMask] = new WeakHashMap<>(whm); - res = segments[(hash >>> segmentShift) & segmentMask].remove(key); - } - return res; + return whm.remove(key); } } @@ -239,10 +225,7 @@ public boolean remove(Object key, Object value) { WeakHashMap whm = segmentFor(hash); synchronized (whm) { if (whm.containsKey(key) && (whm.get(key).equals(value))) { - synchronized (segments) { - segments[(hash >>> segmentShift) & segmentMask] = new WeakHashMap<>(whm); - segments[(hash >>> segmentShift) & segmentMask].remove(key); - } + whm.remove(key); return true; } } @@ -253,16 +236,15 @@ public boolean remove(Object key, Object value) { * This function is not accurate under concurrent environment. * */ - @Override public int size() { int sum = 0; for (int i = 0; i < segments.length; i++) { sum += segments[i].size(); } + return sum; } - @Override public boolean containsKey(Object key) { int hash = hash(key); WeakHashMap whm = segmentFor(hash); @@ -275,44 +257,35 @@ public boolean containsKey(Object key) { * Clear all mappings from all segment, leaving the org.apache.commons.beanutils2.ConcurrentWeakHashMap * empty. */ - @Override public void clear() { - synchronized (segments) { - for (int i = 0; i < segments.length; i++) { - WeakHashMap whm = segments[i]; - synchronized (whm) { - segments[i] = new WeakHashMap<>(); - } + for (int i = 0; i < segments.length; i++) { + WeakHashMap whm = segments[i]; + synchronized (whm) { + whm.clear(); } } } - @Override @SuppressWarnings("unchecked") public Set keySet() { if (keySet == null) { keySet = new java.util.AbstractSet() { - @Override public Iterator iterator() { return new HashIterator(); } - @Override public int size() { return ConcurrentWeakHashMap.this.size(); } - @Override public boolean contains(Object o) { return containsKey(o); } - @Override public boolean remove(Object o) { return ConcurrentWeakHashMap.this.remove(o) != null; } - @Override public void clear() { ConcurrentWeakHashMap.this.clear(); } @@ -333,16 +306,14 @@ final boolean advance() { if (currSegmentIndex < segments.length) { internal_iter = segments[currSegmentIndex].keySet().iterator(); return true; - } else { + } else return false; - } } - @Override public boolean hasNext() { - if (currSegmentIndex >= segments.length) { + if (currSegmentIndex >= segments.length) return false; - } else { + else { boolean ret = internal_iter.hasNext(); while (!ret && advance()) { ret = internal_iter.hasNext(); @@ -351,20 +322,17 @@ public boolean hasNext() { } } - @Override public K next() { try { return internal_iter.next(); } catch (NoSuchElementException e) { - if (advance()) { + if (advance()) return internal_iter.next(); - } else { + else throw new NoSuchElementException(); - } } } - @Override public void remove() { ConcurrentWeakHashMap.this.remove(next()); } @@ -376,15 +344,9 @@ public V putIfAbsent(K key, V value) { WeakHashMap whm = segmentFor(hash); synchronized (whm) { if (!whm.containsKey(key)) { - V res; - synchronized (segments) { - segments[(hash >>> segmentShift) & segmentMask] = new WeakHashMap<>(whm); - res = segments[(hash >>> segmentShift) & segmentMask].put(key, value); - } - return res; - } else { + return whm.put(key, value); + } else return whm.get(key); - } } } @@ -394,10 +356,7 @@ public boolean replace(K key, V oldValue, V newValue) { WeakHashMap whm = segmentFor(hash); synchronized (whm) { if (whm.containsKey(key) && (whm.get(key).equals(oldValue))) { - synchronized (segments) { - segments[(hash >>> segmentShift) & segmentMask] = new WeakHashMap<>(whm); - segments[(hash >>> segmentShift) & segmentMask].put(key, newValue); - } + whm.put(key, newValue); return true; } } @@ -410,12 +369,7 @@ public V replace(K key, V value) { WeakHashMap whm = segmentFor(hash); synchronized (whm) { if (whm.containsKey(key)) { - V res; - synchronized (segments) { - segments[(hash >>> segmentShift) & segmentMask] = new WeakHashMap<>(whm); - res = segments[(hash >>> segmentShift) & segmentMask].put(key, value); - } - return res; + return whm.put(key, value); } } return null; @@ -423,12 +377,7 @@ public V replace(K key, V value) { @Override public Set> entrySet() { - Set> res = new HashSet<>(); - for (WeakHashMap whm : segments) { - synchronized (whm) { - res.addAll(whm.entrySet()); - } - } - return res; + // TODO: + return null; } } \ No newline at end of file From 8b57699df682fe9f5e7e1f5f4d359e65d8c5b946 Mon Sep 17 00:00:00 2001 From: XenoAmess Date: Mon, 1 Jun 2020 04:13:25 +0800 Subject: [PATCH 8/8] change from to WeakFastHashMap to WeakHashtable --- .../apache/commons/beanutils2/BeanUtils.java | 4 +- .../beanutils2/ConcurrentWeakHashMap.java | 383 --------- .../commons/beanutils2/ConvertUtilsBean.java | 3 +- .../beanutils2/OriginalWeakFastHashMap.java | 746 ------------------ .../commons/beanutils2/PropertyUtilsBean.java | 5 +- .../performance/WeakFastHashMapTest.java | 4 +- 6 files changed, 10 insertions(+), 1135 deletions(-) delete mode 100644 src/main/java/org/apache/commons/beanutils2/ConcurrentWeakHashMap.java delete mode 100644 src/main/java/org/apache/commons/beanutils2/OriginalWeakFastHashMap.java diff --git a/src/main/java/org/apache/commons/beanutils2/BeanUtils.java b/src/main/java/org/apache/commons/beanutils2/BeanUtils.java index 173041a56..7f2e39e41 100644 --- a/src/main/java/org/apache/commons/beanutils2/BeanUtils.java +++ b/src/main/java/org/apache/commons/beanutils2/BeanUtils.java @@ -18,6 +18,8 @@ package org.apache.commons.beanutils2; +import org.apache.commons.logging.impl.WeakHashtable; + import java.lang.reflect.InvocationTargetException; import java.util.Map; @@ -436,6 +438,6 @@ public static boolean initCause(final Throwable throwable, final Throwable cause * @since 1.8.0 */ public static Map createCache() { - return new ConcurrentWeakHashMap<>(); + return new WeakHashtable(); } } diff --git a/src/main/java/org/apache/commons/beanutils2/ConcurrentWeakHashMap.java b/src/main/java/org/apache/commons/beanutils2/ConcurrentWeakHashMap.java deleted file mode 100644 index ed78a1b84..000000000 --- a/src/main/java/org/apache/commons/beanutils2/ConcurrentWeakHashMap.java +++ /dev/null @@ -1,383 +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.commons.beanutils2; - -import java.util.AbstractMap; -import java.util.Iterator; -import java.util.NoSuchElementException; -import java.util.Set; -import java.util.WeakHashMap; -import java.util.concurrent.ConcurrentMap; - -/** - * org.apache.commons.beanutils2.ConcurrentWeakHashMap from https://issues.apache.org/jira/browse/HARMONY-6434?jql=text%20~%20%22concurrent%20weak%20hash%20map%22 - * donated by James Gan - */ -public class ConcurrentWeakHashMap extends AbstractMap implements - ConcurrentMap { - /* ---------------- Constants -------------- */ - /** - * The default initial capacity for this table, used when not otherwise - * specified in a constructor. - */ - static final int DEFAULT_INITIAL_CAPACITY = 16; - - /** - * The default load factor for this table, used when not otherwise specified - * in a constructor. - */ - static final float DEFAULT_LOAD_FACTOR = 0.75f; - - /** - * The default concurrency level for this table, used when not otherwise - * specified in a constructor. - */ - static final int DEFAULT_CONCURRENCY_LEVEL = 16; - - /** - * The maximum capacity, used if a higher value is implicitly specified by - * either of the constructors with arguments. MUST be a power of two <= - * 1<<30 to ensure that entries are indexable using ints. - */ - static final int MAXIMUM_CAPACITY = 1 << 30; - - /** - * The maximum number of segments to allow; used to bound constructor - * arguments. - */ - static final int MAX_SEGMENTS = 1 << 16; // slightly conservative - - /* ---------------- Fields -------------- */ - - /** - * Mask value for indexing into segments. The upper bits of a key's hash - * code are used to choose the segment. - */ - final int segmentMask; - - /** - * Shift value for indexing within segments. - */ - final int segmentShift; - - final WeakHashMap[] segments; - - transient Set keySet; - - /** - * Generate hash code for input object in order to map it to different - * segments. - */ - private static int hash(Object o) { - if (o == null) - return 0; - int hashCode = o.hashCode(); - hashCode ^= (hashCode << 7); - hashCode ^= (hashCode >>> 3); - hashCode ^= (hashCode << 27); - hashCode ^= (hashCode >>> 15); - return hashCode; - } - - /** - * Returns the segment that should be used for key with given hash - * - * @param hash - * the hash code for the key - * @return the segment - */ - final private WeakHashMap segmentFor(int hash) { - return segments[(hash >>> segmentShift) & segmentMask]; - } - - /** - * Constructs a new, empty HashMap with the specified initial capacity and - * the specified load factor. - * - * @param initialCapacity - * the initial capacity of the HashMap. - * @param loadFactor - * a number between 0.0 and 1.0. - * @param concurrencyLevel - * the estimated number of concurrently updating threads. The - * implementation performs internal sizing to try to accommodate - * this many threads. - * @throws IllegalArgumentException - * if neither keys nor values use hard references, if the - * initial capacity is less than or equal to zero, or if the - * load factor is less than or equal to zero - */ - @SuppressWarnings("unchecked") - public ConcurrentWeakHashMap(int initialCapacity, float loadFactor, - int concurrencyLevel) { - if (initialCapacity < 0) { - throw new IllegalArgumentException("Illegal Initial Capacity: " - + initialCapacity); - } - - if ((loadFactor >= 1) || (loadFactor <= 0)) { - throw new IllegalArgumentException("Illegal Load factor: " - + loadFactor); - } - - if (concurrencyLevel <= 1) - throw new IllegalArgumentException("Illegal concurrencyLevel: " - + concurrencyLevel); - - if (concurrencyLevel > MAX_SEGMENTS) - concurrencyLevel = MAX_SEGMENTS; - - // Find power-of-two sizes best matching arguments - int sshift = 0; - int ssize = 1; - while (ssize < concurrencyLevel) { - ++sshift; - ssize <<= 1; - } - segmentShift = 32 - sshift; - segmentMask = ssize - 1; - - this.segments = new WeakHashMap[ssize]; - - if (initialCapacity > MAXIMUM_CAPACITY) - initialCapacity = MAXIMUM_CAPACITY; - int c = initialCapacity / ssize; - if (c * ssize < initialCapacity) - ++c; - int cap = 1; - while (cap < c) - cap <<= 1; - - for (int i = 0; i < this.segments.length; ++i) - this.segments[i] = new WeakHashMap(cap, loadFactor); - } - - /** - * Constructs a new, empty HashMap with the specified initial capacity and - * default load factor. - * - * @param initialCapacity - * the initial capacity of the HashMap. - */ - public ConcurrentWeakHashMap(int initialCapacity) { - this(initialCapacity, DEFAULT_LOAD_FACTOR, DEFAULT_CONCURRENCY_LEVEL); - } - - /** - * Constructs a new, empty HashMap with a default capacity and load factor. - */ - public ConcurrentWeakHashMap() { - this(32, DEFAULT_LOAD_FACTOR, DEFAULT_CONCURRENCY_LEVEL); - } - - @Override - public V put(K key, V value) { - int hash = hash(key); - WeakHashMap whm = segmentFor(hash); - synchronized (whm) { - return whm.put(key, value); - } - } - - /** - * Returns the value to which this HashMap maps the specified key. Returns - * null if the HashMap contains no mapping for this key. - * - * @param key - * key whose associated value is to be returned. - */ - @Override - public V get(Object key) { - int hash = hash(key); - WeakHashMap whm = segmentFor(hash); - //synchronized (whm) { - return whm.get(key); - //} - } - - @Override - public V remove(Object key) { - int hash = hash(key); - WeakHashMap whm = segmentFor(hash); - synchronized (whm) { - return whm.remove(key); - } - } - - @Override - public boolean remove(Object key, Object value) { - int hash = hash(key); - WeakHashMap whm = segmentFor(hash); - synchronized (whm) { - if (whm.containsKey(key) && (whm.get(key).equals(value))) { - whm.remove(key); - return true; - } - } - return false; - } - - /** - * This function is not accurate under concurrent environment. - * - */ - public int size() { - int sum = 0; - for (int i = 0; i < segments.length; i++) { - sum += segments[i].size(); - } - - return sum; - } - - public boolean containsKey(Object key) { - int hash = hash(key); - WeakHashMap whm = segmentFor(hash); - synchronized (whm) { - return whm.containsKey(key); - } - } - - /** - * Clear all mappings from all segment, leaving the org.apache.commons.beanutils2.ConcurrentWeakHashMap - * empty. - */ - public void clear() { - for (int i = 0; i < segments.length; i++) { - WeakHashMap whm = segments[i]; - synchronized (whm) { - whm.clear(); - } - } - } - - @SuppressWarnings("unchecked") - public Set keySet() { - if (keySet == null) { - keySet = new java.util.AbstractSet() { - public Iterator iterator() { - return new HashIterator(); - } - - public int size() { - return ConcurrentWeakHashMap.this.size(); - } - - public boolean contains(Object o) { - return containsKey(o); - } - - public boolean remove(Object o) { - return ConcurrentWeakHashMap.this.remove(o) != null; - } - - public void clear() { - ConcurrentWeakHashMap.this.clear(); - } - }; - } - return keySet; - } - - class HashIterator implements Iterator { - int currSegmentIndex = 0; - Iterator internal_iter = segments[0].keySet().iterator(); - - HashIterator() { - } - - final boolean advance() { - currSegmentIndex++; - if (currSegmentIndex < segments.length) { - internal_iter = segments[currSegmentIndex].keySet().iterator(); - return true; - } else - return false; - } - - public boolean hasNext() { - if (currSegmentIndex >= segments.length) - return false; - else { - boolean ret = internal_iter.hasNext(); - while (!ret && advance()) { - ret = internal_iter.hasNext(); - } - return ret; - } - } - - public K next() { - try { - return internal_iter.next(); - } catch (NoSuchElementException e) { - if (advance()) - return internal_iter.next(); - else - throw new NoSuchElementException(); - } - } - - public void remove() { - ConcurrentWeakHashMap.this.remove(next()); - } - } - - @Override - public V putIfAbsent(K key, V value) { - int hash = hash(key); - WeakHashMap whm = segmentFor(hash); - synchronized (whm) { - if (!whm.containsKey(key)) { - return whm.put(key, value); - } else - return whm.get(key); - } - } - - @Override - public boolean replace(K key, V oldValue, V newValue) { - int hash = hash(key); - WeakHashMap whm = segmentFor(hash); - synchronized (whm) { - if (whm.containsKey(key) && (whm.get(key).equals(oldValue))) { - whm.put(key, newValue); - return true; - } - } - return false; - } - - @Override - public V replace(K key, V value) { - int hash = hash(key); - WeakHashMap whm = segmentFor(hash); - synchronized (whm) { - if (whm.containsKey(key)) { - return whm.put(key, value); - } - } - return null; - } - - @Override - public Set> entrySet() { - // TODO: - return null; - } -} \ No newline at end of file diff --git a/src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java b/src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java index ffc5a5b50..b0891f901 100644 --- a/src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java +++ b/src/main/java/org/apache/commons/beanutils2/ConvertUtilsBean.java @@ -83,6 +83,7 @@ import org.apache.commons.beanutils2.converters.ZonedDateTimeConverter; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.commons.logging.impl.WeakHashtable; /** @@ -193,7 +194,7 @@ protected static ConvertUtilsBean getInstance() { * into objects of a specified Class, keyed by the destination Class. */ private final Map, Converter> converters = - new ConcurrentWeakHashMap(); + new WeakHashtable(); /** * The {@code Log} instance for this class. diff --git a/src/main/java/org/apache/commons/beanutils2/OriginalWeakFastHashMap.java b/src/main/java/org/apache/commons/beanutils2/OriginalWeakFastHashMap.java deleted file mode 100644 index 827ff41c0..000000000 --- a/src/main/java/org/apache/commons/beanutils2/OriginalWeakFastHashMap.java +++ /dev/null @@ -1,746 +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.commons.beanutils2; - -import java.util.*; - -/** - *

A customized implementation of {@code java.util.HashMap} designed - * to operate in a multi-threaded environment where the large majority of - * method calls are read-only, instead of structural changes. When operating - * in "fast" mode, read calls are non-synchronized and write calls perform the - * following steps:

- *
    - *
  • Clone the existing collection - *
  • Perform the modification on the clone - *
  • Replace the existing collection with the (modified) clone - *
- *

When first created, objects of this class default to "slow" mode, where - * all accesses of any type are synchronized but no cloning takes place. This - * is appropriate for initially populating the collection, followed by a switch - * to "fast" mode (by calling {@code setFast(true)}) after initialization - * is complete.

- * - *

NOTE: If you are creating and accessing a - * {@code HashMap} only within a single thread, you should use - * {@code java.util.HashMap} directly (with no synchronization), for - * maximum performance.

- * - *

NOTE: This class is not cross-platform. - * Using it may cause unexpected failures on some architectures. - * It suffers from the same problems as the double-checked locking idiom. - * In particular, the instruction that clones the internal collection and the - * instruction that sets the internal reference to the clone can be executed - * or perceived out-of-order. This means that any read operation might fail - * unexpectedly, as it may be reading the state of the internal collection - * before the internal collection is fully formed. - * For more information on the double-checked locking idiom, see the - * - * Double-Checked Locking Idiom Is Broken Declaration.

- * - * @since Commons Collections 1.0 - */ -public class OriginalWeakFastHashMap extends HashMap { - - private static final long serialVersionUID = 1L; - - /** - * The underlying map we are managing. - */ - private volatile Map map = null; - - /** - * Are we currently operating in "fast" mode? - */ - private boolean fast = false; - - // Constructors - - - /** - * Construct an empty map. - */ - public OriginalWeakFastHashMap() { - super(); - this.map = createMap(); - } - - /** - * Construct an empty map with the specified capacity. - * - * @param capacity the initial capacity of the empty map - */ - public OriginalWeakFastHashMap(final int capacity) { - super(); - this.map = createMap(capacity); - } - - /** - * Construct an empty map with the specified capacity and load factor. - * - * @param capacity the initial capacity of the empty map - * @param factor the load factor of the new map - */ - public OriginalWeakFastHashMap(final int capacity, final float factor) { - super(); - this.map = createMap(capacity, factor); - } - - /** - * Construct a new map with the same mappings as the specified map. - * - * @param map the map whose mappings are to be copied - */ - public OriginalWeakFastHashMap(final Map map) { - super(); - this.map = createMap(map); - } - - // Property access - - - /** - * Returns true if this map is operating in fast mode. - * - * @return true if this map is operating in fast mode - */ - public boolean getFast() { - return this.fast; - } - - /** - * Sets whether this map is operating in fast mode. - * - * @param fast true if this map should operate in fast mode - */ - public void setFast(final boolean fast) { - this.fast = fast; - } - - // Map access - - // These methods can forward straight to the wrapped Map in 'fast' mode. - // (because they are query methods) - - /** - * Return the value to which this map maps the specified key. Returns - * {@code null} if the map contains no mapping for this key, or if - * there is a mapping with a value of {@code null}. Use the - * {@code containsKey()} method to disambiguate these cases. - * - * @param key the key whose value is to be returned - * @return the value mapped to that key, or null - */ - @Override - public V get(final Object key) { - if (fast) { - return map.get(key); - } - synchronized (map) { - return map.get(key); - } - } - - /** - * Return the number of key-value mappings in this map. - * - * @return the current size of the map - */ - @Override - public int size() { - if (fast) { - return map.size(); - } - synchronized (map) { - return map.size(); - } - } - - /** - * Return {@code true} if this map contains no mappings. - * - * @return is the map currently empty - */ - @Override - public boolean isEmpty() { - if (fast) { - return map.isEmpty(); - } - synchronized (map) { - return map.isEmpty(); - } - } - - /** - * Return {@code true} if this map contains a mapping for the - * specified key. - * - * @param key the key to be searched for - * @return true if the map contains the key - */ - @Override - public boolean containsKey(final Object key) { - if (fast) { - return map.containsKey(key); - } - synchronized (map) { - return map.containsKey(key); - } - } - - /** - * Return {@code true} if this map contains one or more keys mapping - * to the specified value. - * - * @param value the value to be searched for - * @return true if the map contains the value - */ - @Override - public boolean containsValue(final Object value) { - if (fast) { - return map.containsValue(value); - } - synchronized (map) { - return map.containsValue(value); - } - } - - // Map modification - - // These methods perform special behavior in 'fast' mode. - // The map is cloned, updated and then assigned back. - // See the comments at the top as to why this won't always work. - - /** - * Associate the specified value with the specified key in this map. - * If the map previously contained a mapping for this key, the old - * value is replaced and returned. - * - * @param key the key with which the value is to be associated - * @param value the value to be associated with this key - * @return the value previously mapped to the key, or null - */ - @Override - public V put(final K key, final V value) { - if (fast) { - synchronized (this) { - final Map temp = cloneMap(map); - final V result = temp.put(key, value); - map = temp; - return result; - } - } - synchronized (map) { - return map.put(key, value); - } - } - - /** - * Copy all of the mappings from the specified map to this one, replacing - * any mappings with the same keys. - * - * @param in the map whose mappings are to be copied - */ - @Override - public void putAll(final Map in) { - if (fast) { - synchronized (this) { - final Map temp = cloneMap(map); - temp.putAll(in); - map = temp; - } - } else { - synchronized (map) { - map.putAll(in); - } - } - } - - /** - * Remove any mapping for this key, and return any previously - * mapped value. - * - * @param key the key whose mapping is to be removed - * @return the value removed, or null - */ - @Override - public V remove(final Object key) { - if (fast) { - synchronized (this) { - final Map temp = cloneMap(map); - final V result = temp.remove(key); - map = temp; - return result; - } - } - synchronized (map) { - return map.remove(key); - } - } - - /** - * Remove all mappings from this map. - */ - @Override - public void clear() { - if (fast) { - synchronized (this) { - map = createMap(); - } - } else { - synchronized (map) { - map.clear(); - } - } - } - - // Basic object methods - - - /** - * Compare the specified object with this list for equality. This - * implementation uses exactly the code that is used to define the - * list equals function in the documentation for the - * {@code Map.equals} method. - * - * @param o the object to be compared to this list - * @return true if the two maps are equal - */ - @Override - public boolean equals(final Object o) { - // Simple tests that require no synchronization - if (o == this) { - return true; - } else if (!(o instanceof Map)) { - return false; - } - final Map mo = (Map) o; - - // Compare the two maps for equality - if (fast) { - if (mo.size() != map.size()) { - return false; - } - for (final Entry e : map.entrySet()) { - final K key = e.getKey(); - final V value = e.getValue(); - if (value == null) { - if (!(mo.get(key) == null && mo.containsKey(key))) { - return false; - } - } else { - if (!value.equals(mo.get(key))) { - return false; - } - } - } - return true; - - } - synchronized (map) { - if (mo.size() != map.size()) { - return false; - } - for (final Entry e : map.entrySet()) { - final K key = e.getKey(); - final V value = e.getValue(); - if (value == null) { - if (!(mo.get(key) == null && mo.containsKey(key))) { - return false; - } - } else { - if (!value.equals(mo.get(key))) { - return false; - } - } - } - return true; - } - } - - /** - * Return the hash code value for this map. This implementation uses - * exactly the code that is used to define the list hash function in the - * documentation for the {@code Map.hashCode} method. - * - * @return suitable integer hash code - */ - @Override - public int hashCode() { - if (fast) { - int h = 0; - for (final Entry e : map.entrySet()) { - h += e.hashCode(); - } - return h; - } - synchronized (map) { - int h = 0; - for (final Entry e : map.entrySet()) { - h += e.hashCode(); - } - return h; - } - } - - /** - * Return a shallow copy of this {@code FastHashMap} instance. - * The keys and values themselves are not copied. - * - * @return a clone of this map - */ - @Override - public Object clone() { - OriginalWeakFastHashMap results = null; - if (fast) { - results = new OriginalWeakFastHashMap<>(map); - } else { - synchronized (map) { - results = new OriginalWeakFastHashMap<>(map); - } - } - results.setFast(getFast()); - return results; - } - - // Map views - - - /** - * Return a collection view of the mappings contained in this map. Each - * element in the returned collection is a {@code Map.Entry}. - * @return the set of map Map entries - */ - @Override - public Set> entrySet() { - return new EntrySet(); - } - - /** - * Return a set view of the keys contained in this map. - * @return the set of the Map's keys - */ - @Override - public Set keySet() { - return new KeySet(); - } - - /** - * Return a collection view of the values contained in this map. - * @return the set of the Map's values - */ - @Override - public Collection values() { - return new Values(); - } - - // Abstractions on Map creations (for subclasses such as WeakFastHashMap) - - - protected Map createMap() { - return new WeakHashMap<>(); - } - - protected Map createMap(final int capacity) { - return new WeakHashMap<>(capacity); - } - - protected Map createMap(final int capacity, final float factor) { - return new WeakHashMap<>(capacity, factor); - } - - protected Map createMap(final Map map) { - return new WeakHashMap<>(map); - } - - protected Map cloneMap(final Map map) { - return createMap(map); - } - - // Map view inner classes - - - /** - * Abstract collection implementation shared by keySet(), values() and entrySet(). - * - * @param the element type - */ - private abstract class CollectionView implements Collection { - - public CollectionView() { - } - - protected abstract Collection get(Map map); - protected abstract E iteratorNext(Entry entry); - - @Override - public void clear() { - if (fast) { - synchronized (OriginalWeakFastHashMap.this) { - map = createMap(); - } - } else { - synchronized (map) { - get(map).clear(); - } - } - } - - @Override - public boolean remove(final Object o) { - if (fast) { - synchronized (OriginalWeakFastHashMap.this) { - final Map temp = cloneMap(map); - final boolean r = get(temp).remove(o); - map = temp; - return r; - } - } - synchronized (map) { - return get(map).remove(o); - } - } - - @Override - public boolean removeAll(final Collection o) { - if (fast) { - synchronized (OriginalWeakFastHashMap.this) { - final Map temp = cloneMap(map); - final boolean r = get(temp).removeAll(o); - map = temp; - return r; - } - } - synchronized (map) { - return get(map).removeAll(o); - } - } - - @Override - public boolean retainAll(final Collection o) { - if (fast) { - synchronized (OriginalWeakFastHashMap.this) { - final Map temp = cloneMap(map); - final boolean r = get(temp).retainAll(o); - map = temp; - return r; - } - } - synchronized (map) { - return get(map).retainAll(o); - } - } - - @Override - public int size() { - if (fast) { - return get(map).size(); - } - synchronized (map) { - return get(map).size(); - } - } - - @Override - public boolean isEmpty() { - if (fast) { - return get(map).isEmpty(); - } - synchronized (map) { - return get(map).isEmpty(); - } - } - - @Override - public boolean contains(final Object o) { - if (fast) { - return get(map).contains(o); - } - synchronized (map) { - return get(map).contains(o); - } - } - - @Override - public boolean containsAll(final Collection o) { - if (fast) { - return get(map).containsAll(o); - } - synchronized (map) { - return get(map).containsAll(o); - } - } - - @Override - public T[] toArray(final T[] o) { - if (fast) { - return get(map).toArray(o); - } - synchronized (map) { - return get(map).toArray(o); - } - } - - @Override - public Object[] toArray() { - if (fast) { - return get(map).toArray(); - } - synchronized (map) { - return get(map).toArray(); - } - } - - @Override - public boolean equals(final Object o) { - if (o == this) { - return true; - } - if (fast) { - return get(map).equals(o); - } - synchronized (map) { - return get(map).equals(o); - } - } - - @Override - public int hashCode() { - if (fast) { - return get(map).hashCode(); - } - synchronized (map) { - return get(map).hashCode(); - } - } - - @Override - public boolean add(final E o) { - throw new UnsupportedOperationException(); - } - - @Override - public boolean addAll(final Collection c) { - throw new UnsupportedOperationException(); - } - - @Override - public Iterator iterator() { - return new CollectionViewIterator(); - } - - private class CollectionViewIterator implements Iterator { - - private Map expected; - private Entry lastReturned = null; - private final Iterator> iterator; - - public CollectionViewIterator() { - this.expected = map; - this.iterator = expected.entrySet().iterator(); - } - - @Override - public boolean hasNext() { - if (expected != map) { - throw new ConcurrentModificationException(); - } - return iterator.hasNext(); - } - - @Override - public E next() { - if (expected != map) { - throw new ConcurrentModificationException(); - } - lastReturned = iterator.next(); - return iteratorNext(lastReturned); - } - - @Override - public void remove() { - if (lastReturned == null) { - throw new IllegalStateException(); - } - if (fast) { - synchronized (OriginalWeakFastHashMap.this) { - if (expected != map) { - throw new ConcurrentModificationException(); - } - OriginalWeakFastHashMap.this.remove(lastReturned.getKey()); - lastReturned = null; - expected = map; - } - } else { - iterator.remove(); - lastReturned = null; - } - } - } - } - - /** - * Set implementation over the keys of the FastHashMap - */ - private class KeySet extends CollectionView implements Set { - - @Override - protected Collection get(final Map map) { - return map.keySet(); - } - - @Override - protected K iteratorNext(final Entry entry) { - return entry.getKey(); - } - - } - - /** - * Collection implementation over the values of the FastHashMap - */ - private class Values extends CollectionView { - - @Override - protected Collection get(final Map map) { - return map.values(); - } - - @Override - protected V iteratorNext(final Entry entry) { - return entry.getValue(); - } - } - - /** - * Set implementation over the entries of the FastHashMap - */ - private class EntrySet extends CollectionView> implements Set> { - - @Override - protected Collection> get(final Map map) { - return map.entrySet(); - } - - @Override - protected Entry iteratorNext(final Entry entry) { - return entry; - } - - } - -} diff --git a/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java b/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java index 55d53da59..506ae2adf 100644 --- a/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java +++ b/src/main/java/org/apache/commons/beanutils2/PropertyUtilsBean.java @@ -34,6 +34,7 @@ import org.apache.commons.beanutils2.expression.Resolver; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.commons.logging.impl.WeakHashtable; /** * Utility methods for using Java Reflection APIs to facilitate generic @@ -124,9 +125,9 @@ protected static PropertyUtilsBean getInstance() { /** Base constructor */ public PropertyUtilsBean() { - descriptorsCache = new ConcurrentWeakHashMap(); + descriptorsCache = new WeakHashtable(); // descriptorsCache.setFast(true); - mappedDescriptorsCache = new ConcurrentWeakHashMap(); + mappedDescriptorsCache = new WeakHashtable(); // mappedDescriptorsCache.setFast(true); introspectors = new CopyOnWriteArrayList<>(); resetBeanIntrospectors(); diff --git a/src/test/java/org/apache/commons/beanutils2/performance/WeakFastHashMapTest.java b/src/test/java/org/apache/commons/beanutils2/performance/WeakFastHashMapTest.java index d26154fde..0f65d2beb 100644 --- a/src/test/java/org/apache/commons/beanutils2/performance/WeakFastHashMapTest.java +++ b/src/test/java/org/apache/commons/beanutils2/performance/WeakFastHashMapTest.java @@ -16,8 +16,8 @@ */ package org.apache.commons.beanutils2.performance; -import org.apache.commons.beanutils2.ConcurrentWeakHashMap; import org.apache.commons.beanutils2.WeakFastHashMap; +import org.apache.commons.logging.impl.WeakHashtable; import java.util.Map; @@ -30,7 +30,7 @@ public class WeakFastHashMapTest { static final TreeSet treeSet = new TreeSet<>(); static final Random random = new Random(); static final WeakFastHashMap weakFastHashMap = new WeakFastHashMap<>(); - static final Map weakHashtable = new ConcurrentWeakHashMap(); + static final Map weakHashtable = new WeakHashtable(); static final int INIT_SIZE = 50000; static final double WRITE_CHANCE = 0.01;