Re: [core] performance: use indices instead of iterator (MYFACES-3130)
If getChildren() is always of type List, then it really doesn't matter if it's ArrayList or ChildArrayList or some other kind of list. You can use indexes for any type of List. On Tue, May 10, 2011 at 4:11 PM, Martin Koci martin.kocicak.k...@gmail.com wrote: Hi, in current codebase, myfaces use mostly enhanced loop for iterating over chidren: for (UIComponent child: getChildren()) that creates new instance of iterator. After change to plain old indices: for (i = 0; i childCount; i++) child = getChildren().get(i); I achieved following results in my test case: 1) number of AbstractList$Itr from ~ 100 000 down to ~1500 instances per one render/response 2) time for one render/response from ~1500ms down to ~900ms please see https://issues.apache.org/jira/browse/MYFACES-3130 for more details. We know that getChildren() is type of ArrayList (javax.faces.component._ComponentChildrenList) - in this situation is index-based loop safe change. But custom component can override implementation of getChildren() and provide own implementation which can be slower: I know about Trinidad: uses ArrayList too, so no risk here (org.apache.myfaces.trinidad.component.ChildArrayList) I use RichFaces and PrimeFaces too and don't see custom implementation of children in their codebase. What do you think about this problem? The performance gain is pretty big but also risky. Regards, Kočičák
Re: [core] performance: use indices instead of iterator (MYFACES-3130)
Mike, What Martin is talking about is that if the List doesn't implement the Marker interface RandomAccess then the List may implement indexed-based access through iteration, in which case iterating the list is n^2/2 -- Blake Sullivan On 5/10/11 1:17 PM, Mike Kienenberger wrote: If getChildren() is always of type List, then it really doesn't matter if it's ArrayList or ChildArrayList or some other kind of list. You can use indexes for any type of List. On Tue, May 10, 2011 at 4:11 PM, Martin Koci martin.kocicak.k...@gmail.com wrote: Hi, in current codebase, myfaces use mostly enhanced loop for iterating over chidren: for (UIComponent child: getChildren()) that creates new instance of iterator. After change to plain old indices: for (i = 0; i childCount; i++) child = getChildren().get(i); I achieved following results in my test case: 1) number of AbstractList$Itr from ~ 100 000 down to ~1500 instances per one render/response 2) time for one render/response from ~1500ms down to ~900ms please see https://issues.apache.org/jira/browse/MYFACES-3130 for more details. We know that getChildren() is type of ArrayList (javax.faces.component._ComponentChildrenList) - in this situation is index-based loop safe change. But custom component can override implementation of getChildren() and provide own implementation which can be slower: I know about Trinidad: uses ArrayList too, so no risk here (org.apache.myfaces.trinidad.component.ChildArrayList) I use RichFaces and PrimeFaces too and don't see custom implementation of children in their codebase. What do you think about this problem? The performance gain is pretty big but also risky. Regards, Kočičák
Re: [core] performance: use indices instead of iterator (MYFACES-3130)
Hi +1 to change to plain old indices. One curious thing to note is Trinidad variant uses plain old indices to do its own stuff. See: http://svn.apache.org/repos/asf/myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/ChildArrayList.java Trinidad stuff is rock solid, well tested and with a lot of performance hacks. So I think use plain old indices for all critical iteration code is our best bet. regards, Leonardo Uribe 2011/5/10 Blake Sullivan blake.sulli...@oracle.com: Mike, What Martin is talking about is that if the List doesn't implement the Marker interface RandomAccess then the List may implement indexed-based access through iteration, in which case iterating the list is n^2/2 -- Blake Sullivan On 5/10/11 1:17 PM, Mike Kienenberger wrote: If getChildren() is always of type List, then it really doesn't matter if it's ArrayList or ChildArrayList or some other kind of list. You can use indexes for any type of List. On Tue, May 10, 2011 at 4:11 PM, Martin Koci martin.kocicak.k...@gmail.com wrote: Hi, in current codebase, myfaces use mostly enhanced loop for iterating over chidren: for (UIComponent child: getChildren()) that creates new instance of iterator. After change to plain old indices: for (i = 0; i childCount; i++) child = getChildren().get(i); I achieved following results in my test case: 1) number of AbstractList$Itr from ~ 100 000 down to ~1500 instances per one render/response 2) time for one render/response from ~1500ms down to ~900ms please see https://issues.apache.org/jira/browse/MYFACES-3130 for more details. We know that getChildren() is type of ArrayList (javax.faces.component._ComponentChildrenList) - in this situation is index-based loop safe change. But custom component can override implementation of getChildren() and provide own implementation which can be slower: I know about Trinidad: uses ArrayList too, so no risk here (org.apache.myfaces.trinidad.component.ChildArrayList) I use RichFaces and PrimeFaces too and don't see custom implementation of children in their codebase. What do you think about this problem? The performance gain is pretty big but also risky. Regards, Kočičák
Re: [core] performance: use indices instead of iterator (MYFACES-3130)
Hi, yes, every List support indexes, but it dependes on implementation if that index-based access is fast or not. For example, ArrayList is fast, because it uses array internally; and also flags that with interface java.util.RandomAccess But LinkedList for example just iterates the list until it reaches the index you specified - there is the dangerous problem. Mike Kienenberger píše v Út 10. 05. 2011 v 16:17 -0400: If getChildren() is always of type List, then it really doesn't matter if it's ArrayList or ChildArrayList or some other kind of list. You can use indexes for any type of List. On Tue, May 10, 2011 at 4:11 PM, Martin Koci martin.kocicak.k...@gmail.com wrote: Hi, in current codebase, myfaces use mostly enhanced loop for iterating over chidren: for (UIComponent child: getChildren()) that creates new instance of iterator. After change to plain old indices: for (i = 0; i childCount; i++) child = getChildren().get(i); I achieved following results in my test case: 1) number of AbstractList$Itr from ~ 100 000 down to ~1500 instances per one render/response 2) time for one render/response from ~1500ms down to ~900ms please see https://issues.apache.org/jira/browse/MYFACES-3130 for more details. We know that getChildren() is type of ArrayList (javax.faces.component._ComponentChildrenList) - in this situation is index-based loop safe change. But custom component can override implementation of getChildren() and provide own implementation which can be slower: I know about Trinidad: uses ArrayList too, so no risk here (org.apache.myfaces.trinidad.component.ChildArrayList) I use RichFaces and PrimeFaces too and don't see custom implementation of children in their codebase. What do you think about this problem? The performance gain is pretty big but also risky. Regards, Kočičák
Re: [core] performance: use indices instead of iterator (MYFACES-3130)
It's not dangerous or risky. It's just slower. It won't break anything. The common case is that this change will have better performance in all known situations. I haven't looked at the code, but what about creating a static Iterator instead of creating a new one each time? Or picking whether to use iterator() or for based on the list type? On Tue, May 10, 2011 at 4:29 PM, Martin Koci martin.kocicak.k...@gmail.com wrote: Hi, yes, every List support indexes, but it dependes on implementation if that index-based access is fast or not. For example, ArrayList is fast, because it uses array internally; and also flags that with interface java.util.RandomAccess But LinkedList for example just iterates the list until it reaches the index you specified - there is the dangerous problem. Mike Kienenberger píše v Út 10. 05. 2011 v 16:17 -0400: If getChildren() is always of type List, then it really doesn't matter if it's ArrayList or ChildArrayList or some other kind of list. You can use indexes for any type of List. On Tue, May 10, 2011 at 4:11 PM, Martin Koci martin.kocicak.k...@gmail.com wrote: Hi, in current codebase, myfaces use mostly enhanced loop for iterating over chidren: for (UIComponent child: getChildren()) that creates new instance of iterator. After change to plain old indices: for (i = 0; i childCount; i++) child = getChildren().get(i); I achieved following results in my test case: 1) number of AbstractList$Itr from ~ 100 000 down to ~1500 instances per one render/response 2) time for one render/response from ~1500ms down to ~900ms please see https://issues.apache.org/jira/browse/MYFACES-3130 for more details. We know that getChildren() is type of ArrayList (javax.faces.component._ComponentChildrenList) - in this situation is index-based loop safe change. But custom component can override implementation of getChildren() and provide own implementation which can be slower: I know about Trinidad: uses ArrayList too, so no risk here (org.apache.myfaces.trinidad.component.ChildArrayList) I use RichFaces and PrimeFaces too and don't see custom implementation of children in their codebase. What do you think about this problem? The performance gain is pretty big but also risky. Regards, Kočičák
