You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by js...@apache.org on 2020/09/25 14:22:39 UTC
[sling-org-apache-sling-resourceresolver] branch
feature/SLING-9769-minor-memory-leak-in-MapEntries created (now 1eac8dd)
This is an automated email from the ASF dual-hosted git repository.
jsedding pushed a change to branch feature/SLING-9769-minor-memory-leak-in-MapEntries
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git.
at 1eac8dd SLING-9769 - Minor memory leak in MapEntries class
This branch includes the following new commits:
new 1eac8dd SLING-9769 - Minor memory leak in MapEntries class
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[sling-org-apache-sling-resourceresolver] 01/01: SLING-9769 - Minor
memory leak in MapEntries class
Posted by js...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jsedding pushed a commit to branch feature/SLING-9769-minor-memory-leak-in-MapEntries
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git
commit 1eac8dda6aa984e76b18cc1d42b0f2f1b0229d53
Author: Julian Sedding <js...@apache.org>
AuthorDate: Fri Sep 25 16:21:41 2020 +0200
SLING-9769 - Minor memory leak in MapEntries class
---
.../resourceresolver/impl/mapping/MapEntries.java | 75 ++++++++--------------
1 file changed, 26 insertions(+), 49 deletions(-)
diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
index ee6cf34..debc99d 100644
--- a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
+++ b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
@@ -48,6 +48,8 @@ import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.locks.ReentrantLock;
+import java.util.function.Supplier;
+import java.util.stream.Stream;
import javax.servlet.http.HttpServletResponse;
@@ -140,7 +142,7 @@ public class MapEntries implements
private Timer timer;
- private boolean updateBloomFilterFile = false;
+ private final AtomicBoolean updateBloomFilterFile = new AtomicBoolean(false);
private final StringInterpolationProvider stringInterpolationProvider;
@@ -235,6 +237,7 @@ public class MapEntries implements
log.debug("creating bloom filter file {}",
vanityBloomFilterFile.getAbsolutePath());
vanityBloomFilter = createVanityBloomFilter();
+ updateBloomFilterFile.set(true);
persistBloomFilter();
createVanityBloomFilter = true;
} else {
@@ -247,8 +250,8 @@ public class MapEntries implements
}
// task for persisting the bloom filter every minute (if changes exist)
- timer = new Timer();
- timer.schedule(new BloomFilterTask(), 60_000);
+ timer = new Timer("VanityPathBloomFilterUpdater", true);
+ timer.schedule(new BloomFilterTask(), 60_000, 60_000);
this.vanityTargets = loadVanityPaths(createVanityBloomFilter);
}
@@ -447,7 +450,7 @@ public class MapEntries implements
needsUpdate = loadVanityPath(resource, resolveMapsMap, vanityTargets, false, true);
}
if ( needsUpdate ) {
- updateBloomFilterFile = true;
+ updateBloomFilterFile.set(true);
return true;
}
return false;
@@ -534,11 +537,10 @@ public class MapEntries implements
* Cleans up this class.
*/
public void dispose() {
- try {
- persistBloomFilter();
- } catch (IOException e) {
- log.error("Error while saving bloom filter to disk", e);
- }
+
+ timer.cancel();
+
+ persistBloomFilter();
if (this.registration != null) {
this.registration.unregister();
@@ -620,7 +622,7 @@ public class MapEntries implements
public Map<String, String> getAliasMap(final String parentPath) {
return aliasMap.get(parentPath);
}
-
+
@Override
public Map<String, List<String>> getVanityPathMappings() {
return Collections.unmodifiableMap(vanityTargets);
@@ -768,13 +770,14 @@ public class MapEntries implements
return bloomFilter;
}
- private void persistBloomFilter() throws IOException {
- if (vanityBloomFilterFile != null && vanityBloomFilter != null) {
- FileOutputStream out = new FileOutputStream(vanityBloomFilterFile);
- try {
+ private void persistBloomFilter() {
+ if (vanityBloomFilterFile != null && vanityBloomFilter != null
+ && updateBloomFilterFile.getAndSet(false)) {
+ try (FileOutputStream out = new FileOutputStream(vanityBloomFilterFile)) {
out.write(this.vanityBloomFilter);
- } finally {
- out.close();
+ } catch (IOException e) {
+ throw new RuntimeException(
+ "Error while saving bloom filter to disk", e);
}
}
}
@@ -1123,30 +1126,15 @@ public class MapEntries implements
final String queryString = "SELECT sling:vanityPath, sling:redirect, sling:redirectStatus FROM nt:base WHERE sling:vanityPath IS NOT NULL";
final Iterator<Resource> i = resolver.findResources(queryString, "sql");
- while (i.hasNext() && (createVanityBloomFilter || isAllVanityPathEntriesCached() || vanityCounter.longValue() < this.factory.getMaxCachedVanityPathEntries())) {
+ Supplier<Boolean> isCacheComplete = () -> isAllVanityPathEntriesCached() || vanityCounter.longValue() < this.factory.getMaxCachedVanityPathEntries();
+ while (i.hasNext() && (createVanityBloomFilter || isCacheComplete.get())) {
final Resource resource = i.next();
- boolean isValid = false;
- for(final Path sPath : this.factory.getObservationPaths()) {
- if ( sPath.matches(resource.getPath())) {
- isValid = true;
- break;
- }
+ final String resourcePath = resource.getPath();
+ if (Stream.of(this.factory.getObservationPaths()).anyMatch(path -> path.matches(resourcePath))) {
+ loadVanityPath(resource, resolveMapsMap, targetPaths, isCacheComplete.get(), createVanityBloomFilter);
}
- if ( isValid ) {
- if (isAllVanityPathEntriesCached() || vanityCounter.longValue() < this.factory.getMaxCachedVanityPathEntries()) {
- // fill up the cache and the bloom filter
- loadVanityPath(resource, resolveMapsMap, targetPaths, true,
- createVanityBloomFilter);
- } else {
- // fill up the bloom filter
- loadVanityPath(resource, resolveMapsMap, targetPaths, false,
- createVanityBloomFilter);
- }
- }
-
}
-
return targetPaths;
}
@@ -1160,10 +1148,7 @@ public class MapEntries implements
}
final ValueMap props = resource.getValueMap();
- long vanityOrder = 0;
- if (props.containsKey(PROP_VANITY_ORDER)) {
- vanityOrder = props.get(PROP_VANITY_ORDER, Long.class);
- }
+ long vanityOrder = props.get(PROP_VANITY_ORDER, 0L);
// url is ignoring scheme and host.port and the path is
// what is stored in the sling:vanityPath property
@@ -1534,15 +1519,7 @@ public class MapEntries implements
final class BloomFilterTask extends TimerTask {
@Override
public void run() {
- try {
- if (updateBloomFilterFile) {
- persistBloomFilter();
- updateBloomFilterFile = false;
- }
- } catch (IOException e) {
- throw new RuntimeException(
- "Error while saving bloom filter to disk", e);
- }
+ persistBloomFilter();
}
}