-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
use ConcurrentLinkedHashMap as backing for LRUMap #3531
Changes from 26 commits
dd7a597
cd4e033
47e9ae3
8befa70
8f1bdcd
0ae71ae
7d901fb
111f3b3
bfc5302
3882f17
b2c9b08
0c01d2b
674a407
53c3205
f246b9c
bac364a
517f432
c57d7e1
6936cd9
669a562
f145f0a
946549a
47a1bb1
8463041
64bd51f
f916019
1696ac0
8d29de8
a5581f7
8e6897d
6a5a5b2
4a26b39
2e132ca
dfcf976
fc6fa05
6820db2
ff39d5f
b438091
f85c808
3bc8acd
a0e8839
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,6 @@ | ||
package com.fasterxml.jackson.databind.util; | ||
|
||
import java.io.*; | ||
import java.util.concurrent.ConcurrentHashMap; | ||
import com.fasterxml.jackson.databind.util.clhm.ConcurrentLinkedHashMap; | ||
|
||
/** | ||
* Helper for simple bounded maps used for reusing lookup values. | ||
|
@@ -25,29 +24,26 @@ public class LRUMap<K,V> | |
implements LookupCache<K,V>, // since 2.12 | ||
java.io.Serializable | ||
{ | ||
private static final long serialVersionUID = 1L; | ||
private static final long serialVersionUID = 2L; | ||
|
||
protected final transient int _maxEntries; | ||
protected final int _initialEntries; | ||
protected final int _maxEntries; | ||
protected final transient ConcurrentLinkedHashMap<K,V> _map; | ||
|
||
protected final transient ConcurrentHashMap<K,V> _map; | ||
|
||
public LRUMap(int initialEntries, int maxEntries) | ||
{ | ||
// We'll use concurrency level of 4, seems reasonable | ||
_map = new ConcurrentHashMap<K,V>(initialEntries, 0.8f, 4); | ||
_initialEntries = initialEntries; | ||
_maxEntries = maxEntries; | ||
// We'll use concurrency level of 4, seems reasonable | ||
pjfanning marked this conversation as resolved.
Show resolved
Hide resolved
|
||
_map = new ConcurrentLinkedHashMap.Builder<K, V>() | ||
.initialCapacity(initialEntries) | ||
.maximumWeightedCapacity(maxEntries) | ||
.concurrencyLevel(4) | ||
pjfanning marked this conversation as resolved.
Show resolved
Hide resolved
|
||
.build(); | ||
} | ||
|
||
@Override | ||
public V put(K key, V value) { | ||
if (_map.size() >= _maxEntries) { | ||
// double-locking, yes, but safe here; trying to avoid "clear storms" | ||
synchronized (this) { | ||
if (_map.size() >= _maxEntries) { | ||
clear(); | ||
} | ||
} | ||
} | ||
return _map.put(key, value); | ||
} | ||
|
||
|
@@ -56,21 +52,12 @@ public V put(K key, V value) { | |
*/ | ||
@Override | ||
public V putIfAbsent(K key, V value) { | ||
// not 100% optimal semantically, but better from correctness (never exceeds | ||
// defined maximum) and close enough all in all: | ||
if (_map.size() >= _maxEntries) { | ||
synchronized (this) { | ||
if (_map.size() >= _maxEntries) { | ||
clear(); | ||
} | ||
} | ||
} | ||
return _map.putIfAbsent(key, value); | ||
} | ||
|
||
// NOTE: key is of type Object only to retain binary backwards-compatibility | ||
@Override | ||
public V get(Object key) { return _map.get(key); } | ||
public V get(Object key) { return _map.get(key); } | ||
|
||
@Override | ||
public void clear() { _map.clear(); } | ||
|
@@ -84,23 +71,7 @@ public V putIfAbsent(K key, V value) { | |
/********************************************************** | ||
*/ | ||
|
||
/** | ||
* Ugly hack, to work through the requirement that _value is indeed final, | ||
* and that JDK serialization won't call ctor(s) if Serializable is implemented. | ||
* | ||
* @since 2.1 | ||
*/ | ||
protected transient int _jdkSerializeMaxEntries; | ||
|
||
private void readObject(ObjectInputStream in) throws IOException { | ||
_jdkSerializeMaxEntries = in.readInt(); | ||
} | ||
|
||
private void writeObject(ObjectOutputStream out) throws IOException { | ||
out.writeInt(_jdkSerializeMaxEntries); | ||
} | ||
|
||
protected Object readResolve() { | ||
return new LRUMap<Object,Object>(_jdkSerializeMaxEntries, _jdkSerializeMaxEntries); | ||
return new LRUMap<K,V>(_initialEntries, _maxEntries); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Possibly not a big deal, but the idea here was that JDK serialization would retain size settings, if possible. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I hit issues with the old _jdkSerializeMaxEntries values - after my initial changes, the cloned LRUMap was being constructed with _jdkSerializeMaxEntries set to 0 and this meant that CLHM immediately evicted all new entries. @cowtowncoder would it be ok to allow the backing CLHM be non-transient, then we wouldn't need any special serialization logic? As @ben-manes highlights, the eviction order would be lost but that isn't too bad. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think cached content serialization should not be attempted, since contents are not necessarily serializable -- and at least for serializers, deserializers, are not. Alternatively if we really want to retain contents in some cases, could make it configurable. But at this point I think I'd prefer just wiping out contents. I think losing some of the settings is probably fine: ability to JDK serialize Jackson components is bit of an odd feature in general. Although, to be honest, I think the main important thing there is to retain configuration: as far as I know, this is relied upon by frameworks like Spark etc where workers are being sent serialized set up of handlers. |
||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps update the
LRUMap
javadoc?