You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by "Martin Kočí (JIRA)" <de...@myfaces.apache.org> on 2011/05/06 19:43:04 UTC
[jira] [Created] (MYFACES-3130) [PERF] Avoid unnecessary
AbstractList$Itr instances
[PERF] Avoid unnecessary AbstractList$Itr instances
---------------------------------------------------
Key: MYFACES-3130
URL: https://issues.apache.org/jira/browse/MYFACES-3130
Project: MyFaces Core
Issue Type: Improvement
Components: JSR-314
Environment: myfaces core trunk
Reporter: Martin Kočí
Priority: Minor
Similar issue: MYFACES-3129
loop from java 5:
for (Object object: objects)
creates new instance of AbstractList$Itr, if objects are instance of ArrayList.
Similar situation is with explicit .iterator() call.
In my testcases, it is about ~ 100 000 instances per request/response. Creation itself is cheap, but triggers GC lately.
I suggest to use old index-style for (i = 0; i < childCount; i++) if possible. Are there any rawbacks of index-based iteration?
Children is List and as implementation detail we know that it is instance of ArrayList.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
Re: [jira] [Commented] (MYFACES-3130) [PERF] Avoid unnecessary
AbstractList$Itr instances
Posted by Jan-Kees van Andel <ja...@gmail.com>.
+1!
I thought RandomAccess was implemented by way more classes, but looking at
the list of implementors, I guess checking for RandomAccess is the best
choice.
2011/5/14 Jakob Korherr (JIRA) <de...@myfaces.apache.org>
>
> [
> https://issues.apache.org/jira/browse/MYFACES-3130?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13033496#comment-13033496]
>
> Jakob Korherr commented on MYFACES-3130:
> ----------------------------------------
>
> +1 for that Jan-Kees.
>
> But I would check for (instanceof RandomAccess) rather than ArrayList (see
> [1]).
>
> [1]
> http://download.oracle.com/javase/1,5.0/docs/api/java/util/RandomAccess.html
>
> > [PERF] Avoid unnecessary AbstractList$Itr instances
> > ---------------------------------------------------
> >
> > Key: MYFACES-3130
> > URL: https://issues.apache.org/jira/browse/MYFACES-3130
> > Project: MyFaces Core
> > Issue Type: Improvement
> > Components: JSR-314
> > Environment: myfaces core trunk
> > Reporter: Martin Kočí
> > Attachments: MYFACES-3130-example.patch
> >
> >
> > Similar issue: MYFACES-3129
> > loop from java 5:
> > for (Object object: objects)
> > creates new instance of AbstractList$Itr, if objects are instance of
> ArrayList.
> > Similar situation is with explicit .iterator() call.
> > In my testcases, it is about ~ 100 000 instances per request/response.
> Creation itself is cheap, but triggers GC lately.
> > I suggest to use old index-style for (i = 0; i < childCount; i++) if
> possible. Are there any rawbacks of index-based iteration?
> > Children is List and as implementation detail we know that it is
> instance of ArrayList.
>
> --
> This message is automatically generated by JIRA.
> For more information on JIRA, see: http://www.atlassian.com/software/jira
>
[jira] [Commented] (MYFACES-3130) [PERF] Avoid unnecessary
AbstractList$Itr instances
Posted by "Martin Kočí (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-3130?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13032340#comment-13032340 ]
Martin Kočí commented on MYFACES-3130:
--------------------------------------
If no objections, I'll commit the patch soon!
> [PERF] Avoid unnecessary AbstractList$Itr instances
> ---------------------------------------------------
>
> Key: MYFACES-3130
> URL: https://issues.apache.org/jira/browse/MYFACES-3130
> Project: MyFaces Core
> Issue Type: Improvement
> Components: JSR-314
> Environment: myfaces core trunk
> Reporter: Martin Kočí
> Attachments: MYFACES-3130-example.patch
>
>
> Similar issue: MYFACES-3129
> loop from java 5:
> for (Object object: objects)
> creates new instance of AbstractList$Itr, if objects are instance of ArrayList.
> Similar situation is with explicit .iterator() call.
> In my testcases, it is about ~ 100 000 instances per request/response. Creation itself is cheap, but triggers GC lately.
> I suggest to use old index-style for (i = 0; i < childCount; i++) if possible. Are there any rawbacks of index-based iteration?
> Children is List and as implementation detail we know that it is instance of ArrayList.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MYFACES-3130) [PERF] Avoid unnecessary
AbstractList$Itr instances
Posted by "Jakob Korherr (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-3130?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13033496#comment-13033496 ]
Jakob Korherr commented on MYFACES-3130:
----------------------------------------
+1 for that Jan-Kees.
But I would check for (instanceof RandomAccess) rather than ArrayList (see [1]).
[1] http://download.oracle.com/javase/1,5.0/docs/api/java/util/RandomAccess.html
> [PERF] Avoid unnecessary AbstractList$Itr instances
> ---------------------------------------------------
>
> Key: MYFACES-3130
> URL: https://issues.apache.org/jira/browse/MYFACES-3130
> Project: MyFaces Core
> Issue Type: Improvement
> Components: JSR-314
> Environment: myfaces core trunk
> Reporter: Martin Kočí
> Attachments: MYFACES-3130-example.patch
>
>
> Similar issue: MYFACES-3129
> loop from java 5:
> for (Object object: objects)
> creates new instance of AbstractList$Itr, if objects are instance of ArrayList.
> Similar situation is with explicit .iterator() call.
> In my testcases, it is about ~ 100 000 instances per request/response. Creation itself is cheap, but triggers GC lately.
> I suggest to use old index-style for (i = 0; i < childCount; i++) if possible. Are there any rawbacks of index-based iteration?
> Children is List and as implementation detail we know that it is instance of ArrayList.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MYFACES-3130) [PERF] Avoid unnecessary
AbstractList$Itr instances
Posted by "Martin Kočí (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-3130?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13034178#comment-13034178 ]
Martin Kočí commented on MYFACES-3130:
--------------------------------------
Patch commited in trunk 2.1. Now I'm going to implement it for rest of code (mostly visitTree method and some internals like ApplicationImpl._handleListenerForAnnotations). I minimize what profiler shows so if your work on some code where ArrayList is 100% sure (like ApplicationImpl._handleListenerForAnnotations) please change it to index-for;
RandomAccess: (a instanceof RandomAccess) check in production code is unwanted, but we can detect custom implementation of Component.children in initializer code, try to initialize new component, put one child in it and test, if getChildren() returns RandomAccess. I've check RichFaces and Primefaces and they don't use own children list. Trinidad uses ArrayList.
Our _ComponentChildrenList does not implement RandomAccess, only delegates on one, I'll change it.
> [PERF] Avoid unnecessary AbstractList$Itr instances
> ---------------------------------------------------
>
> Key: MYFACES-3130
> URL: https://issues.apache.org/jira/browse/MYFACES-3130
> Project: MyFaces Core
> Issue Type: Improvement
> Components: JSR-314
> Environment: myfaces core trunk
> Reporter: Martin Kočí
> Attachments: MYFACES-3130-example.patch
>
>
> Similar issue: MYFACES-3129
> loop from java 5:
> for (Object object: objects)
> creates new instance of AbstractList$Itr, if objects are instance of ArrayList.
> Similar situation is with explicit .iterator() call.
> In my testcases, it is about ~ 100 000 instances per request/response. Creation itself is cheap, but triggers GC lately.
> I suggest to use old index-style for (i = 0; i < childCount; i++) if possible. Are there any rawbacks of index-based iteration?
> Children is List and as implementation detail we know that it is instance of ArrayList.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MYFACES-3130) [PERF] Avoid unnecessary
AbstractList$Itr instances
Posted by "Martin Kočí (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-3130?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13031945#comment-13031945 ]
Martin Kočí commented on MYFACES-3130:
--------------------------------------
mail topic: [core] performance: use indices instead of iterator (MYFACES-3130)
http://www.mail-archive.com/dev@myfaces.apache.org/msg52979.html
> [PERF] Avoid unnecessary AbstractList$Itr instances
> ---------------------------------------------------
>
> Key: MYFACES-3130
> URL: https://issues.apache.org/jira/browse/MYFACES-3130
> Project: MyFaces Core
> Issue Type: Improvement
> Components: JSR-314
> Environment: myfaces core trunk
> Reporter: Martin Kočí
> Attachments: MYFACES-3130-example.patch
>
>
> Similar issue: MYFACES-3129
> loop from java 5:
> for (Object object: objects)
> creates new instance of AbstractList$Itr, if objects are instance of ArrayList.
> Similar situation is with explicit .iterator() call.
> In my testcases, it is about ~ 100 000 instances per request/response. Creation itself is cheap, but triggers GC lately.
> I suggest to use old index-style for (i = 0; i < childCount; i++) if possible. Are there any rawbacks of index-based iteration?
> Children is List and as implementation detail we know that it is instance of ArrayList.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MYFACES-3130) [PERF] Avoid unnecessary
AbstractList$Itr instances
Posted by "Jan-Kees van Andel (JIRA)" <de...@myfaces.apache.org>.
[ https://issues.apache.org/jira/browse/MYFACES-3130?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13032347#comment-13032347 ]
Jan-Kees van Andel commented on MYFACES-3130:
---------------------------------------------
One comment. Since we know some of the collections where the performance gain is substantial, I suggest to put some "instanceof ArrayList" checks in the code and log a warning that an ArrayList is more suitable.
I know it's not strictly necessary, but at least it warns users for bad performance.
About the concurrentmodificationexception, why not just do a list.toArray() and then loop over that array? It might even improve performance (not tested)...
> [PERF] Avoid unnecessary AbstractList$Itr instances
> ---------------------------------------------------
>
> Key: MYFACES-3130
> URL: https://issues.apache.org/jira/browse/MYFACES-3130
> Project: MyFaces Core
> Issue Type: Improvement
> Components: JSR-314
> Environment: myfaces core trunk
> Reporter: Martin Kočí
> Attachments: MYFACES-3130-example.patch
>
>
> Similar issue: MYFACES-3129
> loop from java 5:
> for (Object object: objects)
> creates new instance of AbstractList$Itr, if objects are instance of ArrayList.
> Similar situation is with explicit .iterator() call.
> In my testcases, it is about ~ 100 000 instances per request/response. Creation itself is cheap, but triggers GC lately.
> I suggest to use old index-style for (i = 0; i < childCount; i++) if possible. Are there any rawbacks of index-based iteration?
> Children is List and as implementation detail we know that it is instance of ArrayList.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira