Skip to content
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

Merged
merged 41 commits into from
Jul 1, 2022
Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
dd7a597
use ConcurrentLinkedHashMap as backing for LRUMap
pjfanning Jun 26, 2022
cd4e033
move new package
pjfanning Jun 26, 2022
47e9ae3
remove new jar dependency (jsr 305) - use apache cayenne version of CLHM
pjfanning Jun 26, 2022
8befa70
Create ConcurrentLinkedHashMapTest.java
pjfanning Jun 27, 2022
8f1bdcd
Create LRUMapTest.java
pjfanning Jun 27, 2022
0ae71ae
move serialization test so that internal state can be checked
pjfanning Jun 27, 2022
7d901fb
Update TestJDKSerialization.java
pjfanning Jun 27, 2022
111f3b3
Update LRUMapTest.java
pjfanning Jun 27, 2022
bfc5302
extra tests - some fail and will be investigated in coming days
pjfanning Jun 27, 2022
3882f17
prevent add being used on the map entrySet
pjfanning Jun 27, 2022
b2c9b08
Create ConcurrentLinkedHashMapStressTest.java
pjfanning Jun 28, 2022
0c01d2b
Update ConcurrentLinkedHashMapStressTest.java
pjfanning Jun 28, 2022
674a407
wip
pjfanning Jun 28, 2022
53c3205
try to make test reliable
pjfanning Jun 28, 2022
f246b9c
Merge pull request #2 from pjfanning/stress2
pjfanning Jun 28, 2022
bac364a
Update ConcurrentLinkedHashMapStressTest.java
pjfanning Jun 28, 2022
517f432
Update ConcurrentLinkedHashMapStressTest.java
pjfanning Jun 28, 2022
c57d7e1
Update ConcurrentLinkedHashMapStressTest.java
pjfanning Jun 28, 2022
6936cd9
rename package
pjfanning Jun 28, 2022
669a562
Create package-info.java
pjfanning Jun 28, 2022
f145f0a
remove some unused weigher implementations
pjfanning Jun 28, 2022
946549a
stop using apache cayenne version and use latest Ben Mane's code agai…
pjfanning Jun 28, 2022
47a1bb1
Update ConcurrentLinkedHashMapStressTest.java
pjfanning Jun 28, 2022
8463041
Update ConcurrentLinkedHashMapStressTest.java
pjfanning Jun 28, 2022
64bd51f
see if updating clhm and map as an atomic operation helps to make tes…
pjfanning Jun 28, 2022
f916019
Update ConcurrentLinkedHashMapStressTest.java
pjfanning Jun 28, 2022
1696ac0
remove unnecessary concurrency level
pjfanning Jun 29, 2022
8d29de8
remove more code for concurrency level
pjfanning Jun 29, 2022
a5581f7
make this CLHM non-serializable
pjfanning Jun 30, 2022
8e6897d
remove EvictionListener
pjfanning Jun 30, 2022
6a5a5b2
Revert "remove EvictionListener"
pjfanning Jun 30, 2022
4a26b39
Revert "make this CLHM non-serializable"
pjfanning Jun 30, 2022
2e132ca
Revert "remove more code for concurrency level"
pjfanning Jun 30, 2022
dfcf976
Revert "remove unnecessary concurrency level"
pjfanning Jun 30, 2022
fc6fa05
Update package-info.java
pjfanning Jun 30, 2022
6820db2
make some more classes package private
pjfanning Jun 30, 2022
ff39d5f
rename internal map
pjfanning Jun 30, 2022
b438091
remove more weigher logic
pjfanning Jun 30, 2022
f85c808
Update LRUMap.java
pjfanning Jun 30, 2022
3bc8acd
remove Weigher interface
pjfanning Jul 1, 2022
a0e8839
remove EntryWeigher interface
pjfanning Jul 1, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@
<version>${version.powermock}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava-testlib</artifactId>
<version>31.1-jre</version>
<scope>test</scope>
</dependency>
<!-- For testing TestNoClassDefFoundDeserializer -->
<dependency>
<groupId>javax.measure</groupId>
Expand Down
70 changes: 18 additions & 52 deletions src/main/java/com/fasterxml/jackson/databind/util/LRUMap.java
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.internal.PrivateMaxEntriesMap;

