joerghoh commented on code in PR #411:
URL:
https://github.com/apache/jackrabbit-filevault/pull/411#discussion_r2729217666
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/AggregateImpl.java:
##########
@@ -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
+ mgr.cacheAggregatePrefixes(path, namespacePrefixes);
Review 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?
##########
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/AggregateManagerImpl.java:
##########
@@ -124,6 +130,18 @@ 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<>();
+
+ /**
+ * 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<>();
Review Comment:
This is an unbounded cache; and with a potentially very large amount of
paths this can lead a huge memory consumption.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
