You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by Marcel Reutegger <mr...@adobe.com> on 2013/03/19 15:00:42 UTC
RE: svn commit: r1458234 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/core/
oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/
oak-core/src/main/java/o
Hi,
I'd rather avoid casting to an implementation specific class, even if it is
conditional. This may introduce implementation dependent behavior,
which makes debugging more difficult.
If it is for performance reasons, we should look into finding another solution
because the SegmentMK doesn't have KernelNodeStates!
Regards
Marcel
> --- jackrabbit/oak/trunk/oak-
> core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableTree.java
> (original)
> +++ jackrabbit/oak/trunk/oak-
> core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableTree.java
> Tue Mar 19 11:48:36 2013
> @@ -26,6 +26,7 @@ import org.apache.jackrabbit.oak.api.Pro
> import org.apache.jackrabbit.oak.api.Root;
> import org.apache.jackrabbit.oak.api.Tree;
> import org.apache.jackrabbit.oak.commons.PathUtils;
> +import org.apache.jackrabbit.oak.kernel.KernelNodeState;
> import org.apache.jackrabbit.oak.plugins.version.VersionConstants;
> import org.apache.jackrabbit.oak.spi.security.Context;
> import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry;
> @@ -91,9 +92,14 @@ public final class ImmutableTree extends
> return "/";
> }
>
> - StringBuilder sb = new StringBuilder();
> - buildPath(sb);
> - return sb.toString();
> + NodeState nodeState = getNodeState();
> + if (nodeState instanceof KernelNodeState) {
> + return ((KernelNodeState) nodeState).getPath();
> + } else {
> + StringBuilder sb = new StringBuilder();
> + buildPath(sb);
> + return sb.toString();
> + }
> }
>
> private void buildPath(StringBuilder sb) {
>
Re: svn commit: r1458234 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/core/
oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/
oak-core/src/main/java/o
Posted by Angela Schreiber <an...@adobe.com>.
hi marcel
On 3/19/13 4:09 PM, Marcel Reutegger wrote:
>> right... but i don't see why we have to recalculate the path
>> when the node states already know about them and we usually
>> do know the path of an immutable tree because we have
>> traversed to there using the TreeLocations.
>
> the problem is, only the KernelNodeState knows about its path,
> but none of the other five classes implementing NodeState do.
>
>> IMO we should have NodeState#getPath but as long as we don't
>> have that i will need sort of workaround to have an efficient
>> way to handle ImmutableTree#getPath.
>
> AFAIU, NodeState was specifically designed to not require a path.
>
> how about caching the path in ImmutableTree? IIUC this tree
> implementation works on top of NodeStates, which means the
> path will never change, right?
we had that already in a previous version but it got reverted.
i can live with any solution as long as the path is not
calculated when it's actually known already.
kind regards
angela
> Regards
> Marcel
Re: svn commit: r1458234 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/core/ oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/ oak-core/src/main/java/o
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Fri, Mar 22, 2013 at 7:25 PM, Angela Schreiber <an...@adobe.com> wrote:
> however, please keep in mind that if we are really going to move
> the permission evaluation below the tree level, we
> a) need to have an efficient way to determine the path of a
> given node state (or it's immutabletree representation)
> as our permission evaluation basically is path-based
Agreed. My main point above was just to note that the cost of caching
parent node states around is lower than that of caching paths.
We can definitely keep the path around in places where it is needed,
but as mentioned earlier I'd really like to avoid having to calculate
it for *all* node states. For example, assuming we implement the
optimization of detecting that the current user has full access to an
entire subtree, then there should be no need to keep track of paths
within that subtree as we can just skip all further access checks in
that subtree.
BR,
Jukka Zitting
Re: svn commit: r1458234 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/core/
oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/
oak-core/src/main/java/o
Posted by Angela Schreiber <an...@adobe.com>.
hi jukka
sure... you may want to look at the items accessed for a
regular cq page request in order to come up with a realistic
scenario and then scale that up to a huge repository.
however, please keep in mind that if we are really going to move
the permission evaluation below the tree level, we
a) need to have an efficient way to determine the path of a
given node state (or it's immutabletree representation)
as our permission evaluation basically is path-based
b) may end up with different caches for different set of
principals (as discussed during the last okathon).
kind regards
angela
On 3/22/13 4:41 PM, Jukka Zitting wrote:
> Hi,
>
> On Fri, Mar 22, 2013 at 5:01 PM, Angela Schreiber<an...@adobe.com> wrote:
>> quite frankly: IMHO the bigger impact on memory usage is
>> that afaik we currently load all parent states which most likely
>> will not be all accessed by the oak/jcr API calls.
>
> I don't think so. The number of intermediate nodes that need to be
> traversed to access distinct N nodes is O(log N) in a typical
> hierarchical structure. In contrast the number of paths that would
> have to be kept around is O(N).
>
> That should be easy to verify with a benchmark. I'll see if I can come
> up with one.
>
> BR,
>
> Jukka Zitting
Re: svn commit: r1458234 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/core/ oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/ oak-core/src/main/java/o
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Fri, Mar 22, 2013 at 5:01 PM, Angela Schreiber <an...@adobe.com> wrote:
> quite frankly: IMHO the bigger impact on memory usage is
> that afaik we currently load all parent states which most likely
> will not be all accessed by the oak/jcr API calls.
I don't think so. The number of intermediate nodes that need to be
traversed to access distinct N nodes is O(log N) in a typical
hierarchical structure. In contrast the number of paths that would
have to be kept around is O(N).
That should be easy to verify with a benchmark. I'll see if I can come
up with one.
BR,
Jukka Zitting
Re: svn commit: r1458234 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/core/
oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/
oak-core/src/main/java/o
Posted by Angela Schreiber <an...@adobe.com>.
quite frankly: IMHO the bigger impact on memory usage is
that afaik we currently load all parent states which most likely
will not be all accessed by the oak/jcr API calls. what we keep
having in CQ apart from the original installation rather
resembles random access (here a image node, from there the
design from the third location the script to render it and so
forth) than strict hierarchical access.
kind regards
angela
On 3/22/13 12:48 PM, Michael Dürig wrote:
>
>
> On 19.3.13 15:09, Marcel Reutegger wrote:
>
>> how about caching the path in ImmutableTree? IIUC this tree
>> implementation works on top of NodeStates, which means the
>> path will never change, right?
>
> Caching the path has a quite bad impact on memory usage. See
> http://markmail.org/message/xhlr44umptmdnxck
>
> Michael
Re: svn commit: r1458234 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/core/
oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/
oak-core/src/main/java/o
Posted by Michael Dürig <md...@apache.org>.
On 19.3.13 15:09, Marcel Reutegger wrote:
> how about caching the path in ImmutableTree? IIUC this tree
> implementation works on top of NodeStates, which means the
> path will never change, right?
Caching the path has a quite bad impact on memory usage. See
http://markmail.org/message/xhlr44umptmdnxck
Michael
RE: svn commit: r1458234 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/core/
oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/
oak-core/src/main/java/o
Posted by Marcel Reutegger <mr...@adobe.com>.
> right... but i don't see why we have to recalculate the path
> when the node states already know about them and we usually
> do know the path of an immutable tree because we have
> traversed to there using the TreeLocations.
the problem is, only the KernelNodeState knows about its path,
but none of the other five classes implementing NodeState do.
> IMO we should have NodeState#getPath but as long as we don't
> have that i will need sort of workaround to have an efficient
> way to handle ImmutableTree#getPath.
AFAIU, NodeState was specifically designed to not require a path.
how about caching the path in ImmutableTree? IIUC this tree
implementation works on top of NodeStates, which means the
path will never change, right?
Regards
Marcel
Re: svn commit: r1458234 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/core/
oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/
oak-core/src/main/java/o
Posted by Angela Schreiber <an...@adobe.com>.
right... but i don't see why we have to recalculate the path
when the node states already know about them and we usually
do know the path of an immutable tree because we have
traversed to there using the TreeLocations.
IMO we should have NodeState#getPath but as long as we don't
have that i will need sort of workaround to have an efficient
way to handle ImmutableTree#getPath.
kind regards
angela
On 3/19/13 3:00 PM, Marcel Reutegger wrote:
> Hi,
>
> I'd rather avoid casting to an implementation specific class, even if it is
> conditional. This may introduce implementation dependent behavior,
> which makes debugging more difficult.
>
> If it is for performance reasons, we should look into finding another solution
> because the SegmentMK doesn't have KernelNodeStates!
>
> Regards
> Marcel
>
>> --- jackrabbit/oak/trunk/oak-
>> core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableTree.java
>> (original)
>> +++ jackrabbit/oak/trunk/oak-
>> core/src/main/java/org/apache/jackrabbit/oak/core/ImmutableTree.java
>> Tue Mar 19 11:48:36 2013
>> @@ -26,6 +26,7 @@ import org.apache.jackrabbit.oak.api.Pro
>> import org.apache.jackrabbit.oak.api.Root;
>> import org.apache.jackrabbit.oak.api.Tree;
>> import org.apache.jackrabbit.oak.commons.PathUtils;
>> +import org.apache.jackrabbit.oak.kernel.KernelNodeState;
>> import org.apache.jackrabbit.oak.plugins.version.VersionConstants;
>> import org.apache.jackrabbit.oak.spi.security.Context;
>> import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry;
>> @@ -91,9 +92,14 @@ public final class ImmutableTree extends
>> return "/";
>> }
>>
>> - StringBuilder sb = new StringBuilder();
>> - buildPath(sb);
>> - return sb.toString();
>> + NodeState nodeState = getNodeState();
>> + if (nodeState instanceof KernelNodeState) {
>> + return ((KernelNodeState) nodeState).getPath();
>> + } else {
>> + StringBuilder sb = new StringBuilder();
>> + buildPath(sb);
>> + return sb.toString();
>> + }
>> }
>>
>> private void buildPath(StringBuilder sb) {
>>