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

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Not A Problem
• Affects Version/s: 1.1.9, 1.1.10
• Component/s: selectOneRadio / radio
• Environment: Myfaces 1.2.9/ 1.2.10, Richfaces 3.3.3, Facelets, Tomcat 6.0.32
• Assignee: Leonardo Uribe
• Reporter: Michael Heinen
• Created: 26/Oct/11 15:52
• Updated: 18/Apr/12 01:59
• Resolved: 18/Apr/12 01:59

Description

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