[jira] [Commented] (MYFACES-3435) [perf] _DeltaList: use ArrayList as parent
[ https://issues.apache.org/jira/browse/MYFACES-3435?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16125609#comment-16125609 ] Thomas Andraschko commented on MYFACES-3435: I didn't find anything special which requires the wrapping. My applications are working still fine with and without viewpooling. Applied the new inheritance for _DeltaList and _ComponentChildrenList. _DeltaList is much cleaner now. > [perf] _DeltaList: use ArrayList as parent > -- > > Key: MYFACES-3435 > URL: https://issues.apache.org/jira/browse/MYFACES-3435 > Project: MyFaces Core > Issue Type: Improvement > Components: General >Affects Versions: 2.0.12-SNAPSHOT, 2.1.6-SNAPSHOT >Reporter: Martin Kočí >Assignee: Martin Kočí >Priority: Minor > Fix For: 2.3.0 > > Attachments: _ComponentChildrenList2.java, MYFACES-3435-2.patch, > MYFACES-3435-7.patch, MYFACES-3435.patch > > > Two internal classes _DeltaList in API: > 1) now use delegation pattern, but are always initialized with an ArrayList > instance -> use inheritance and ArrayList as parent -> improvement in memory > area, reduces number of GCed object in one request/response of (_DeltaList > instances/2) > 2) initialize expected size of _DeltaList (for example, number of validators > per one component is perhaps never 10, use: new _DeltaList(5)) > 3) use indexes in 'for' instead of iterator (java.util.RandomAccess) -> > improvement in performance -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (MYFACES-3435) [perf] _DeltaList: use ArrayList as parent
[ https://issues.apache.org/jira/browse/MYFACES-3435?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16123378#comment-16123378 ] Leonardo Uribe commented on MYFACES-3435: - Please check it > [perf] _DeltaList: use ArrayList as parent > -- > > Key: MYFACES-3435 > URL: https://issues.apache.org/jira/browse/MYFACES-3435 > Project: MyFaces Core > Issue Type: Improvement > Components: General >Affects Versions: 2.0.12-SNAPSHOT, 2.1.6-SNAPSHOT >Reporter: Martin Kočí >Assignee: Martin Kočí >Priority: Minor > Attachments: _ComponentChildrenList2.java, MYFACES-3435-2.patch, > MYFACES-3435-7.patch, MYFACES-3435.patch > > > Two internal classes _DeltaList in API: > 1) now use delegation pattern, but are always initialized with an ArrayList > instance -> use inheritance and ArrayList as parent -> improvement in memory > area, reduces number of GCed object in one request/response of (_DeltaList > instances/2) > 2) initialize expected size of _DeltaList (for example, number of validators > per one component is perhaps never 10, use: new _DeltaList(5)) > 3) use indexes in 'for' instead of iterator (java.util.RandomAccess) -> > improvement in performance -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (MYFACES-3435) [perf] _DeltaList: use ArrayList as parent
[ https://issues.apache.org/jira/browse/MYFACES-3435?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16123374#comment-16123374 ] Thomas Andraschko commented on MYFACES-3435: Yep, i would also only do something for 2.3. I didn't checked the View Pooling till now. Would you like to do this ticket? otherwise i could check it. > [perf] _DeltaList: use ArrayList as parent > -- > > Key: MYFACES-3435 > URL: https://issues.apache.org/jira/browse/MYFACES-3435 > Project: MyFaces Core > Issue Type: Improvement > Components: General >Affects Versions: 2.0.12-SNAPSHOT, 2.1.6-SNAPSHOT >Reporter: Martin Kočí >Assignee: Martin Kočí >Priority: Minor > Attachments: _ComponentChildrenList2.java, MYFACES-3435-2.patch, > MYFACES-3435-7.patch, MYFACES-3435.patch > > > Two internal classes _DeltaList in API: > 1) now use delegation pattern, but are always initialized with an ArrayList > instance -> use inheritance and ArrayList as parent -> improvement in memory > area, reduces number of GCed object in one request/response of (_DeltaList > instances/2) > 2) initialize expected size of _DeltaList (for example, number of validators > per one component is perhaps never 10, use: new _DeltaList(5)) > 3) use indexes in 'for' instead of iterator (java.util.RandomAccess) -> > improvement in performance -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (MYFACES-3435) [perf] _DeltaList: use ArrayList as parent
[ https://issues.apache.org/jira/browse/MYFACES-3435?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16123359#comment-16123359 ] Leonardo Uribe commented on MYFACES-3435: - I remember it is a trade-off between memory and speed/better code. View Pooling adds some extra cases to _DeltaList (reset delta), so it is an open case, that's the reason why the issue is still open. I would not apply it in 2.2 or lower versions but maybe for 2.3 > [perf] _DeltaList: use ArrayList as parent > -- > > Key: MYFACES-3435 > URL: https://issues.apache.org/jira/browse/MYFACES-3435 > Project: MyFaces Core > Issue Type: Improvement > Components: General >Affects Versions: 2.0.12-SNAPSHOT, 2.1.6-SNAPSHOT >Reporter: Martin Kočí >Assignee: Martin Kočí >Priority: Minor > Attachments: _ComponentChildrenList2.java, MYFACES-3435-2.patch, > MYFACES-3435-7.patch, MYFACES-3435.patch > > > Two internal classes _DeltaList in API: > 1) now use delegation pattern, but are always initialized with an ArrayList > instance -> use inheritance and ArrayList as parent -> improvement in memory > area, reduces number of GCed object in one request/response of (_DeltaList > instances/2) > 2) initialize expected size of _DeltaList (for example, number of validators > per one component is perhaps never 10, use: new _DeltaList(5)) > 3) use indexes in 'for' instead of iterator (java.util.RandomAccess) -> > improvement in performance -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (MYFACES-3435) [perf] _DeltaList: use ArrayList as parent
[ https://issues.apache.org/jira/browse/MYFACES-3435?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=1612#comment-1612 ] Thomas Andraschko commented on MYFACES-3435: [~lu4242] Not sure! I checked our applications and on _SMALL_ views, MyFaces creates around ~350 _DeltaLists, which means mean the patch could save around ~350 ArrayList instances of we change from wrapping to inheritance. For multiple users and on bigger views, this are a lot of ArrayList instances we could just save. > [perf] _DeltaList: use ArrayList as parent > -- > > Key: MYFACES-3435 > URL: https://issues.apache.org/jira/browse/MYFACES-3435 > Project: MyFaces Core > Issue Type: Improvement > Components: General >Affects Versions: 2.0.12-SNAPSHOT, 2.1.6-SNAPSHOT >Reporter: Martin Kočí >Assignee: Martin Kočí >Priority: Minor > Attachments: _ComponentChildrenList2.java, MYFACES-3435-2.patch, > MYFACES-3435-7.patch, MYFACES-3435.patch > > > Two internal classes _DeltaList in API: > 1) now use delegation pattern, but are always initialized with an ArrayList > instance -> use inheritance and ArrayList as parent -> improvement in memory > area, reduces number of GCed object in one request/response of (_DeltaList > instances/2) > 2) initialize expected size of _DeltaList (for example, number of validators > per one component is perhaps never 10, use: new _DeltaList(5)) > 3) use indexes in 'for' instead of iterator (java.util.RandomAccess) -> > improvement in performance -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (MYFACES-3435) [perf] _DeltaList: use ArrayList as parent
[ https://issues.apache.org/jira/browse/MYFACES-3435?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16123317#comment-16123317 ] Leonardo Uribe commented on MYFACES-3435: - I think the current code is ok. The patch is a good idea, but there are not enough compelling reasons to include it, the current code works just fine and there is no performance improvement here. Please note a lot of things happened over years that has made things change, and since a partial patch was already applied we can close this one as fixed. > [perf] _DeltaList: use ArrayList as parent > -- > > Key: MYFACES-3435 > URL: https://issues.apache.org/jira/browse/MYFACES-3435 > Project: MyFaces Core > Issue Type: Improvement > Components: General >Affects Versions: 2.0.12-SNAPSHOT, 2.1.6-SNAPSHOT >Reporter: Martin Kočí >Assignee: Martin Kočí >Priority: Minor > Attachments: _ComponentChildrenList2.java, MYFACES-3435-2.patch, > MYFACES-3435-7.patch, MYFACES-3435.patch > > > Two internal classes _DeltaList in API: > 1) now use delegation pattern, but are always initialized with an ArrayList > instance -> use inheritance and ArrayList as parent -> improvement in memory > area, reduces number of GCed object in one request/response of (_DeltaList > instances/2) > 2) initialize expected size of _DeltaList (for example, number of validators > per one component is perhaps never 10, use: new _DeltaList(5)) > 3) use indexes in 'for' instead of iterator (java.util.RandomAccess) -> > improvement in performance -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (MYFACES-3435) [perf] _DeltaList: use ArrayList as parent
[ https://issues.apache.org/jira/browse/MYFACES-3435?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16123240#comment-16123240 ] Thomas Andraschko commented on MYFACES-3435: [~lu4242] It seems that none of those patches were applied in the past. Do you think we should do it? IMO the patches are looking good and inheritance seems nicer as wrapping for me. > [perf] _DeltaList: use ArrayList as parent > -- > > Key: MYFACES-3435 > URL: https://issues.apache.org/jira/browse/MYFACES-3435 > Project: MyFaces Core > Issue Type: Improvement > Components: General >Affects Versions: 2.0.12-SNAPSHOT, 2.1.6-SNAPSHOT >Reporter: Martin Kočí >Assignee: Martin Kočí >Priority: Minor > Attachments: _ComponentChildrenList2.java, MYFACES-3435-2.patch, > MYFACES-3435-7.patch, MYFACES-3435.patch > > > Two internal classes _DeltaList in API: > 1) now use delegation pattern, but are always initialized with an ArrayList > instance -> use inheritance and ArrayList as parent -> improvement in memory > area, reduces number of GCed object in one request/response of (_DeltaList > instances/2) > 2) initialize expected size of _DeltaList (for example, number of validators > per one component is perhaps never 10, use: new _DeltaList(5)) > 3) use indexes in 'for' instead of iterator (java.util.RandomAccess) -> > improvement in performance -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (MYFACES-3435) [perf] _DeltaList: use ArrayList as parent
[ https://issues.apache.org/jira/browse/MYFACES-3435?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13223439#comment-13223439 ] Leonardo Uribe commented on MYFACES-3435: - I have checked again the patch and I founded some inconsistencies in FacetHashMap that were fixed in _ComponentFacetMap some time ago (implement fully Set interface for entrySet(), keySet() and valuesSet() ). Here is when delegation pattern shines. It is possible to use ArrayMap instead HashMap and improve performance in one step. If we use inheritance, I don't want to imagine the work required to fix this. I'm still thinking we can do other improvements (adjust structures sizes, cache some variables in some parts, ... ) and get bigger improvements without sacrify the code structure. In other words, there are other parts of the code that generate a lot more objects that this place. Anyway, I splitted the patch in two and I applied it partially, adjusting the structure's size and replacing HashMap with ArrayMap for facets. The other part of the patch (MYFACES-3435-7.patch) was attached here if in the future we came back to this point again. I'll let this issue open, but for now that's all I'm going to do for the next release cycle. > [perf] _DeltaList: use ArrayList as parent > -- > > Key: MYFACES-3435 > URL: https://issues.apache.org/jira/browse/MYFACES-3435 > Project: MyFaces Core > Issue Type: Improvement > Components: General >Affects Versions: 2.0.12-SNAPSHOT, 2.1.6-SNAPSHOT >Reporter: Martin Kočí >Assignee: Martin Kočí >Priority: Minor > Attachments: MYFACES-3435-2.patch, MYFACES-3435-7.patch, > MYFACES-3435.patch, _ComponentChildrenList2.java > > > Two internal classes _DeltaList in API: > 1) now use delegation pattern, but are always initialized with an ArrayList > instance -> use inheritance and ArrayList as parent -> improvement in memory > area, reduces number of GCed object in one request/response of (_DeltaList > instances/2) > 2) initialize expected size of _DeltaList (for example, number of validators > per one component is perhaps never 10, use: new _DeltaList(5)) > 3) use indexes in 'for' instead of iterator (java.util.RandomAccess) -> > improvement in performance -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MYFACES-3435) [perf] _DeltaList: use ArrayList as parent
[ https://issues.apache.org/jira/browse/MYFACES-3435?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13194003#comment-13194003 ] Leonardo Uribe commented on MYFACES-3435: - Ok, let's let this change for the next release cycle. Note a change in that part needs to be checked fully to prevent introduce bugs. > [perf] _DeltaList: use ArrayList as parent > -- > > Key: MYFACES-3435 > URL: https://issues.apache.org/jira/browse/MYFACES-3435 > Project: MyFaces Core > Issue Type: Improvement > Components: General >Affects Versions: 2.0.12-SNAPSHOT, 2.1.6-SNAPSHOT >Reporter: Martin Kočí >Assignee: Martin Kočí >Priority: Minor > Attachments: MYFACES-3435.patch, _ComponentChildrenList2.java > > > Two internal classes _DeltaList in API: > 1) now use delegation pattern, but are always initialized with an ArrayList > instance -> use inheritance and ArrayList as parent -> improvement in memory > area, reduces number of GCed object in one request/response of (_DeltaList > instances/2) > 2) initialize expected size of _DeltaList (for example, number of validators > per one component is perhaps never 10, use: new _DeltaList(5)) > 3) use indexes in 'for' instead of iterator (java.util.RandomAccess) -> > improvement in performance -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MYFACES-3435) [perf] _DeltaList: use ArrayList as parent
[ https://issues.apache.org/jira/browse/MYFACES-3435?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13193948#comment-13193948 ] Martin Kočí commented on MYFACES-3435: -- > because we can't assume the parent is implemented in some specific way ... I don't understand that fully: current code uses delegation pattern with ArrayList : it already assumes, that ArrayList implements methods in fashion we need. But I agree, that delegation pattern is elegant, but inheritance is sometimes worth too. Ad performance improvement: small if views are small and concurrency low. But consider this example: a view with 500 component + 500 concurrent request: 500 component= with for example 200 leafs = 300 ComponentChildrenList instances + 300 ArrayList instances 200 leafs (for example UIInput with a validator): 200 _DeltaList instances + 200 ArrayList instances = 1000 instances 1000 x 500 concurrent request = 500 000 instances, very short lived, normally under 500 ms. Those instances come only from VLD.buildView "pseudo" phase and represent very short lived memory representation of components tree (other part of code like state saving produce other amount of short lived instances) Garbage collector must for each instance compute if it is GCable or not (or something like that) : it takes very small time in modern JVM but this time is noticeable if the amout of object is enormous. Then, if user will perform a simple ajax request/response in such view with 500 components like: takes the garbage collecting more time that the render response phase. Yes, it depends on GC algorithm and normally does not block the http request/response thread. I tested it on HotSpot from JVM 6.0.30 with all server-side suitable GCs and with JRockit 4.1 Generational Parallel Mark & Sweep. But this issue is clearly not very important right now: the performace gain is not so big as in other improvement which I'm going to propose (as String's id caching in ComponentTagHandler or isRenderedSet() method) > [perf] _DeltaList: use ArrayList as parent > -- > > Key: MYFACES-3435 > URL: https://issues.apache.org/jira/browse/MYFACES-3435 > Project: MyFaces Core > Issue Type: Improvement > Components: General >Affects Versions: 2.0.12-SNAPSHOT, 2.1.6-SNAPSHOT >Reporter: Martin Kočí >Assignee: Martin Kočí >Priority: Minor > Attachments: MYFACES-3435.patch, _ComponentChildrenList2.java > > > Two internal classes _DeltaList in API: > 1) now use delegation pattern, but are always initialized with an ArrayList > instance -> use inheritance and ArrayList as parent -> improvement in memory > area, reduces number of GCed object in one request/response of (_DeltaList > instances/2) > 2) initialize expected size of _DeltaList (for example, number of validators > per one component is perhaps never 10, use: new _DeltaList(5)) > 3) use indexes in 'for' instead of iterator (java.util.RandomAccess) -> > improvement in performance -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MYFACES-3435) [perf] _DeltaList: use ArrayList as parent
[ https://issues.apache.org/jira/browse/MYFACES-3435?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13193903#comment-13193903 ] Leonardo Uribe commented on MYFACES-3435: - After study this issue, the problem about extend from ArrayList is it force us to implement all methods, because we can't assume the parent is implemented in some specific way. Things are different for classes extending AbstractList, because the contract specify that it is just required to implementing some methods. I did a draft for _ComponentChildrenList but I would like to think this change more carefully. My first impression is sometimes performance improvements can ruin you code and make it difficult to read. Is it worth to do it? suggestions are welcome. > [perf] _DeltaList: use ArrayList as parent > -- > > Key: MYFACES-3435 > URL: https://issues.apache.org/jira/browse/MYFACES-3435 > Project: MyFaces Core > Issue Type: Improvement > Components: General >Affects Versions: 2.0.12-SNAPSHOT, 2.1.6-SNAPSHOT >Reporter: Martin Kočí >Assignee: Martin Kočí >Priority: Minor > Attachments: MYFACES-3435.patch, _ComponentChildrenList2.java > > > Two internal classes _DeltaList in API: > 1) now use delegation pattern, but are always initialized with an ArrayList > instance -> use inheritance and ArrayList as parent -> improvement in memory > area, reduces number of GCed object in one request/response of (_DeltaList > instances/2) > 2) initialize expected size of _DeltaList (for example, number of validators > per one component is perhaps never 10, use: new _DeltaList(5)) > 3) use indexes in 'for' instead of iterator (java.util.RandomAccess) -> > improvement in performance -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
