You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jmeter.apache.org by vl...@apache.org on 2023/05/11 12:35:15 UTC
[jmeter] branch master updated: Use Caffeine for caching HTTP headers instead of commons-collections4 LRUMap (#5911)
This is an automated email from the ASF dual-hosted git repository.
vladimirsitnikov pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/jmeter.git
The following commit(s) were added to refs/heads/master by this push:
new b2c1f2c6d8 Use Caffeine for caching HTTP headers instead of commons-collections4 LRUMap (#5911)
b2c1f2c6d8 is described below
commit b2c1f2c6d891d68fa6ed759aac05911f98993083
Author: Vladimir Sitnikov <si...@gmail.com>
AuthorDate: Thu May 11 15:35:08 2023 +0300
Use Caffeine for caching HTTP headers instead of commons-collections4 LRUMap (#5911)
It removes compile-time dependency on commons-collections4 from protocols:http
---
src/protocol/http/build.gradle.kts | 4 +-
.../jmeter/protocol/http/control/CacheManager.java | 43 ++++++++++++----------
.../http/control/TestCacheManagerBase.java | 17 +++++----
.../control/TestCacheManagerThreadIteration.java | 9 +++--
.../jmeter/protocol/http/parser/TestCssParser.java | 9 +++--
xdocs/changes.xml | 1 +
6 files changed, 48 insertions(+), 35 deletions(-)
diff --git a/src/protocol/http/build.gradle.kts b/src/protocol/http/build.gradle.kts
index 1d68ed064b..49d0c0e635 100644
--- a/src/protocol/http/build.gradle.kts
+++ b/src/protocol/http/build.gradle.kts
@@ -58,7 +58,9 @@ dependencies {
}
implementation("org.jsoup:jsoup")
implementation("oro:oro")
- implementation("org.apache.commons:commons-collections4")
+ runtimeOnly("org.apache.commons:commons-collections4") {
+ because("commons-collections4 was a dependency in previous JMeter versions, so we keep it for compatibility")
+ }
implementation("commons-net:commons-net")
implementation("com.helger.commons:ph-commons") {
// We don't really need to use/distribute jsr305
diff --git a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/CacheManager.java b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/CacheManager.java
index 4d9ee398e7..c86f6fbe11 100644
--- a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/CacheManager.java
+++ b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/CacheManager.java
@@ -23,7 +23,6 @@ import java.net.URL;
import java.net.URLConnection;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
@@ -31,7 +30,6 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
-import org.apache.commons.collections4.map.LRUMap;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.tuple.ImmutablePair;
import org.apache.commons.lang3.tuple.Pair;
@@ -54,6 +52,9 @@ import org.apache.jmeter.util.JMeterUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+
/**
* Handles HTTP Caching.
*/
@@ -81,7 +82,7 @@ public class CacheManager extends ConfigTestElement implements TestStateListener
public static final String MAX_SIZE = "maxSize"; // $NON-NLS-1$
//-
- private transient InheritableThreadLocal<Map<String, CacheEntry>> threadCache;
+ private transient InheritableThreadLocal<Cache<String, CacheEntry>> threadCache;
private transient boolean useExpires; // Cached value
@@ -89,7 +90,7 @@ public class CacheManager extends ConfigTestElement implements TestStateListener
* used to share the cache between 2 cache managers
* @see CacheManager#createCacheManagerProxy()
* @since 3.0 */
- private transient Map<String, CacheEntry> localCache;
+ private transient Cache<String, CacheEntry> localCache;
public CacheManager() {
setProperty(new BooleanProperty(CLEAR, false));
@@ -98,7 +99,7 @@ public class CacheManager extends ConfigTestElement implements TestStateListener
useExpires = false;
}
- CacheManager(Map<String, CacheEntry> localCache, boolean useExpires) {
+ CacheManager(Cache<String, CacheEntry> localCache, boolean useExpires) {
this.localCache = localCache;
this.useExpires = useExpires;
}
@@ -282,13 +283,16 @@ public class CacheManager extends ConfigTestElement implements TestStateListener
getCache().put(url, new CacheEntry(lastModified, expiresDate, etag, varyHeader.getLeft()));
getCache().put(varyUrl(url, varyHeader.getLeft(), varyHeader.getRight()), new CacheEntry(lastModified, expiresDate, etag, null));
} else {
- if (getCache().get(url) != null) {
- log.debug("Entry for {} already in cache.", url);
- return;
- }
- CacheEntry cacheEntry = new CacheEntry(lastModified, expiresDate, etag, null);
- log.debug("Set entry {} into cache for url {}", url, cacheEntry);
- getCache().put(url, cacheEntry);
+ // Makes expiresDate effectively-final
+ Date entryExpiresDate = expiresDate;
+ getCache().get(
+ url,
+ key -> {
+ CacheEntry cacheEntry = new CacheEntry(lastModified, entryExpiresDate, etag, null);
+ log.debug("Set entry {} into cache for url {}", url, cacheEntry);
+ return cacheEntry;
+ }
+ );
}
}
@@ -536,7 +540,7 @@ public class CacheManager extends ConfigTestElement implements TestStateListener
}
private CacheEntry getEntry(String url, Header[] headers) {
- CacheEntry entry = getCache().get(url);
+ CacheEntry entry = getCache().getIfPresent(url);
log.debug("getEntry url:{} entry:{} header:{}", url, entry, headers);
if (entry == null) {
log.debug("No entry found for url {}", url);
@@ -566,7 +570,7 @@ public class CacheManager extends ConfigTestElement implements TestStateListener
return "vary-" + headerName + "-" + headerValue + "-" + url;
}
- private Map<String, CacheEntry> getCache() {
+ private Cache<String, CacheEntry> getCache() {
return localCache != null ? localCache : threadCache.get();
}
@@ -609,12 +613,13 @@ public class CacheManager extends ConfigTestElement implements TestStateListener
private void clearCache() {
log.debug("Clear cache");
- threadCache = new InheritableThreadLocal<Map<String, CacheEntry>>(){
+ // TODO: avoid re-creating the thread local every time, reset its contents instead
+ threadCache = new InheritableThreadLocal<Cache<String, CacheEntry>>(){
@Override
- protected Map<String, CacheEntry> initialValue(){
- // Bug 51942 - this map may be used from multiple threads
- Map<String, CacheEntry> map = new LRUMap<>(getMaxSize());
- return Collections.synchronizedMap(map);
+ protected Cache<String, CacheEntry> initialValue() {
+ return Caffeine.newBuilder()
+ .maximumSize(getMaxSize())
+ .build();
}
};
}
diff --git a/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerBase.java b/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerBase.java
index e776c2842f..ba38114f8c 100644
--- a/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerBase.java
+++ b/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerBase.java
@@ -29,7 +29,6 @@ import java.time.Instant;
import java.time.ZoneId;
import java.time.format.DateTimeFormatter;
import java.util.Locale;
-import java.util.Map;
import org.apache.jmeter.junit.JMeterTestCase;
import org.apache.jmeter.protocol.http.control.CacheManager.CacheEntry;
@@ -39,6 +38,8 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.parallel.Execution;
import org.junit.jupiter.api.parallel.ExecutionMode;
+import com.github.benmanes.caffeine.cache.Cache;
+
@Execution(ExecutionMode.CONCURRENT)
public abstract class TestCacheManagerBase extends JMeterTestCase {
protected static final String LOCAL_HOST = "http://localhost/";
@@ -386,7 +387,7 @@ public abstract class TestCacheManagerBase extends JMeterTestCase {
@Test
public void testSaveDetailsWithEmptySampleResultGivesNoCacheEntry() throws Exception {
cacheResultWithGivenCode("");
- assertTrue(getThreadCache().isEmpty(), "Saving details with empty SampleResult should not make cache entry.");
+ assertTrue(getThreadCache().asMap().isEmpty(), "Saving details with empty SampleResult should not make cache entry.");
}
@Test
@@ -423,13 +424,13 @@ public abstract class TestCacheManagerBase extends JMeterTestCase {
@Test
public void testClearCache() throws Exception {
- assertTrue(getThreadCache().isEmpty(), "ThreadCache should be empty initially.");
+ assertTrue(getThreadCache().asMap().isEmpty(), "ThreadCache should be empty initially.");
cacheResultWithGivenCode("200");
assertFalse(
- getThreadCache().isEmpty(),
+ getThreadCache().asMap().isEmpty(),
"ThreadCache should be populated after saving details for HttpMethod with SampleResult with response code 200.");
this.cacheManager.clear();
- assertTrue(getThreadCache().isEmpty(), "ThreadCache should be emptied by call to clear.");
+ assertTrue(getThreadCache().asMap().isEmpty(), "ThreadCache should be emptied by call to clear.");
}
protected HTTPSampleResult getSampleResultWithSpecifiedResponseCode(String code) {
@@ -440,17 +441,17 @@ public abstract class TestCacheManagerBase extends JMeterTestCase {
return sampleResult;
}
- private Map<String, CacheManager.CacheEntry> getThreadCache() throws Exception {
+ private Cache<String, CacheManager.CacheEntry> getThreadCache() throws Exception {
Field threadLocalfield = CacheManager.class.getDeclaredField("threadCache");
threadLocalfield.setAccessible(true);
@SuppressWarnings("unchecked")
- ThreadLocal<Map<String, CacheManager.CacheEntry>> threadLocal = (ThreadLocal<Map<String, CacheManager.CacheEntry>>) threadLocalfield
+ ThreadLocal<Cache<String, CacheManager.CacheEntry>> threadLocal = (ThreadLocal<Cache<String, CacheManager.CacheEntry>>) threadLocalfield
.get(this.cacheManager);
return threadLocal.get();
}
protected CacheManager.CacheEntry getThreadCacheEntry(String url) throws Exception {
- return getThreadCache().get(url);
+ return getThreadCache().getIfPresent(url);
}
diff --git a/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerThreadIteration.java b/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerThreadIteration.java
index 68c260cc9f..f8ca9a1b70 100644
--- a/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerThreadIteration.java
+++ b/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerThreadIteration.java
@@ -31,7 +31,6 @@ import java.time.format.DateTimeFormatter;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
-import java.util.Map;
import org.apache.http.Header;
import org.apache.http.HttpEntity;
@@ -53,6 +52,8 @@ import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import com.github.benmanes.caffeine.cache.Cache;
+
/**
* Test {@link CacheManager} that uses HTTPHC4Impl
*/
@@ -292,17 +293,17 @@ public class TestCacheManagerThreadIteration {
this.cacheManager.setHeaders(this.url, this.httpMethod);
}
- private Map<String, CacheManager.CacheEntry> getThreadCache() throws Exception {
+ private Cache<String, CacheManager.CacheEntry> getThreadCache() throws Exception {
Field threadLocalfield = CacheManager.class.getDeclaredField("threadCache");
threadLocalfield.setAccessible(true);
@SuppressWarnings("unchecked")
- ThreadLocal<Map<String, CacheManager.CacheEntry>> threadLocal = (ThreadLocal<Map<String, CacheManager.CacheEntry>>) threadLocalfield
+ ThreadLocal<Cache<String, CacheManager.CacheEntry>> threadLocal = (ThreadLocal<Cache<String, CacheManager.CacheEntry>>) threadLocalfield
.get(this.cacheManager);
return threadLocal.get();
}
protected CacheManager.CacheEntry getThreadCacheEntry(String url) throws Exception {
- return getThreadCache().get(url);
+ return getThreadCache().getIfPresent(url);
}
@Test
public void testCacheControlCleared() throws Exception {
diff --git a/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/parser/TestCssParser.java b/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/parser/TestCssParser.java
index 5429c89894..0482d58261 100644
--- a/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/parser/TestCssParser.java
+++ b/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/parser/TestCssParser.java
@@ -25,9 +25,10 @@ import static org.hamcrest.collection.IsEmptyCollection.empty;
import java.net.MalformedURLException;
import java.net.URL;
import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Iterator;
import java.util.List;
-import org.apache.commons.collections4.IteratorUtils;
import org.apache.jmeter.junit.JMeterTestCase;
import org.hamcrest.CoreMatchers;
import org.junit.jupiter.api.Test;
@@ -74,10 +75,12 @@ public class TestCssParser extends JMeterTestCase {
private List<URL> extractUrls(CssParser parser, String css)
throws LinkExtractorParseException, MalformedURLException {
- List<URL> result = IteratorUtils.toList(parser.getEmbeddedResourceURLs(
+ List<URL> result = new ArrayList<>();
+ Iterator<URL> urlIterator = parser.getEmbeddedResourceURLs(
"Mozilla", css.getBytes(StandardCharsets.UTF_8), new URL(
"http://example.org/"), StandardCharsets.UTF_8
- .displayName()));
+ .displayName());
+ urlIterator.forEachRemaining(result::add);
return result;
}
}
diff --git a/xdocs/changes.xml b/xdocs/changes.xml
index 5819e64aad..05690513a1 100644
--- a/xdocs/changes.xml
+++ b/xdocs/changes.xml
@@ -74,6 +74,7 @@ Summary
<h3>HTTP Samplers and Test Script Recorder</h3>
<ul>
+ <li><pr>5911</pr> Use Caffeine for caching HTTP headers instead of commons-collections4 LRUMap</li>
</ul>
<h3>Other samplers</h3>