JCRVLT-837 adding cached namespace impl to prevent thrashing in the r…#411
JCRVLT-837 adding cached namespace impl to prevent thrashing in the r…#411pat-lego wants to merge 2 commits intoapache:masterfrom
Conversation
| // Check if this aggregate's namespaces are already cached | ||
| String[] cachedPrefixes = mgr.getCachedAggregatePrefixes(path); | ||
| if (cachedPrefixes != null) { | ||
| log.debug("Using cached namespace prefixes for '{}': {}", path, cachedPrefixes); |
There was a problem hiding this comment.
AFAICT, this code is never reached when running the filevault test suite.
We need a test case that actually covers this case.
There was a problem hiding this comment.
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.
| * @param prefix the namespace prefix | ||
| * @return the namespace URI | ||
| * @throws RepositoryException if an error occurs | ||
| */ |
There was a problem hiding this comment.
How is this better than using the JCR namespace registry? Why do we need a cache here?
| * Cache of namespace prefixes per aggregate path. This cache stores the discovered prefixes | ||
| * for each aggregate path to avoid re-walking the same subtrees. | ||
| */ | ||
| private final ConcurrentHashMap<String, String[]> aggregateNamespaceCache = new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
This is an unbounded cache; and with a potentially very large amount of paths this can lead a huge memory consumption.
| namespacePrefixes = prefixes.toArray(new String[prefixes.size()]); | ||
|
|
||
| // Cache the discovered prefixes for this aggregate path | ||
| mgr.cacheAggregatePrefixes(path, namespacePrefixes); |
There was a problem hiding this comment.
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?
|
I added some optimizations to the code what would be interesting is to use this in the environment that is seeing the performance hit and see if there is an optimization |
|
@pat-lego - this can be closed, right? |
3 participants
Avoid walking the JCR for namespaces.
Use a concurrent hash map to store the prefix and url in a map and update the map when necessary.