[ https://jira.jboss.org/jira/browse/DNA-466?page=com.atlassian.jira.plugin... ] Randall Hauch commented on DNA-466: ----------------------------------- The reason this was done in the first place was because the path of a node can change (as it or its parents are moved). Ensuring the cached path is difficult to do efficiently. If the Path is stored on each node, then the paths of all nodes below any moved node becomes invalid; changing all the cached paths on all nodes in this branch could be expensive if the cached branch is large. Different means can be used to determine whether a cached path is still valid (e.g., if ChildPath objects are used, each ChildPath instance references the Path of it's parent; walk up the ancestors and ensure the ChildPath is indeed referencing the same Path instance as it's parent), but all those I've thought of add significant complexity. Plus, although the Paths could be stored in a Map<UUID,Path>, the NodeInfo objects are already stored in a Map<UUID,NodeInfo>, and it would be more efficient to cache the Path in the NodeInfo. But since the Path may change, the NodeInfo could no longer be mutable. Perhaps the simplest solution is simply a Map<UUID,Path>, and to simply clear this map every time a node is used. Retrieving a Path for a node would involve a simple map lookup, and any returned Path could be used as is. If the map contains no Path, then one would be computed (by walking up the ancestors, checking of course for the existence in the map of a Path for each ancestor) and added to the map. Although any move would require recomputing all node paths from scratch, this is not really any worse than what we have now. And in many ways, it would actually be an improvement as there appears to be frequently-occurring situations where the Path for a node is retrieved multiple times. Before I make these changes, I would like to measure the performance of reading all the nodes in a graph and obtaining each node's path (and perhaps doing this multiple times). This can be done in the SessionCacheTest. After the path caching is implemented, the performance should be no worse. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://jira.jboss.org/jira/browse/DNA-466?page=com.atlassian.jira.plugin... ] Randall Hauch commented on DNA-466: ----------------------------------- Changed the JCR implementation to use the path and UUID when both are known. However, there still is a problem. When the JCR session is saved but the client holds onto Node objects, and then those Node objects are used, only the UUID of those nodes are retained (because they're in the Node implementation objects, and the session's cache is cleared after a save). -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://jira.jboss.org/jira/browse/DNA-466?page=com.atlassian.jira.plugin... ] Randall Hauch commented on DNA-466: ----------------------------------- I've been working on a redesign of SessionCache that does not rely upon UUIDs from the connectors. Current status is that although there were a few regressions that can be fixed pretty easily, I'm at a point where I'm merging more recent commits and resolving quite a few conflicts. But that shouldn't take too long, and I'll be able to commit this new design. Rather than maintain the cache keyed by the UUIDs, the new design maintains a structured graph of the cached information kept in-sync with the changes to the session, making traversals and navigation significantly faster. Because this structure is maintained, getting the path for any loaded node is extremely fast. In fact, all paths are cached (and actually make use of the ChildPath implementation, where each Path object is a single segment plus a reference to the parent Path; this is much better use of memory) I'll go more into the design later, but it's interesting to see some of the same stats for walking the same graph of 42 nodes in a JCR repository, AFTER the SessionCache redesign. 42 samples: min=00:00:00.000,002; avg=00:00:00.001,046; median=00:00:00.000,002; stddev=6679752.669664584; max=00:00:00.043,817 -> 42 42 samples: min=00:00:00.000,001; avg=00:00:00.000,003; median=00:00:00.000,002; stddev=4310.247242620428; max=00:00:00.000,03 -> 0 42 samples: min=00:00:00.000,001; avg=00:00:00.000,002; median=00:00:00.000,002; stddev=2795.0596195151174; max=00:00:00.000,02 -> 0 Note that almost all of the stats are significantly lower (often several order of magnitude smaller); the maximum on the initial warmup run is higher. Note that with the new design, I'm pretty sure the new tests are doing more than the old tests - the new tests force loading of every node being walked, not just having enough information to construct the path. The two different designs were just apples and oranges. When I only include the time required to obtain the paths (what the old test was doing), the numbers are significantly smaller still: 42 samples: min=00:00:00.000,002; avg=00:00:00.000,006; median=00:00:00.000,002; stddev=22080.827094338383; max=00:00:00.000,147 -> 42 42 samples: min=00:00:00.000,001; avg=00:00:00.000,002; median=00:00:00.000,002; stddev=3262.2523276768625; max=00:00:00.000,023 -> 0 42 samples: min=00:00:00.000,001; avg=00:00:00.000,002; median=00:00:00.000,002; stddev=1807.0158058105023; max=00:00:00.000,013 -> 0 BTW, a small change was made to the printout. In each line, the last number represents the total number of connections made to the source to do the various reads as the test walked the graph. In the above example, the first walking produced a separate read for each node; subsequent walks found everything and required no loading. The new design also has a setting (in GraphSession) that defines the minimum depth to read for every node being loaded (theoretically reducing the total number of read operations). Here is a value of 2 (meaning a node and its children are all read as a subgraph). With a total depth of 5 (including the root), there's not a whole lot of reduction in the number of connections (33, down from 42). Again, these stats include fully loading each node (rather than just accessing the path, which is near instantaneous): 42 samples: min=00:00:00.000,001; avg=00:00:00.000,439; median=00:00:00.000,002; stddev=2800757.049523132; max=00:00:00.018,373 -> 33 42 samples: min=00:00:00.000,001; avg=00:00:00.000,003; median=00:00:00.000,002; stddev=3339.1106396682303; max=00:00:00.000,023 -> 0 42 samples: min=00:00:00.000,001; avg=00:00:00.000,002; median=00:00:00.000,002; stddev=3382.377298657882; max=00:00:00.000,024 -> 0 Here is the same run, but with a depth value of 6, meaning that the entire graph is read in with one read operation. Yes, that one read took 217 milliseconds, but considering this test uses an in-memory source, all the connections are fast, while other sources (e.g., databases, file systems, etc.) will likely take longer. 42 samples: min=00:00:00.000,001; avg=00:00:00.005,188; median=00:00:00.000,002; stddev=3.3205427276732728E7; max=00:00:00.217,806 -> 1 42 samples: min=00:00:00.000,001; avg=00:00:00.000,002; median=00:00:00.000,002; stddev=3136.0857790573014; max=00:00:00.000,022 -> 0 42 samples: min=00:00:00.000,001; avg=00:00:00.000,002; median=00:00:00.000,002; stddev=3423.027925887531; max=00:00:00.000,024 -> 0 It's not clear what depth value will be useful and most efficient. But it is a parameter that can be optimized later (perhaps with different recommended values for different kinds of sources). All the better, considering that there was not much additional code required to handle depth reading values > 1. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://jira.jboss.org/jira/browse/DNA-466?page=com.atlassian.jira.plugin... ] Randall Hauch commented on DNA-466: ----------------------------------- Okay, after several weeks of working on this issue (including several failed attempts at more direct fixes to address the problem), I was finally able to commit the code. The new design is obviously much improved when it comes to getting and using the paths. But it also relies upon the paths and no longer expects UUIDs from connectors (other than in cases where nodes are referenceable, per the JCR specification). The new design broke the functionality in SessionCache into two distinct parts. A new GraphSession class (in org.jboss.dna.graph.session) now handles the job of maintaining a cached representation of a portion of the graph structure, including accumulating changes and batching various connector requests. The JCR-specific functionality is now in SessionCache (still in org.jboss.dna.jcr) and is responsible for checking constraints, ensuring proper node and property types, maintaining cached representations of type names, and management of the JCR Node and Property implementation objects. One big plus is that it helps separate the different kinds of logic, making testing easier. The GraphSession class maintains a structured tree of node objects, ensuring that this transient structure stays in sync as various nodes are created, moved, deleted, cloned, or changed. The session's behavior is patterned after JCR: it maintains transient state and changes, and these changes are pushed to the store upon 'save()'. The node structure is not designed to be extended; rather, each node has a payload object that can be configured and customized (via generics). Similarly, each node consists of a property info object for each property, and this property info contains a payload object that is customizable. (SessionCache uses payload classes that store/cache the JCR-specific data.) Quite a few hooks are available for inserting logic at various points in the lifecycle; for example, when a node is being loaded into the session, a hook is called to allow a client to populate the property info objects and child nodes, given the Graph node object. This allows SessionCache to insert its own logic, including the determination of the primary type and child node definition, as well as caching any JCR-specific information in the payload. As alluded to in a previous comment, the GraphSession structure maintains a Location object for every node. That Path object in that Location is created from the Path of the parent node, which means that the ChildPath implementation can be used for all Path objects. This means that each Path.Segment appears only once but is reused in every Path object on all nodes at or below that spot. The Location and Path objects are also made to reflects the SNS indexes, so as the SNS indexes change, the paths are recomputed. (Yes, this makes moves, deletes, and reorders more expensive, especially for large subgraphs, but then almost every other operation becomes much faster.) And like the previous SessionCache implementation, the GraphSession uses a MultiMap to efficiently manage the children of a node, without having to compute the SNS indexes (other than when updating the Location objects, which is done only when needed). The GraphSession also takes a different approach to storing the transient changes. The old SessionCache design kept immutable snapshots of each node state as read from the store. The idea was that the changes could be easily discarded while keeping the last-read state. But in practice, the only times the changes are discarded is via javax.jcr.Session.refresh() or javax.jcr.Node.refresh() methods, which also throws out any cached state. Thus, the old design unnecessarily maintained separate "read" states from the mutable "changed" states. The new design simply keeps the structure in-sync with all changes, and each node maintains some state about whether it is new or has changed, and whether any nodes below it have changed. This makes it easy to walk the structure to find out what areas are changed, what's new, and what hasn't even been loaded yet. (The new design uses visitors to accomplish the walking activities.) So, apart from the faster access to paths, the new design was to get around the difficulty of the javax.jcr.Node and javax.jcr.Property implementation objects requiring UUIDs. In the old design, these implementation objects were very lightweight - almost every activity required looking up in the SessionCache the corresponding node/property information object and then delegating to methods on that object. All nodes were identified by UUID, so each javax.jcr.Node and javax.jcr.Property implementation object simply had to know the UUID. Unfortunately, this design doesn't work when the node has no UUID. (Earlier, I experimented with creating artificial UUIDs, but this is not durable and causes problems when 'save()' or 'refresh()' are performed, causing the javax.jcr.Node and javax.jcr.Property implementation objects to become stale and unable to find the corresponding node/property info objects.) In the new design, we still have very lightweight javax.jcr.Node and javax.jcr.Property implementation objects that still hold on to a unique key. However, rather than a single UUID, the key is a session-specific NodeId (which can become invalid) and a durable Location of the node. (The javax.jcr.Property implementations maintain a reference to the javax.jcr.Node implementation object and the property's name). The SessionCache is able to find the corresponding GraphSession node/property given this combined key, and can almost always locate the node/property because the key has the last-known path. Also, because the Location objects *might* have identification properties, these are used when the path is not sufficient or is no longer valid. There are cases when this is not successful, but those cases correspond to those in the JCR specification (e.g., nodes being deleted or nodes being changed in other sessions). In these cases, the JCR client gets an InvalidItemStateException, and retrieves the node from the session. There are a few outstanding items and known issues that have to be resolved. However, these will be tracked as separate issues. In fact, this issue will likely be dependent on a few of those - when they're finished, this issue can be marked as resolved. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira