[jira] Created: (TRINIDAD-1164) getFacetsAndChildren from UIXComponentBase allocates a lot of memory

[My Faces - Dev mailing list]

getFacetsAndChildren from UIXComponentBase allocates a lot of memory
--------------------------------------------------------------------

                 Key: TRINIDAD-1164
                 URL: https://issues.apache.org/jira/browse/TRINIDAD-1164
             Project: MyFaces Trinidad
          Issue Type: Bug
    Affects Versions:  1.2.8-core
            Reporter: Stevan Malesevic


getFacetsAndChildren of UIXComponentBase is invoked number of times during request processing (_findInsideOf , decode,....). In the case where component has facets the code will rebuild a ArrayList on every invocation. This is very expensive in terms of total allocated transient memory and CPU. We should save the ArrayList of facets and children first time it is built and recreate only in case of change.
Also,  the call _facets.values() which creates iterator seems very expensive. Maybe better approach is to just do:

    for(int i = 0; i < _facets.size(); i++)
    {
        childrenAndFacets.add(((FacetHashMap)_facets).getValue(i));
    }

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[My Faces - Dev mailing list]

    [ https://issues.apache.org/jira/browse/TRINIDAD-1164?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12615442#action_12615442 ]

Blake Sullivan commented on TRINIDAD-1164:
------------------------------------------

In the case where both the list of indexed children and the map of facet children are non-empty, Trinidad should be returning a compositer iterator, delegating to the other two iterators, rather than using an ArrayList.

> getFacetsAndChildren from UIXComponentBase allocates a lot of memory
> --------------------------------------------------------------------
>
>                 Key: TRINIDAD-1164
>                 URL: https://issues.apache.org/jira/browse/TRINIDAD-1164
>             Project: MyFaces Trinidad
>          Issue Type: Bug
>    Affects Versions:  1.2.8-core
>            Reporter: Stevan Malesevic
>
> getFacetsAndChildren of UIXComponentBase is invoked number of times during request processing (_findInsideOf , decode,....). In the case where component has facets the code will rebuild a ArrayList on every invocation. This is very expensive in terms of total allocated transient memory and CPU. We should save the ArrayList of facets and children first time it is built and recreate only in case of change.
> Also,  the call _facets.values() which creates iterator seems very expensive. Maybe better approach is to just do:
>     for(int i = 0; i < _facets.size(); i++)
>     {
>         childrenAndFacets.add(((FacetHashMap)_facets).getValue(i));
>     }

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[My Faces - Dev mailing list]

    [ https://issues.apache.org/jira/browse/TRINIDAD-1164?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12620866#action_12620866 ]

Blake Sullivan commented on TRINIDAD-1164:
------------------------------------------

To calrify my comment further.  There is no need to ever build an array list that contains both items.  In the case where both the child list and Facet Map are non-empty we should return a composite Iterator containing the Iterator of the child List and the ValueSet iterator of the the Facet Map.  It would use the child List iterator until it is empty and then internally switch to the Facet map's value iterator.

The implementation class would look something like this:

/** not thread safe, though it could be as thread-safe as the underlying Iterators at a slight cost to performance and size */
public final class CompositeIterator<T> implements Iterator<T>
{
  public CompositeIterator(Iterator<T> firstIterator, Iterator<T> secondIterator)
  {
    if (firstIterator == null)
      throw new NullPointerException();

    if (secondIterator == null)
      throw new NullPointerException();

    // determine whether we have another element aggressively
    _hasNext = firstIterator.hasNext();

    if (_hasNext)
    {
      _firstIterator = firstIterator;
      _secondIterator = secondIterator;
    }
    else
    {
      // first iterator is empty so shift up the second iterator
      _firstIterator = secondIterator;
      _secondIterator = null;
      _hasNext = secondIterator.hasNext();
    }
  }

  public boolean hasNext()
  {
    // return cached value
    return _hasNext;
  }

  public T next()
  {
    // remove is only valid after next(), so assign it here.
    // This also deals with issue where next() is called on the
    // last element of the firstIterator and then remove is called
    // this way the removeIterator correctly lags shifting from the
    // old _firstIterator to the _secondIterator
    _removeIterator = _firstIterator;

    T nextValue = _firstIterator.next();
   
    _hasNext = _firstIterator.hasNext();

    if (!hasNext && (_secondIterator  != null))
    {
      // firsIterator is done, so shift up the secondIterator
      _firstIterator = _secondIterator;
      _secondIterator = null;
      _hasNext = _firstIterator.hasNext();
    }

    return nextValue;
  }

  public void remove()
  {
    if (_removeIterator == null)
      throw new IllegalStateException();

    _removeIterator.remove();
  }

  private Iterator<T> _firstIterator;
  private Iterator<T> _secondIterator;
  private Iterator<T> _removeIterator;
  private boolean _hasNext;
}

> getFacetsAndChildren from UIXComponentBase allocates a lot of memory
> --------------------------------------------------------------------
>
>                 Key: TRINIDAD-1164
>                 URL: https://issues.apache.org/jira/browse/TRINIDAD-1164
>             Project: MyFaces Trinidad
>          Issue Type: Bug
>    Affects Versions:  1.2.8-core
>            Reporter: Stevan Malesevic
>         Attachments: trinidad-1164.patch
>
>
> getFacetsAndChildren of UIXComponentBase is invoked number of times during request processing (_findInsideOf , decode,....). In the case where component has facets the code will rebuild a ArrayList on every invocation. This is very expensive in terms of total allocated transient memory and CPU. We should save the ArrayList of facets and children first time it is built and recreate only in case of change.
> Also,  the call _facets.values() which creates iterator seems very expensive. Maybe better approach is to just do:
>     for(int i = 0; i < _facets.size(); i++)
>     {
>         childrenAndFacets.add(((FacetHashMap)_facets).getValue(i));
>     }

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[My Faces - Dev mailing list]

     [ https://issues.apache.org/jira/browse/TRINIDAD-1164?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Matthias Weßendorf resolved TRINIDAD-1164.
------------------------------------------

       Resolution: Fixed
    Fix Version/s: 1.0.10-core
                   1.2.10-core

> getFacetsAndChildren from UIXComponentBase allocates a lot of memory
> --------------------------------------------------------------------
>
>                 Key: TRINIDAD-1164
>                 URL: https://issues.apache.org/jira/browse/TRINIDAD-1164
>             Project: MyFaces Trinidad
>          Issue Type: Bug
>    Affects Versions:  1.2.8-core
>            Reporter: Stevan Malesevic
>            Assignee: Matthias Weßendorf
>             Fix For: 1.2.10-core, 1.0.10-core
>
>         Attachments: trinidad-1164.patch, trinidad-1164_blake.patch
>
>
> getFacetsAndChildren of UIXComponentBase is invoked number of times during request processing (_findInsideOf , decode,....). In the case where component has facets the code will rebuild a ArrayList on every invocation. This is very expensive in terms of total allocated transient memory and CPU. We should save the ArrayList of facets and children first time it is built and recreate only in case of change.
> Also,  the call _facets.values() which creates iterator seems very expensive. Maybe better approach is to just do:
>     for(int i = 0; i < _facets.size(); i++)
>     {
>         childrenAndFacets.add(((FacetHashMap)_facets).getValue(i));
>     }

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.