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 Thomas Mueller <mu...@adobe.com> on 2012/09/13 17:42:37 UTC
ContentSession.getCurrentRoot() is slow
Hi,
To read a node, the query engine currently uses:
session.getCurrentRoot().getTree(path);
The query engine calls this whenever it has to evaluate a property. It
turns out internally the getCurrentRoot() method always calls
MicroKernel.getHeadRevision(). I wonder if this is required, and if yes,
why? The javadoc for this method is:
/**
* The current root as seen by this content session. Use
* {@link Root#commit(ConflictHandler)} to atomically apply the
changes made in that
* subtree the underlying Microkernel.
*
* @return the current root
*/
Should I cache the Root instance in the query engine for the duration of a
query? Or would it be possible to change the Root implementation so this
is not needed?
Regards,
Thomas
Re: ContentSession.getCurrentRoot() is slow
Posted by Michael Dürig <md...@apache.org>.
On 13.9.12 16:42, Thomas Mueller wrote:
> Hi,
>
> To read a node, the query engine currently uses:
>
> session.getCurrentRoot().getTree(path);
>
> The query engine calls this whenever it has to evaluate a property. It
> turns out internally the getCurrentRoot() method always calls
> MicroKernel.getHeadRevision(). I wonder if this is required, and if yes,
> why? The javadoc for this method is:
>
> /**
> * The current root as seen by this content session. Use
> * {@link Root#commit(ConflictHandler)} to atomically apply the
> changes made in that
> * subtree the underlying Microkernel.
> *
> * @return the current root
> */
>
> Should I cache the Root instance in the query engine for the duration of a
> query? Or would it be possible to change the Root implementation so this
> is not needed?
The root instance gives you a stable view of the tree at the time you
acquired the root. That's why acquiring a new root does a round trip to
the Microkernel. However evaluating a query I think its not only better
but also the correct thing to acquire the root once for the whole query
evaluation.
Michael
>
> Regards,
> Thomas
>
>
Re: ContentSession.getCurrentRoot() is slow
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Fri, Sep 14, 2012 at 7:32 AM, Thomas Mueller <mu...@adobe.com> wrote:
> This is actually what I expected that ContentSession.getCurrentRoot()
> would provide me: I expected getCurrentRoot() would always return the same
> root until the session has committed or refreshed. It wasn't obvious to me
> neither from the method name nor from the javadocs that there is
> (potentially) a new current root every time the method is called. Not a
> problem, I can change the javadocs and add that info.
Yep, the javadocs should probably be improved for this. The rationale
for not always returning the same root is to allow oak-jcr to
implement directly persisted operations like workspace copies without
having to acquire a separate ContentSession (and thus re-authenticate
itself).
> But now I see that the ContentSession doesn't in fact know the current
> revision and doesn't provide snapshot isolation, how do I use the same
> tree snapshot as the rest of the session? Is it the oak-jcr
> SessionDelegate.root field? I guess I will have to change the oak-core
> query API then, and add the Root as a parameter to the method
> SessionQueryEngine.executeQuery?
A better alternative might be to move the getQueryEngine() method from
ContentSession to Root. That way the returned QueryEngine instance can
already contain an internal reference to the relevant Root.
BR,
Jukka Zitting
Re: ContentSession.getCurrentRoot() is slow
Posted by Michael Dürig <md...@apache.org>.
Hi,
On 14.9.12 9:52, Thomas Mueller wrote:
> Hi,
>
>>> About workspaces - I know we said we don't support workspaces currently,
>>> but there still is ContentSession.getWorkspaceName(). Could we remove
>>> this
>>> method for now?
>
>> Currently the workspace root and the repository root are the same
>> thing, while we only support a single workspace. Once we get to adding
>> support for multiple workspaces we need to define these semantics in
>> more detail.
>
> I know, but what about removing ContentSession.workspaceName for now? As
> far as I see, it's used in the constructors of ContentSessionImpl and
> RootImpl, and in some other places, but not really used. It would make the
> code clearer to me.
This is a left over from when I removed workspace support back then when
we decided not to implement it for now. To me it was obvious to leave
these methods there as stubs to remind us that we need to implement this
later on. There are even TODOs in the relevant places in the code where
the implementation would/should go.
>
>>> About the class Root: is the data returned by this class filtered for the
>>> ContentSessions access rights? I can't find that info in the Javadocs.
>>
>> Yes, access controls are applied to Root and everything you can access
>> through it.
>
> OK I will document that. I guess it's the same for the class Tree? What
> about PropertyState? As far as I see, you can get such objects from a Tree
> and in other ways - I guess access rights are only applied when using
> Tree.getProperty?
Look out for the word "accessible" in the Javadoc of Root and Tree. Its
already there.
Michael
Re: ContentSession.getCurrentRoot() is slow
Posted by Thomas Mueller <mu...@adobe.com>.
Hi,
>>About workspaces - I know we said we don't support workspaces currently,
>> but there still is ContentSession.getWorkspaceName(). Could we remove
>>this
>> method for now?
>Currently the workspace root and the repository root are the same
>thing, while we only support a single workspace. Once we get to adding
>support for multiple workspaces we need to define these semantics in
>more detail.
I know, but what about removing ContentSession.workspaceName for now? As
far as I see, it's used in the constructors of ContentSessionImpl and
RootImpl, and in some other places, but not really used. It would make the
code clearer to me.
>>About the class Root: is the data returned by this class filtered for the
>> ContentSessions access rights? I can't find that info in the Javadocs.
>
>Yes, access controls are applied to Root and everything you can access
>through it.
OK I will document that. I guess it's the same for the class Tree? What
about PropertyState? As far as I see, you can get such objects from a Tree
and in other ways - I guess access rights are only applied when using
Tree.getProperty?
>> If it is, maybe we could (later on) combine Root and ContentSession.
>
>The separation between ContentSession and Root was made to allow for
>directly persisted operations without the need to re-authenticate
>every time.
I think there is a misunderstanding. As far as I see, the ContentSession
does already contain the authentication information (getAuthInfo). I don't
see how combining Root and ContentSession would force you to
re-authenticate.
But I guess it's not the right time to discuss combining Root and
ContentSession. I guess we should wait with that until we know what to do
with workspaces.
Regards,
Thomas
Re: ContentSession.getCurrentRoot() is slow
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Fri, Sep 14, 2012 at 9:47 AM, Thomas Mueller <mu...@adobe.com> wrote:
> About workspaces - I know we said we don't support workspaces currently,
> but there still is ContentSession.getWorkspaceName(). Could we remove this
> method for now? The problem is that it's not clear whether
> getCurrentRoot(), and the Root class, is for the / a workspace or for the
> complete repository.
Currently the workspace root and the repository root are the same
thing, while we only support a single workspace. Once we get to adding
support for multiple workspaces we need to define these semantics in
more detail.
> About the class Root: is the data returned by this class filtered for the
> ContentSessions access rights? I can't find that info in the Javadocs.
Yes, access controls are applied to Root and everything you can access
through it.
> If it is, maybe we could (later on) combine Root and ContentSession.
The separation between ContentSession and Root was made to allow for
directly persisted operations without the need to re-authenticate
every time.
BR,
Jukka Zitting
Re: ContentSession.getCurrentRoot() is slow
Posted by Thomas Mueller <mu...@adobe.com>.
Hi,
What about renaming ContentSession.getCurrentRoot() to getHeadRoot() or
getCurrentHeadRoot()?
About workspaces - I know we said we don't support workspaces currently,
but there still is ContentSession.getWorkspaceName(). Could we remove this
method for now? The problem is that it's not clear whether
getCurrentRoot(), and the Root class, is for the / a workspace or for the
complete repository.
About the class Root: is the data returned by this class filtered for the
ContentSessions access rights? I can't find that info in the Javadocs. If
it is, maybe we could (later on) combine Root and ContentSession. But for
now I guess it's enough to add a comment in the Javadoc.
Regards,
Thomas
On 9/14/12 7:32 AM, "Thomas Mueller" <mu...@adobe.com> wrote:
>Hi,
>
>>It would be better if the query engine used the same tree snapshot as
>>the rest of the session. There shouldn't be any need to call
>>getCurrentRoot().
>
>This is actually what I expected that ContentSession.getCurrentRoot()
>would provide me: I expected getCurrentRoot() would always return the same
>root until the session has committed or refreshed. It wasn't obvious to me
>neither from the method name nor from the javadocs that there is
>(potentially) a new current root every time the method is called. Not a
>problem, I can change the javadocs and add that info.
>
>But now I see that the ContentSession doesn't in fact know the current
>revision and doesn't provide snapshot isolation, how do I use the same
>tree snapshot as the rest of the session? Is it the oak-jcr
>SessionDelegate.root field? I guess I will have to change the oak-core
>query API then, and add the Root as a parameter to the method
>SessionQueryEngine.executeQuery?
>
>Regards,
>Thomas
>
Re: ContentSession.getCurrentRoot() is slow
Posted by Thomas Mueller <mu...@adobe.com>.
Hi,
>It would be better if the query engine used the same tree snapshot as
>the rest of the session. There shouldn't be any need to call
>getCurrentRoot().
This is actually what I expected that ContentSession.getCurrentRoot()
would provide me: I expected getCurrentRoot() would always return the same
root until the session has committed or refreshed. It wasn't obvious to me
neither from the method name nor from the javadocs that there is
(potentially) a new current root every time the method is called. Not a
problem, I can change the javadocs and add that info.
But now I see that the ContentSession doesn't in fact know the current
revision and doesn't provide snapshot isolation, how do I use the same
tree snapshot as the rest of the session? Is it the oak-jcr
SessionDelegate.root field? I guess I will have to change the oak-core
query API then, and add the Root as a parameter to the method
SessionQueryEngine.executeQuery?
Regards,
Thomas
Re: ContentSession.getCurrentRoot() is slow
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Thu, Sep 13, 2012 at 5:42 PM, Thomas Mueller <mu...@adobe.com> wrote:
> Should I cache the Root instance in the query engine for the duration of a
> query? Or would it be possible to change the Root implementation so this
> is not needed?
It would be better if the query engine used the same tree snapshot as
the rest of the session. There shouldn't be any need to call
getCurrentRoot().
BR,
Jukka Zitting