[jira] [Commented] (TOMAHAWK-1605) ValueExpressions for SelectItems are evaluated too often for checboxes or radios with spread layout

["Leonardo Uribe (Commented) (JIRA)" <de...@myfaces.apache.org>]

    [ https://issues.apache.org/jira/browse/TOMAHAWK-1605?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13256139#comment-13256139 ] 

Leonardo Uribe commented on TOMAHAWK-1605:
------------------------------------------

I checked the code and it seems the implementation changed completely in tomahawk20, so the duplicate call is not there anymore in a new version of shared.

I know it is not optimal from perf perspective, but I think we should let it as is in 1.2 branch. If there is a problem with caching at application scope and change it later, that stuff should be handled by app itself, taking the data from app scope and storing it in request scope for example. This is a common bug, but the framework cannot help, because if the data is stored in app scope, in that location is the only place where thread safety can be warranted.

I'll close this one as not a problem, because the latest code does not have the related lines anymore. Thanks for the report.
                
> ValueExpressions for SelectItems are evaluated too often for checboxes or radios with spread layout
> ---------------------------------------------------------------------------------------------------
>
>                 Key: TOMAHAWK-1605
>                 URL: https://issues.apache.org/jira/browse/TOMAHAWK-1605
>             Project: MyFaces Tomahawk
>          Issue Type: Bug
>          Components: selectOneRadio / radio
>    Affects Versions: 1.1.9, 1.1.10
>         Environment: Myfaces 1.2.9/ 1.2.10, Richfaces 3.3.3, Facelets, Tomcat 6.0.32
>            Reporter: Michael Heinen
>
> The method HtmlCheckboxRenderer.renderSingleCheckbox (or HtmlRadioRenderer.renderRadio) calls for each checkbox RendererUtils.getSelectItemList(uiSelectMany) two times.
> RendererUtils creates a new SelectItemsIterator which evaluates the value expression again for each checkbox that should be rendered and creates each time a new List with SelectItems.
> It is caused by
> - method call getConverter(facesContext, uiSelectMany) which calls RendererUtils.getSelectItemList(uiSelectMany)
> - and few lines later by direct call of RendererUtils.getSelectItemList(uiSelectMany)
>  
> From a performance point of view this is not optimal. It would be better to create the list once during response rendering and cache it somewhere (maybe in request scope).
> But even worse (and therefore from my pov a bug) is that this repeated evaluation could result in synchronization problems during the rendering of the response if the bound SelectItems array or collection is changed.
> This could easily happen if the SelectItems are cached in an application scoped bean.
> An IndexOutOfBoundsException will occur during response rendering if an item is removed by another thread.
> This does not happen with "list" or "page" layout because the expression to get the SelectItems is evaluated once and then stored as a local var.
> RendererUtils.internalGetSelectItemList contains already a comment for this:
> "TODO: Shall we cache the list in a component attribute?"

--
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