Skip to content

JCRVLT-837 adding cached namespace impl to prevent thrashing in the r… #411

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.

Sign up for GitHub

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

Open
pat-lego wants to merge 2 commits into
base: master
Choose a base branch
from
+436 −1
Open

JCRVLT-837 adding cached namespace impl to prevent thrashing in the r… #411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a4d380e
JCRVLT-837 adding cached namespace impl to prevent thrashing in the r…
Jan 26, 2026
da20675
JCRVLT-837 adding tests and bounded LRU cache
Jan 26, 2026
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
17 changes: 16 additions & 1 deletion vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/AggregateImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,8 @@ public void invalidate() {
nodeRef = null;
relPath = null;
state = STATE_INITIAL;
// Invalidate the cached namespace prefixes for this aggregate
mgr.invalidateAggregatePrefixes(path);
}

public Aggregate getParent() {
Expand Down Expand Up @@ -410,7 +412,7 @@ public String[] getNamespacePrefixes() {
}

public String getNamespaceURI(String prefix) throws RepositoryException {
return mgr.getNamespaceURI(prefix);
return mgr.getCachedNamespaceURI(prefix);
}

public Collection<Property> getBinaries() {
Expand Down Expand Up @@ -610,6 +612,8 @@ private void addNamespace(Set<String> prefixes, String name) throws RepositoryEx
String pfx = name.substring(0, idx);
if (!prefixes.contains(pfx)) {
prefixes.add(pfx);
// Cache the prefix in the manager to avoid repeated JCR lookups
mgr.cacheNamespacePrefix(pfx);
}
}
}
Expand All @@ -623,6 +627,14 @@ private void addNamespacePath(Set<String> prefixes, String path) throws Reposito

