[jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
[ https://issues.apache.org/jira/browse/JCR-1405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12595201#action_12595201 ] Julian Reschke commented on JCR-1405: - If we have evidence that it actually helps in some cases, +.5. That being said, I think it would be great if we would base these things on reliable test data; that's why I started the jcr-benchmark project after all. > SPI: Introduce NodeInfo.getChildInfos() > --- > > Key: JCR-1405 > URL: https://issues.apache.org/jira/browse/JCR-1405 > Project: Jackrabbit > Issue Type: New Feature > Components: jackrabbit-spi >Reporter: angela > Attachments: JCR-1405.patch, JCR-1405b.patch > > > Improvement suggested by Marcel: > ChildInfo is basically a stripped down NodeInfo. With little effort it would > even be possible to have NodeInfo extends ChildInfo. Not sure how useful that > is, but since we don't have that inheritance in code and at the same time > nearly a 100% overlap it makes me suspicious. > Here's another idea: > introduce a method ChildInfo[] NodeInfo.getChildInfos(). The method either > returns: > - all child infos, which also gives the correct number of child nodes. this > may also mean that an empty array is returned to indicate there are no child > nodes. > - null, to indicate that there are *lots* of child nodes and the method > RepositoryService.getChildInfos() with the iterator should be used. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
[ https://issues.apache.org/jira/browse/JCR-1405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12595181#action_12595181 ] angela commented on JCR-1405: - ok if nobody objects within the next couple of days i will apply michaels patch. angela > SPI: Introduce NodeInfo.getChildInfos() > --- > > Key: JCR-1405 > URL: https://issues.apache.org/jira/browse/JCR-1405 > Project: Jackrabbit > Issue Type: New Feature > Components: jackrabbit-spi >Reporter: angela > Attachments: JCR-1405.patch, JCR-1405b.patch > > > Improvement suggested by Marcel: > ChildInfo is basically a stripped down NodeInfo. With little effort it would > even be possible to have NodeInfo extends ChildInfo. Not sure how useful that > is, but since we don't have that inheritance in code and at the same time > nearly a 100% overlap it makes me suspicious. > Here's another idea: > introduce a method ChildInfo[] NodeInfo.getChildInfos(). The method either > returns: > - all child infos, which also gives the correct number of child nodes. this > may also mean that an empty array is returned to indicate there are no child > nodes. > - null, to indicate that there are *lots* of child nodes and the method > RepositoryService.getChildInfos() with the iterator should be used. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
[ https://issues.apache.org/jira/browse/JCR-1405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12594985#action_12594985 ] Michael Dürig commented on JCR-1405: I tested Angela's initial patch and updated it to the latest revision. The patch works well. In may case it helps to cut down the number of round-trips to the back-end store significantly: In my test case including the ChildInfos within NodeInfo results in 4 calls to RepositoryService.getItemInfos() and in no calls to RepositoryService.getChildInfos() at all. Not including them results in the same 4 calls to RepositoryService.getItemInfos() and additionally in 15 calls to RepositoryService.getChildInfos(). Since the patch does not change existing semantics (just return null for NodeInfo.getChildInfos) but on the other hand has the potential to vastly increase performance in some common case I'd very much like to see this patch applied. > SPI: Introduce NodeInfo.getChildInfos() > --- > > Key: JCR-1405 > URL: https://issues.apache.org/jira/browse/JCR-1405 > Project: Jackrabbit > Issue Type: New Feature > Components: jackrabbit-spi >Reporter: angela > Attachments: JCR-1405.patch, JCR-1405b.patch > > > Improvement suggested by Marcel: > ChildInfo is basically a stripped down NodeInfo. With little effort it would > even be possible to have NodeInfo extends ChildInfo. Not sure how useful that > is, but since we don't have that inheritance in code and at the same time > nearly a 100% overlap it makes me suspicious. > Here's another idea: > introduce a method ChildInfo[] NodeInfo.getChildInfos(). The method either > returns: > - all child infos, which also gives the correct number of child nodes. this > may also mean that an empty array is returned to indicate there are no child > nodes. > - null, to indicate that there are *lots* of child nodes and the method > RepositoryService.getChildInfos() with the iterator should be used. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
[ https://issues.apache.org/jira/browse/JCR-1405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12591972#action_12591972 ] Michael Dürig commented on JCR-1405: I'm in favour of the patch as it is. That is, add getChildInfos() to NodeInfo with the following semantics: If NodeInfo.getChildInfos() returns null, jcr2spi must call RepositoryService.getChildInfos() to determine the children of the current node. Otherwise jcr2spi must not (I can also live with 'should not') call RepositoryService.getChildInfos(). Separate calls to RepositoryService.getChildInfos() result in additional round-trips in cases where the persistence layer returns the children (infos) along with a node. > SPI: Introduce NodeInfo.getChildInfos() > --- > > Key: JCR-1405 > URL: https://issues.apache.org/jira/browse/JCR-1405 > Project: Jackrabbit > Issue Type: New Feature > Components: jackrabbit-spi >Reporter: angela > Attachments: JCR-1405.patch > > > Improvement suggested by Marcel: > ChildInfo is basically a stripped down NodeInfo. With little effort it would > even be possible to have NodeInfo extends ChildInfo. Not sure how useful that > is, but since we don't have that inheritance in code and at the same time > nearly a 100% overlap it makes me suspicious. > Here's another idea: > introduce a method ChildInfo[] NodeInfo.getChildInfos(). The method either > returns: > - all child infos, which also gives the correct number of child nodes. this > may also mean that an empty array is returned to indicate there are no child > nodes. > - null, to indicate that there are *lots* of child nodes and the method > RepositoryService.getChildInfos() with the iterator should be used. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
[ https://issues.apache.org/jira/browse/JCR-1405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12590337#action_12590337 ] angela commented on JCR-1405: - what's the status here? can/should we extend the NodeInfo interface as suggested? > SPI: Introduce NodeInfo.getChildInfos() > --- > > Key: JCR-1405 > URL: https://issues.apache.org/jira/browse/JCR-1405 > Project: Jackrabbit > Issue Type: New Feature > Components: jackrabbit-spi >Reporter: angela > Attachments: JCR-1405.patch > > > Improvement suggested by Marcel: > ChildInfo is basically a stripped down NodeInfo. With little effort it would > even be possible to have NodeInfo extends ChildInfo. Not sure how useful that > is, but since we don't have that inheritance in code and at the same time > nearly a 100% overlap it makes me suspicious. > Here's another idea: > introduce a method ChildInfo[] NodeInfo.getChildInfos(). The method either > returns: > - all child infos, which also gives the correct number of child nodes. this > may also mean that an empty array is returned to indicate there are no child > nodes. > - null, to indicate that there are *lots* of child nodes and the method > RepositoryService.getChildInfos() with the iterator should be used. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
Julian Reschke wrote: Marcel Reutegger wrote: Julian Reschke wrote: Also, let's change public PropertyInfo getPropertyInfo(SessionInfo sessionInfo, PropertyId propertyId) throws ItemNotFoundException, RepositoryException; to public Iterator getPropertyInfo(SessionInfo sessionInfo, PropertyId[] propertyId) throws ItemNotFoundException, RepositoryException; I'm not sure how useful that method is. in what situation will jcr2spi call this method? Anytime it currently calls getPropertyInfo. of course, yes ;) but I meant, when will it take advantage of the PropertyId array? anyway, if we implement JCR-1418 it will be useful then. regards marcel
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
Marcel Reutegger wrote: Julian Reschke wrote: Also, let's change public PropertyInfo getPropertyInfo(SessionInfo sessionInfo, PropertyId propertyId) throws ItemNotFoundException, RepositoryException; to public Iterator getPropertyInfo(SessionInfo sessionInfo, PropertyId[] propertyId) throws ItemNotFoundException, RepositoryException; I'm not sure how useful that method is. in what situation will jcr2spi call this method? Anytime it currently calls getPropertyInfo. BR, Julian
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
Angela Schreiber wrote: Marcel Reutegger wrote: hmmm, what's the exact reason we have ChildInfo? as i say before: the reason for the ChildInfo was to be able to build the hierarchy entry without having to build the complete Node that belongs to that entry. I'm sorry, I was being lazy. should have looked that up myself... the NodeInfo includes - all references pointing to that node I think this should be changed as well in line with the current proposal. I've created a jira issue: https://issues.apache.org/jira/browse/JCR-1418 - the ids of all properties this is only moderate in size and I'd argue that it doesn't add much overhead. regards marcel
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
Julian Reschke wrote: Also, let's change public PropertyInfo getPropertyInfo(SessionInfo sessionInfo, PropertyId propertyId) throws ItemNotFoundException, RepositoryException; to public Iterator getPropertyInfo(SessionInfo sessionInfo, PropertyId[] propertyId) throws ItemNotFoundException, RepositoryException; I'm not sure how useful that method is. in what situation will jcr2spi call this method? regards marcel
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
Angela Schreiber wrote: Michael Dürig wrote: i.e. RepositoryService.getChildInfos() returns NodeInfos instead of ChildInfos. I'd favour such a solution. We might even want to further generalize it to RepositoryService.getNodeInfos(SessionInfo sessionInfo, NodeId[] nodeId) i don't see the benefit of either of those changes since they are already covered by RepositoryService.getItemInfos aren't they? if the overall fealing is, that we don't need getNodeInfo or getChildInfos i'd rather deprecate them. No, they aren't covered. The problem I (still) want to solve is browsing through large collections. The current API requires first obtaining the NodeIds of the children, and then resolving them individually. This is expensive. What I think we need is a way to ask for the NodeInfos of the children directly. BR, Julian
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
Michael Dürig wrote: i.e. RepositoryService.getChildInfos() returns NodeInfos instead of ChildInfos. I'd favour such a solution. We might even want to further generalize it to RepositoryService.getNodeInfos(SessionInfo sessionInfo, NodeId[] nodeId) i don't see the benefit of either of those changes since they are already covered by RepositoryService.getItemInfos aren't they? if the overall fealing is, that we don't need getNodeInfo or getChildInfos i'd rather deprecate them. angela
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
Marcel Reutegger wrote: hmmm, what's the exact reason we have ChildInfo? as i say before: the reason for the ChildInfo was to be able to build the hierarchy entry without having to build the complete Node that belongs to that entry. the NodeInfo includes - all references pointing to that node - the ids of all properties angela
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
Michael Dürig wrote: ...we may want to rename it to getNodeInfos() then :-) Actually I'd prefer something along the line of getChildNodeInfos() to be more explicit and to clearly differentiate it from getNodeInfo(). Now we're talking :-) Let's change public Iterator getChildInfos(SessionInfo sessionInfo, NodeId parentId) throws ItemNotFoundException, RepositoryException; to public Iterator getChildNodeInfos(SessionInfo sessionInfo, NodeId parentId) throws ItemNotFoundException, RepositoryException; (using generics notation for clarity) Also, let's change public PropertyInfo getPropertyInfo(SessionInfo sessionInfo, PropertyId propertyId) throws ItemNotFoundException, RepositoryException; to public Iterator getPropertyInfo(SessionInfo sessionInfo, PropertyId[] propertyId) throws ItemNotFoundException, RepositoryException; ...so that batch handling applies to properties as well, and many can ve read in one call (I would assume that many properties are always fetched together, such as those on nt:resource or mix:lockable) BR, Julian
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
...we may want to rename it to getNodeInfos() then :-) Actually I'd prefer something along the line of getChildNodeInfos() to be more explicit and to clearly differentiate it from getNodeInfo(). Michael BR, Julian
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
Michael Dürig wrote: I missed something... the nodeId's are not available to jcr2spi (that is what getChildInfos was/is for). So +1 for Marcel's original suggestion for adding RepositoryService.getChildInfos(). ...the one for which Marcel already agreed it doesn't help for the problem we are trying to solve :-)? No, I dont think so. I was referring to Marcel saying: hmmm, what's the exact reason we have ChildInfo? what's the downside of getting rid of ChildInfo and use NodeInfo instead? i.e. RepositoryService.getChildInfos() returns NodeInfos instead of ChildInfos. Oh, sorry for misunderstanding. ...we may want to rename it to getNodeInfos() then :-) BR, Julian
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
I missed something... the nodeId's are not available to jcr2spi (that is what getChildInfos was/is for). So +1 for Marcel's original suggestion for adding RepositoryService.getChildInfos(). ...the one for which Marcel already agreed it doesn't help for the problem we are trying to solve :-)? No, I dont think so. I was referring to Marcel saying: hmmm, what's the exact reason we have ChildInfo? what's the downside of getting rid of ChildInfo and use NodeInfo instead? i.e. RepositoryService.getChildInfos() returns NodeInfos instead of ChildInfos. Michael
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
Michael Dürig wrote: RepositoryService.getNodeInfos(SessionInfo sessionInfo, NodeId[] nodeId) returning an Iterator of NodeInfo. Or am I missing something here? +1 for getting many NodeInfos in one call. However -- is your proposal for the calls passed in as parameter, or for their child nodes? I missed something... the nodeId's are not available to jcr2spi (that is what getChildInfos was/is for). So +1 for Marcel's original suggestion for adding RepositoryService.getChildInfos(). ...the one for which Marcel already agreed it doesn't help for the problem we are trying to solve :-)? As far as I can tell, we have a problem of having too many indirections. In this case, it causes an extreme overhead in additional roundtrips. This is what we need to solve. IMHO the obvious way to do that would be to allow returning NodeInfos instead of ChildInfos. I don't case particulary how -- allowing the iterator to contain both, or let NodeInfo extend ChildInfo. BR, Julian
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
RepositoryService.getNodeInfos(SessionInfo sessionInfo, NodeId[] nodeId) returning an Iterator of NodeInfo. Or am I missing something here? +1 for getting many NodeInfos in one call. However -- is your proposal for the calls passed in as parameter, or for their child nodes? I missed something... the nodeId's are not available to jcr2spi (that is what getChildInfos was/is for). So +1 for Marcel's original suggestion for adding RepositoryService.getChildInfos(). Michael
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
Michael Dürig wrote: i.e. RepositoryService.getChildInfos() returns NodeInfos instead of ChildInfos. I'd favour such a solution. We might even want to further generalize it to RepositoryService.getNodeInfos(SessionInfo sessionInfo, NodeId[] nodeId) returning an Iterator of NodeInfo. Or am I missing something here? +1 for getting many NodeInfos in one call. However -- is your proposal for the calls passed in as parameter, or for their child nodes? BR, Julian
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
i.e. RepositoryService.getChildInfos() returns NodeInfos instead of ChildInfos. I'd favour such a solution. We might even want to further generalize it to RepositoryService.getNodeInfos(SessionInfo sessionInfo, NodeId[] nodeId) returning an Iterator of NodeInfo. Or am I missing something here? Michael
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
Julian Reschke wrote: Returning ItemInfos in a batch is not going to happen for the children of a large collection. After all, the SPI impl has no idea whether the caller wants to see them. You have a point there. I think with NodeInfo.getChildInfos() we should be able to efficiently traverse nodes with reasonable sized child node entries, assuming an implementation returns child infos and adds the child node infos to a batch read. But I agree with you that the use case with a large collection is not solved. hmmm, what's the exact reason we have ChildInfo? what's the downside of getting rid of ChildInfo and use NodeInfo instead? i.e. RepositoryService.getChildInfos() returns NodeInfos instead of ChildInfos. regards marcel
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
Michael Dürig wrote: I think the idea of adding getChildInfos to NodeInfo was to cut down on individual calls to the SPI. In a call to getItemInfos an implementation may choose to return any number of ItemInfos in one single batch. By The problem, as far as I understand, is that the saving is in the wrong place; at least for the use case I am trying to optimize and which started this thread. For enumerating the children of a collection, the call removes a single NodeId->NodeInfo resolution for the container. The overhead for iterating through the children (resolving ChildInfos zo NodeInfos) is unchanged. Or am I missing something? making the ChildInfos directly available for those nodes having its child nodes in the batch, no further calls to the SPI are needed. Returning ItemInfos in a batch is not going to happen for the children of a large collection. After all, the SPI impl has no idea whether the caller wants to see them. Furthermore by doing so an SPI implementation can decide on how much information it want to read from a back-end store in a single sweep and delegate caching to the upper jcr2spi layer. That's all understood and nice, but it doesn't help *at all* for the problem I'm trying to solve. BR, Julian
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
I think the idea of adding getChildInfos to NodeInfo was to cut down on individual calls to the SPI. In a call to getItemInfos an implementation may choose to return any number of ItemInfos in one single batch. By making the ChildInfos directly available for those nodes having its child nodes in the batch, no further calls to the SPI are needed. Furthermore by doing so an SPI implementation can decide on how much information it want to read from a back-end store in a single sweep and delegate caching to the upper jcr2spi layer. Michael Julian Reschke wrote: Julian Reschke wrote: ... Going back to the original discussion: what I think we need is a way to return NodeInfos, not ChildInfos. Right now, when browsing a collection of 1000 items, getting information about each node (timestamps, dates, ...) requires 1000 individual calls through SPI. This is insane, in particular if the implementation is not allowed to cache anything (which I do not agree at all with). BR, Julian
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
Julian Reschke wrote: ... Going back to the original discussion: what I think we need is a way to return NodeInfos, not ChildInfos. Right now, when browsing a collection of 1000 items, getting information about each node (timestamps, dates, ...) requires 1000 individual calls through SPI. This is insane, in particular if the implementation is not allowed to cache anything (which I do not agree at all with). BR, Julian
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
One of the reasons I really don't like Iterators in SPI is because they make it hard to perform logging. I have a dynamic proxy that performs trace logging for SPI calls. So in my SPI's I use Apache ResettableIterator's wherever I can, which my trace logger knows about. For any other Iterator though, I can't log what's going on, without being sure I'm not impacting behavior. David On Fri, 2008-02-22 at 11:25 -0800, Marcel Reutegger (JIRA) wrote: > [ > https://issues.apache.org/jira/browse/JCR-1405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12571544#action_12571544 > ] > > Marcel Reutegger commented on JCR-1405: > --- > > Julian wrote: > > So what am I supposed to do when internally an error occurs? > > I think you should not even create a NodeInfo in that case. IMO a NodeInfo > should be self contained and shouldn't hold resources tied to the underlying > repository. > > That's actually also a reason why I don't like changing the return values to > an Iterator ;) > Whenever Iterators are returned it implies that an implementation is allowed > (or even encouraged) to lazily retrieve the items. This lazy approach however > has some issues. See also JSR 283 mailing list thread 'I whish there was > '. > > This new method is not intended to return large amounts of ChildInfos that > are lazily instanciated and should therefore use an array. > > > SPI: Introduce NodeInfo.getChildInfos() > > --- > > > > Key: JCR-1405 > > URL: https://issues.apache.org/jira/browse/JCR-1405 > > Project: Jackrabbit > > Issue Type: New Feature > > Components: jackrabbit-spi > >Reporter: angela > > Attachments: JCR-1405.patch > > > > > > Improvement suggested by Marcel: > > ChildInfo is basically a stripped down NodeInfo. With little effort it > > would even be possible to have NodeInfo extends ChildInfo. Not sure how > > useful that is, but since we don't have that inheritance in code and at the > > same time nearly a 100% overlap it makes me suspicious. > > Here's another idea: > > introduce a method ChildInfo[] NodeInfo.getChildInfos(). The method either > > returns: > > - all child infos, which also gives the correct number of child nodes. this > > may also mean that an empty array is returned to indicate there are no > > child nodes. > > - null, to indicate that there are *lots* of child nodes and the method > > RepositoryService.getChildInfos() with the iterator should be used. >
Re: [jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
...continuing on the mailing list; I think this exceeds what an issue tracker is good for. angela (JIRA) wrote: angela commented on JCR-1405: - julian: if you cant determine the childinfos upon creating the nodeinfo you should (as stated by the javadoc) simply return null, Trying to determine the child infos in practice means asking the underlying storage for them. If there are 1000 children, and I get an internal error for the last, I would then have to return null. Which means that JCR2SPI asks again, using RepositoryService. Not good. The only case where this new method would actually help is where the set of child node names is known in advance, such as for nodes of type nt:file. It's nice to be able to optimize those, but not sufficient. We started the discussion because of the horrific performance of JCR2SPI for large collections (where it currently reaches something around 2% of what my persistence layer can do). Are we still trying to solve this? if you cant build the nodeinfo due to some exceptional situation you should throw upon getNodeInfo or getItemInfos respectively. the exception with repositoryservice getChildInfo means the same as the one defined with getNodeInfo or getItemInfos: - the target node does not exist (any more) in the persistent state - the persistent layer cant be accessed or something similar. Well. If the *construction* of the NodeInfo now requires to decide whether to return child infos or not, then this change doesn't help, because it doesn't scale for large collections. I'm not going to retrieve child information unless somebody asks for it -- and that is when NodeInfo.getChildInfos is called, not when the NodeInfo is constructed. therefore i am with marcels explanation how nodeinfo should be created and work. in addition, if you decide to do some lazy loading of the childinfos upon NodeInfo.getChildInfos (or upon RepositoryService.getChildInfos) the exception from my point of view is not raised upon building the iterator but upon retrieving the next element and there you wont be able to throw repository exception either. ...which may be an indication that a generic Iterator is not the right thing to use either. regarding "large": this is just one obvious example what could be a reason for the implementation NOT to reveal the child infos upon NodeInfo.getChildInfos. and the description mentions this as example. Again; I started this discussion because of the performance for large collections. You seem to try to solve an entirely different problem -- do we have any data that indicates that it's worth solving? How exactly is it better than batch read? that it states: if the impl is not willing. Not willing means that the SPI implementations decides upon internal rules whether the childinfos are included or not. examples: the impl. decides - based on the internal structure of the persistent layer in general - based the cost of retrieving childinfos (given the potential chance of never being asked for) See -- that's the problem. It seems to me what we really need is a way to indicate that the children *will* be needed. - based on the known characteristics of the target node: e.g. we have folder and files and other nodes and we assume that folders will be used for displaying the children so send it. for any other nodes we dont See above -- doesn't work in practice. - based on the simple amount of child nodes if we know that (dont calc if more than 14) - based on a implementation specific configuration that could include nodetypes, number of child nodes, day time, session.userId, random... whatever you feel would be appropriate, reasonable or simply a good thing for your specific store. the last is pretty much what we discussed for the getItemInfos method for the batch read. we said that we cant add a config to the spi interfaces and want to leave that to the impl because we would not be able to find something that fits the needs for all potential implementations. I do agree that the SPI impl needs to decide on things like that. But we have to give it sufficient information. if your store cant retrieve the child info you may - create your reposervice with a config and leave the decision to someone else - always calculate the child infos Again, that doesn't work for the use case we're trying to solve. Or at least the one I thought we're trying to solve. - never calculate the child infos - decide based on the characteristics of the requested node -... (see above) so. i am not in favor of adding exceptions to the new method... at least not for the reasons presented so far. angela I'm in favor to first clearly state what we're trying to do; then create tests for obtaining measurements; and then re-discuss what needs to be done. BR, Julian
[jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
[ https://issues.apache.org/jira/browse/JCR-1405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12571713#action_12571713 ] angela commented on JCR-1405: - julian: if you cant determine the childinfos upon creating the nodeinfo you should (as stated by the javadoc) simply return null, if you cant build the nodeinfo due to some exceptional situation you should throw upon getNodeInfo or getItemInfos respectively. the exception with repositoryservice getChildInfo means the same as the one defined with getNodeInfo or getItemInfos: - the target node does not exist (any more) in the persistent state - the persistent layer cant be accessed or something similar. therefore i am with marcels explanation how nodeinfo should be created and work. in addition, if you decide to do some lazy loading of the childinfos upon NodeInfo.getChildInfos (or upon RepositoryService.getChildInfos) the exception from my point of view is not raised upon building the iterator but upon retrieving the next element and there you wont be able to throw repository exception either. regarding "large": this is just one obvious example what could be a reason for the implementation NOT to reveal the child infos upon NodeInfo.getChildInfos. and the description mentions this as example. that it states: if the impl is not willing. Not willing means that the SPI implementations decides upon internal rules whether the childinfos are included or not. examples: the impl. decides - based on the internal structure of the persistent layer in general - based the cost of retrieving childinfos (given the potential chance of never being asked for) - based on the known characteristics of the target node: e.g. we have folder and files and other nodes and we assume that folders will be used for displaying the children so send it. for any other nodes we dont - based on the simple amount of child nodes if we know that (dont calc if more than 14) - based on a implementation specific configuration that could include nodetypes, number of child nodes, day time, session.userId, random... whatever you feel would be appropriate, reasonable or simply a good thing for your specific store. the last is pretty much what we discussed for the getItemInfos method for the batch read. we said that we cant add a config to the spi interfaces and want to leave that to the impl because we would not be able to find something that fits the needs for all potential implementations. if your store cant retrieve the child info you may - create your reposervice with a config and leave the decision to someone else - always calculate the child infos - never calculate the child infos - decide based on the characteristics of the requested node -... (see above) so. i am not in favor of adding exceptions to the new method... at least not for the reasons presented so far. angela > SPI: Introduce NodeInfo.getChildInfos() > --- > > Key: JCR-1405 > URL: https://issues.apache.org/jira/browse/JCR-1405 > Project: Jackrabbit > Issue Type: New Feature > Components: jackrabbit-spi >Reporter: angela > Attachments: JCR-1405.patch > > > Improvement suggested by Marcel: > ChildInfo is basically a stripped down NodeInfo. With little effort it would > even be possible to have NodeInfo extends ChildInfo. Not sure how useful that > is, but since we don't have that inheritance in code and at the same time > nearly a 100% overlap it makes me suspicious. > Here's another idea: > introduce a method ChildInfo[] NodeInfo.getChildInfos(). The method either > returns: > - all child infos, which also gives the correct number of child nodes. this > may also mean that an empty array is returned to indicate there are no child > nodes. > - null, to indicate that there are *lots* of child nodes and the method > RepositoryService.getChildInfos() with the iterator should be used. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
[ https://issues.apache.org/jira/browse/JCR-1405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12571581#action_12571581 ] Julian Reschke commented on JCR-1405: - > I think you should not even create a NodeInfo in that case. IMO a NodeInfo > should be self contained and shouldn't hold resources tied to the underlying > repository. I disagree with that. But anyway, even if it did not an error could occur while building the list of children. I'm really not sure why we're pretending that things always will work when they may not. There may be other problems like these (where I currently have to throw unchecked exceptions), will have to check for that. > That's actually also a reason why I don't like changing the return values to > an Iterator ;) > Whenever Iterators are returned it implies that an implementation is allowed > (or even encouraged) to lazily retrieve the items. This lazy approach however > has some issues. See also JSR 283 mailing list thread 'I whish there was > '. I think it's a feature (to allow lazy construction); we just need to make sure we handle all cases well. > This new method is not intended to return large amounts of ChildInfos that > are lazily instanciated and should therefore use an array. Sounds kind of arbitrary to me. What is "large"? And, if my store doesn't know beforehand (which one does?), do we really want the SPI impl to return null, just for JCR2PI to asking for the same information again through a different method call? > SPI: Introduce NodeInfo.getChildInfos() > --- > > Key: JCR-1405 > URL: https://issues.apache.org/jira/browse/JCR-1405 > Project: Jackrabbit > Issue Type: New Feature > Components: jackrabbit-spi >Reporter: angela > Attachments: JCR-1405.patch > > > Improvement suggested by Marcel: > ChildInfo is basically a stripped down NodeInfo. With little effort it would > even be possible to have NodeInfo extends ChildInfo. Not sure how useful that > is, but since we don't have that inheritance in code and at the same time > nearly a 100% overlap it makes me suspicious. > Here's another idea: > introduce a method ChildInfo[] NodeInfo.getChildInfos(). The method either > returns: > - all child infos, which also gives the correct number of child nodes. this > may also mean that an empty array is returned to indicate there are no child > nodes. > - null, to indicate that there are *lots* of child nodes and the method > RepositoryService.getChildInfos() with the iterator should be used. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
[ https://issues.apache.org/jira/browse/JCR-1405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12571544#action_12571544 ] Marcel Reutegger commented on JCR-1405: --- Julian wrote: > So what am I supposed to do when internally an error occurs? I think you should not even create a NodeInfo in that case. IMO a NodeInfo should be self contained and shouldn't hold resources tied to the underlying repository. That's actually also a reason why I don't like changing the return values to an Iterator ;) Whenever Iterators are returned it implies that an implementation is allowed (or even encouraged) to lazily retrieve the items. This lazy approach however has some issues. See also JSR 283 mailing list thread 'I whish there was '. This new method is not intended to return large amounts of ChildInfos that are lazily instanciated and should therefore use an array. > SPI: Introduce NodeInfo.getChildInfos() > --- > > Key: JCR-1405 > URL: https://issues.apache.org/jira/browse/JCR-1405 > Project: Jackrabbit > Issue Type: New Feature > Components: jackrabbit-spi >Reporter: angela > Attachments: JCR-1405.patch > > > Improvement suggested by Marcel: > ChildInfo is basically a stripped down NodeInfo. With little effort it would > even be possible to have NodeInfo extends ChildInfo. Not sure how useful that > is, but since we don't have that inheritance in code and at the same time > nearly a 100% overlap it makes me suspicious. > Here's another idea: > introduce a method ChildInfo[] NodeInfo.getChildInfos(). The method either > returns: > - all child infos, which also gives the correct number of child nodes. this > may also mean that an empty array is returned to indicate there are no child > nodes. > - null, to indicate that there are *lots* of child nodes and the method > RepositoryService.getChildInfos() with the iterator should be used. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
[ https://issues.apache.org/jira/browse/JCR-1405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12571431#action_12571431 ] Julian Reschke commented on JCR-1405: - ...which I think is a problem as well. In this case, the related method on RepositoryService *does* declare RepositoryException. So what am I supposed to do when internally an error occurs? > SPI: Introduce NodeInfo.getChildInfos() > --- > > Key: JCR-1405 > URL: https://issues.apache.org/jira/browse/JCR-1405 > Project: Jackrabbit > Issue Type: New Feature > Components: jackrabbit-spi >Reporter: angela > Attachments: JCR-1405.patch > > > Improvement suggested by Marcel: > ChildInfo is basically a stripped down NodeInfo. With little effort it would > even be possible to have NodeInfo extends ChildInfo. Not sure how useful that > is, but since we don't have that inheritance in code and at the same time > nearly a 100% overlap it makes me suspicious. > Here's another idea: > introduce a method ChildInfo[] NodeInfo.getChildInfos(). The method either > returns: > - all child infos, which also gives the correct number of child nodes. this > may also mean that an empty array is returned to indicate there are no child > nodes. > - null, to indicate that there are *lots* of child nodes and the method > RepositoryService.getChildInfos() with the iterator should be used. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
[ https://issues.apache.org/jira/browse/JCR-1405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12571430#action_12571430 ] angela commented on JCR-1405: - none of the other methods allows exceptions... > SPI: Introduce NodeInfo.getChildInfos() > --- > > Key: JCR-1405 > URL: https://issues.apache.org/jira/browse/JCR-1405 > Project: Jackrabbit > Issue Type: New Feature > Components: jackrabbit-spi >Reporter: angela > Attachments: JCR-1405.patch > > > Improvement suggested by Marcel: > ChildInfo is basically a stripped down NodeInfo. With little effort it would > even be possible to have NodeInfo extends ChildInfo. Not sure how useful that > is, but since we don't have that inheritance in code and at the same time > nearly a 100% overlap it makes me suspicious. > Here's another idea: > introduce a method ChildInfo[] NodeInfo.getChildInfos(). The method either > returns: > - all child infos, which also gives the correct number of child nodes. this > may also mean that an empty array is returned to indicate there are no child > nodes. > - null, to indicate that there are *lots* of child nodes and the method > RepositoryService.getChildInfos() with the iterator should be used. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
[ https://issues.apache.org/jira/browse/JCR-1405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12571428#action_12571428 ] Julian Reschke commented on JCR-1405: - Hm, no exceptions allowed? > SPI: Introduce NodeInfo.getChildInfos() > --- > > Key: JCR-1405 > URL: https://issues.apache.org/jira/browse/JCR-1405 > Project: Jackrabbit > Issue Type: New Feature > Components: jackrabbit-spi >Reporter: angela > Attachments: JCR-1405.patch > > > Improvement suggested by Marcel: > ChildInfo is basically a stripped down NodeInfo. With little effort it would > even be possible to have NodeInfo extends ChildInfo. Not sure how useful that > is, but since we don't have that inheritance in code and at the same time > nearly a 100% overlap it makes me suspicious. > Here's another idea: > introduce a method ChildInfo[] NodeInfo.getChildInfos(). The method either > returns: > - all child infos, which also gives the correct number of child nodes. this > may also mean that an empty array is returned to indicate there are no child > nodes. > - null, to indicate that there are *lots* of child nodes and the method > RepositoryService.getChildInfos() with the iterator should be used. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
[ https://issues.apache.org/jira/browse/JCR-1405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12571388#action_12571388 ] Michael Dürig commented on JCR-1405: +1 for using an iterator +1 for the patch. There is a typo in the comment: "Return the all ChildInfo". > SPI: Introduce NodeInfo.getChildInfos() > --- > > Key: JCR-1405 > URL: https://issues.apache.org/jira/browse/JCR-1405 > Project: Jackrabbit > Issue Type: New Feature > Components: jackrabbit-spi >Reporter: angela > Attachments: JCR-1405.patch > > > Improvement suggested by Marcel: > ChildInfo is basically a stripped down NodeInfo. With little effort it would > even be possible to have NodeInfo extends ChildInfo. Not sure how useful that > is, but since we don't have that inheritance in code and at the same time > nearly a 100% overlap it makes me suspicious. > Here's another idea: > introduce a method ChildInfo[] NodeInfo.getChildInfos(). The method either > returns: > - all child infos, which also gives the correct number of child nodes. this > may also mean that an empty array is returned to indicate there are no child > nodes. > - null, to indicate that there are *lots* of child nodes and the method > RepositoryService.getChildInfos() with the iterator should be used. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
[ https://issues.apache.org/jira/browse/JCR-1405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12571048#action_12571048 ] angela commented on JCR-1405: - > I'm in favor of Angela's suggestion credit is due to marcel. and i would support the more general interpretation of the null return value suggested by michael. > SPI: Introduce NodeInfo.getChildInfos() > --- > > Key: JCR-1405 > URL: https://issues.apache.org/jira/browse/JCR-1405 > Project: Jackrabbit > Issue Type: New Feature > Components: jackrabbit-spi >Reporter: angela > > Improvement suggested by Marcel: > ChildInfo is basically a stripped down NodeInfo. With little effort it would > even be possible to have NodeInfo extends ChildInfo. Not sure how useful that > is, but since we don't have that inheritance in code and at the same time > nearly a 100% overlap it makes me suspicious. > Here's another idea: > introduce a method ChildInfo[] NodeInfo.getChildInfos(). The method either > returns: > - all child infos, which also gives the correct number of child nodes. this > may also mean that an empty array is returned to indicate there are no child > nodes. > - null, to indicate that there are *lots* of child nodes and the method > RepositoryService.getChildInfos() with the iterator should be used. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (JCR-1405) SPI: Introduce NodeInfo.getChildInfos()
[ https://issues.apache.org/jira/browse/JCR-1405?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12571041#action_12571041 ] Michael Dürig commented on JCR-1405: I'm in favor of Angela's suggestion. I would generalize the meaning of null returned by ChildInfo[] NodeInfo.getChildInfos() to 'ask me later'. That is, whenever ChildInfo[] NodeInfo.getChildInfos() returns null, the spi implementation cannot or for some reason deliberately does not want to return the ChildInfos. The client would then have to collect the ChildInfos later by a call to RepositoryService.getChildInfos. Also I think the client should use the ChildInfos returned by NodeInfo.getChildInfos() whenever possible. It should only call RepositoryService.getChildInfos when absolutely necessary. > SPI: Introduce NodeInfo.getChildInfos() > --- > > Key: JCR-1405 > URL: https://issues.apache.org/jira/browse/JCR-1405 > Project: Jackrabbit > Issue Type: New Feature > Components: jackrabbit-spi >Reporter: angela > > Improvement suggested by Marcel: > ChildInfo is basically a stripped down NodeInfo. With little effort it would > even be possible to have NodeInfo extends ChildInfo. Not sure how useful that > is, but since we don't have that inheritance in code and at the same time > nearly a 100% overlap it makes me suspicious. > Here's another idea: > introduce a method ChildInfo[] NodeInfo.getChildInfos(). The method either > returns: > - all child infos, which also gives the correct number of child nodes. this > may also mean that an empty array is returned to indicate there are no child > nodes. > - null, to indicate that there are *lots* of child nodes and the method > RepositoryService.getChildInfos() with the iterator should be used. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