/**
* Helper for simple bounded maps used for reusing lookup values.
Expand All @@ -10,44 +9,36 @@
* on assumption that all use cases are for caching where persistence
* does not make sense. The only thing serialized is the cache size of Map.
*<p>
* NOTE: since version 2.4.2, this is <b>NOT</b> an LRU-based at all; reason
* being that it is not possible to use JDK components that do LRU _AND_ perform
* well wrt synchronization on multi-core systems. So we choose efficient synchronization
* over potentially more efficient handling of entries.
* NOTE: since Jackson 2.14, the implementation evicts the least recently used
* entry when max size is reached.
*<p>
* And yes, there are efficient LRU implementations such as
* <a href="https://code.google.com/p/concurrentlinkedhashmap/">concurrentlinkedhashmap</a>;
* but at this point we really try to keep external deps to minimum.
* Plan from Jackson 2.12 is to focus more on pluggability as {@link LookupCache} and
* let users, frameworks, provide their own cache implementations.
* Since Jackson 2.12, there has been pluggable {@link LookupCache} interface which
* allows users, frameworks, provide their own cache implementations.
*/
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 PrivateMaxEntriesMap<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 PrivateMaxEntriesMap.Builder<K, V>()
.initialCapacity(initialEntries)
.maximumCapacity(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);
}

Expand All @@ -56,21 +47,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(); }
Expand All @@ -84,23 +66,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);
Copy link
Member

Choose a reason for hiding this comment

The 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.
But perhaps CLHM does not expose these settings after construction?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

long capacity() and setCapacity(long) expose those if helpful. CLHM is serializable where it retains the entries (to mirror ConcurrentHashMap) and discards the eviction order. That differs in Guava/Caffeine, fwiw, which don't keep the entries and just the configuration.

Copy link
Member Author

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

The 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.

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright 2012 Google Inc. All Rights Reserved.
*
* Licensed 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 com.fasterxml.jackson.databind.util.internal;

/**
* A class that can determine the weight of an entry. The total weight threshold
* is used to determine when an eviction is required.
*
* @author [email protected] (Ben Manes)
* @see <a href="http://code.google.com/p/concurrentlinkedhashmap/">
* http://code.google.com/p/concurrentlinkedhashmap/</a>
*/
interface EntryWeigher<K, V> {

/**
* Measures an entry's weight to determine how many units of capacity that
* the key and value consumes. An entry must consume a minimum of one unit.
*
* @param key the key to weigh
* @param value the value to weigh
* @return the entry's weight
*/
int weightOf(K key, V value);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* Copyright 2010 Google Inc. All Rights Reserved.
*
* Licensed 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 com.fasterxml.jackson.databind.util.internal;

/**
* A listener registered for notification when an entry is evicted. An instance
* may be called concurrently by multiple threads to process entries. An
* implementation should avoid performing blocking calls or synchronizing on
* shared resources.
* <p>
* The listener is invoked by {@link PrivateMaxEntriesMap} on a caller's
* thread and will not block other threads from operating on the map. An
* implementation should be aware that the caller's thread will not expect
* long execution times or failures as a side effect of the listener being
* notified. Execution safety and a fast turn around time can be achieved by
* performing the operation asynchronously, such as by submitting a task to an
* {@link java.util.concurrent.ExecutorService}.
*
* @author [email protected] (Ben Manes)
* @see <a href="http://code.google.com/p/concurrentlinkedhashmap/">
* http://code.google.com/p/concurrentlinkedhashmap/</a>
*/
interface EvictionListener<K, V> {

/**
* A call-back notification that the entry was evicted.
*
* @param key the entry's key
* @param value the entry's value
*/
void onEviction(K key, V value);
}
Loading