private void loadNamespaces() {
if (namespacePrefixes == null) {
// Check if this aggregate's namespaces are already cached (bounded LRU cache)
String[] cachedPrefixes = mgr.getCachedAggregatePrefixes(path);
if (cachedPrefixes != null) {
log.debug("Using cached namespace prefixes for '{}': {}", path, cachedPrefixes);
Copy link
Copy Markdown
Contributor

@reschke reschke Jan 26, 2026

Choose a reason for hiding this comment

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

AFAICT, this code is never reached when running the filevault test suite.

We need a test case that actually covers this case.

Copy link
Copy Markdown
Author

@pat-lego pat-lego Jan 26, 2026

Choose a reason for hiding this comment

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

added a test

Copy link
Copy Markdown
Contributor

@reschke reschke Jan 26, 2026

Choose a reason for hiding this comment

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

Unless I'm missing something, that code is never reached. Sonar agrees.

It would only help if the same path is scanned multiple times. If that can happen, we should be able to write a test.

namespacePrefixes = cachedPrefixes;
return;
}

if (log.isDebugEnabled()) {
log.trace("loading namespaces of aggregate {}", path);
}
Expand All @@ -635,6 +647,9 @@ private void loadNamespaces() {
loadNamespaces(prefixes, "", getNode());
namespacePrefixes = prefixes.toArray(new String[prefixes.size()]);

// Cache the discovered prefixes for this aggregate path (bounded LRU cache)
mgr.cacheAggregatePrefixes(path, namespacePrefixes);
Copy link
Copy Markdown
Contributor

@joerghoh joerghoh Jan 26, 2026

Choose a reason for hiding this comment

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

If I understand this code correctly, it stores this String-Array of namespacePrefixes in a global cache, per path. But that also means, that there is benefit only in the case that loadNamespaces() is called at least twice (either as part of the same aggregate or in a different one). Under what circumstances would that be helpful then?

Copy link
Copy Markdown
Author

@pat-lego pat-lego Jan 26, 2026

Choose a reason for hiding this comment

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

added bounded lru cache


// set if and only if in DEBUG level
if (start >= 0) {
Duration duration = Duration.ofNanos(System.nanoTime() - start);
Expand Down
190 changes: 190 additions & 0 deletions vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/AggregateManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,11 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;

import org.apache.jackrabbit.vault.fs.api.AggregateManager;
import org.apache.jackrabbit.vault.fs.api.Aggregator;
Expand Down Expand Up @@ -85,6 +88,11 @@ public class AggregateManagerImpl implements AggregateManager {
*/
private static final String DEFAULT_NODETYPES = "" + "org/apache/jackrabbit/vault/fs/config/nodetypes.cnd";

/**
* default logger
*/
private static final Logger log = LoggerFactory.getLogger(AggregateManagerImpl.class);

/**
* the repository session for this manager
*/
Expand Down Expand Up @@ -124,6 +132,25 @@ public class AggregateManagerImpl implements AggregateManager {
*/
private final Set<String> nodeTypes = new HashSet<String>();

/**
* Cache of namespace prefixes to URIs. This cache is shared across all aggregates
* to avoid expensive JCR tree traversals for namespace discovery.
*/
private final ConcurrentHashMap<String, String> namespacePrefixCache = new ConcurrentHashMap<>();

/**
* Default maximum size for the aggregate namespace prefix cache.
* This limits memory usage while still providing performance benefits for frequently accessed paths.
*/
private static final int DEFAULT_AGGREGATE_CACHE_SIZE = 1000;

/**
* Bounded LRU cache of namespace prefixes per aggregate path.
* This cache stores the discovered prefixes for frequently accessed aggregate paths
* to avoid re-walking the same subtrees, while limiting memory usage through LRU eviction.
*/
private final Map<String, String[]> aggregateNamespaceCache;

/**
* config
*/
Expand Down Expand Up @@ -295,12 +322,25 @@ private AggregateManagerImpl(
aggregatorProvider = new AggregatorProvider(config.getAggregators());
artifactHandlers = Collections.unmodifiableList(config.getHandlers());

// Initialize bounded LRU cache for aggregate namespace prefixes
int cacheSize = getAggregateCacheSizeFromConfig();
this.aggregateNamespaceCache =
Collections.synchronizedMap(new LinkedHashMap<String, String[]>(cacheSize + 1, 0.75f, true) {
@Override
protected boolean removeEldestEntry(Map.Entry<String, String[]> eldest) {
return size() > cacheSize;
}
});

// init root node
Aggregator rootAggregator = rootNode.getDepth() == 0 ? new RootAggregator() : getAggregator(rootNode, null);
root = new AggregateImpl(this, rootNode.getPath(), rootAggregator);

// setup node types
initNodeTypes();

// pre-populate namespace cache with standard JCR namespaces
initNamespaceCache();
}

public Set<String> getNodeTypes() {
Expand All @@ -324,6 +364,135 @@ public String getNamespaceURI(String prefix) throws RepositoryException {
return session.getNamespaceURI(prefix);
}

/**
* Gets a namespace URI from the cache or from the session if not cached.
* This method caches the prefix-to-URI mapping to avoid repeated JCR lookups.
*
* @param prefix the namespace prefix
* @return the namespace URI
* @throws RepositoryException if an error occurs
*/
Copy link
Copy Markdown
Contributor

@reschke reschke Jan 26, 2026

Choose a reason for hiding this comment

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

How is this better than using the JCR namespace registry? Why do we need a cache here?

public String getCachedNamespaceURI(String prefix) throws RepositoryException {
return namespacePrefixCache.computeIfAbsent(prefix, p -> {
try {
return session.getNamespaceURI(p);
} catch (RepositoryException e) {
throw new RuntimeException("Failed to get namespace URI for prefix: " + p, e);
}
});
}

/**
* Adds a namespace prefix to the cache.
*
* @param prefix the namespace prefix to cache
*/
public void cacheNamespacePrefix(String prefix) {
if (prefix != null && !prefix.isEmpty() && !namespacePrefixCache.containsKey(prefix)) {
try {
String uri = session.getNamespaceURI(prefix);
namespacePrefixCache.put(prefix, uri);
} catch (RepositoryException e) {
// Log but don't fail - the prefix might be checked later
log.debug("Could not resolve namespace URI for prefix: {}", prefix, e);
}
}
}

/**
* Gets the cached namespace prefixes.
*
* @return a set of all cached namespace prefixes
*/
public Set<String> getCachedNamespacePrefixes() {
return namespacePrefixCache.keySet();
}

/**
* Gets the cache size configuration for the aggregate namespace cache.
*
* @return the maximum cache size, defaulting to {@link #DEFAULT_AGGREGATE_CACHE_SIZE}
*/
private int getAggregateCacheSizeFromConfig() {
String cacheSizeStr = config.getProperty("aggregateNamespaceCacheSize");
if (cacheSizeStr != null) {
try {
int size = Integer.parseInt(cacheSizeStr);
if (size > 0) {
log.info("Using configured aggregate namespace cache size: {}", size);
return size;
}
} catch (NumberFormatException e) {
log.warn(
"Invalid aggregate cache size '{}', using default: {}",
cacheSizeStr,
DEFAULT_AGGREGATE_CACHE_SIZE);
}
}
return DEFAULT_AGGREGATE_CACHE_SIZE;
}

/**
* Gets cached namespace prefixes for a specific aggregate path.
* Uses a bounded LRU cache to prevent unbounded memory growth.
*
* @param path the aggregate path
* @return the cached prefixes, or null if not cached
*/
public String[] getCachedAggregatePrefixes(String path) {
return aggregateNamespaceCache.get(path);
}

/**
* Caches namespace prefixes for a specific aggregate path.
* Uses a bounded LRU cache with automatic eviction of least recently used entries.
*
* @param path the aggregate path
* @param prefixes the namespace prefixes to cache
*/
public void cacheAggregatePrefixes(String path, String[] prefixes) {
if (path != null && prefixes != null) {
aggregateNamespaceCache.put(path, prefixes);
if (log.isTraceEnabled()) {
log.trace(
"Cached namespace prefixes for path '{}': {} (cache size: {})",
path,
prefixes,
aggregateNamespaceCache.size());
}
}
}

/**
* Invalidates the aggregate namespace cache for a specific path.
* This should be called when content at that path is modified.
*
* @param path the aggregate path to invalidate
*/
public void invalidateAggregatePrefixes(String path) {
if (path != null) {
String[] removed = aggregateNamespaceCache.remove(path);
if (removed != null) {
log.debug("Invalidated namespace cache for path: {}", path);
}
}
}

/**
* Invalidates all namespace caches. This should be called if namespace
* definitions are added or modified in the repository.
*/
public void invalidateNamespaceCaches() {
log.info(
"Invalidating namespace caches ({} prefix mappings, {} aggregate caches)",
namespacePrefixCache.size(),
aggregateNamespaceCache.size());
namespacePrefixCache.clear();
aggregateNamespaceCache.clear();
// Re-initialize the prefix cache with current repository namespaces
initNamespaceCache();
}

public void startTracking(ProgressTrackerListener pTracker) {
tracker = new AggregatorTracker(pTracker);
}
Expand Down Expand Up @@ -426,6 +595,27 @@ private void initNodeTypes() throws RepositoryException {
}
}

/**
* Pre-populates the namespace cache with all namespaces registered in the repository.
* This optimization reduces expensive JCR tree traversals during namespace discovery.
*/
private void initNamespaceCache() {
try {
String[] prefixes = session.getNamespacePrefixes();
for (String prefix : prefixes) {
try {
String uri = session.getNamespaceURI(prefix);
namespacePrefixCache.put(prefix, uri);
} catch (RepositoryException e) {
log.debug("Could not cache namespace prefix '{}': {}", prefix, e.getMessage());
}
}
log.info("Initialized namespace cache with {} prefixes", namespacePrefixCache.size());
} catch (RepositoryException e) {
log.warn("Could not initialize namespace cache", e);
}
}

public Aggregator getAggregator(Node node, String path) throws RepositoryException {
return aggregatorProvider.getAggregator(node, path);
}
Expand Down
Loading