Yes, this helps. The OldGen no longer maxes out, and the memory behavior
is the same as when I've removed the _oldSerializedView collection
completely, so this was a simpler solution.

But I must say this is a peculiar parameter. From what I gather, it
defines an upper limit on the number of times the user can hit "Back".
Except that with the implementation used here, the user might be able to
go further, if the necessary state happens to be in memory. For our
case, we only have a very few places where there is any use for this
functionality, but if we crank up this parameter to something like 7, we
greatly reduce the number of concurrent users we can support due to
memory restrictions, and for a vast majority of these users, this
parameter will not have any effect whatsoever, as the majority of
requests go against simple read-only views.

André Næss



Thomas Spiegl wrote:
> interesting. Actually it's using a soft reference. What if you substitute
>
> _oldSerializedViews = new ReferenceMap();
> by
> _oldSerializedViews = new ReferenceMap(AbstractReferenceMap.WEAK,
> AbstractReferenceMap.WEAK, true);
> ?
> I'm curious if the behaviour is still the same.
>
> Thomas
>
>
> On 6/4/07, André Næss <[EMAIL PROTECTED]> wrote:
>> Hi
>>
>> We are working on a JSF project and recently started stress testing the
>> system. This stress-testing caused the JVM to run out of memory and
>> spend most of it time doing Full GC. Analyzing the heap revealed that
>> most of the memory was being retained by SerializedViewCollection
>> instances. We also noticed that the value of NUMBER_OF_VIEWS_IN_SESSION
>> didn't seem to affect the memory usage.
>>
>> I then decided to have a look at the code, and found something very
>> interesting at line 614 in JspStateManagerImpl. When views are taken out
>> of the main collection they are stuffed into the _oldSerializedViews
>> collection which is supposed to be a weak hashmap. The idea here, I
>> suppose, is to use this as a kind of cache which trades some extra
>> memory for performance, but only if the memory is available.
>> Unfortunately it doesn't seem like this works as the author expected
>> when he wrote this:
>>
>>         // old views will be hold as soft references which will be
>> removed by
>>         // the garbage collector if free memory is low
>>
>> The problem we saw when profiling was the the OldGen would grow and grow
>> and reach a plateau, at which point the GC wasn't able to clean anything
>> out if it. I removed the usage of the _oldSerializedViews and this
>> immediately improved the situation. When the OldGen maxes out, a full GC
>> is executed which reclaims most of the data. I've successfully tested
>> this using JMeter, with up to 2000 threads running at a constant load of
>> 30 requests per second, and it seems fine.
>>
>> Unless the _oldSerializedViews object can be made to work properly, I
>> think it should be removed. After all, the developers can control the
>> memory in a more predictable manner using the NUMBER_OF_VIEWS_IN_SESSION
>> parameter. However, it is not entirely clear to me what the purpose of
>> storing the views in the SerializedViewsCollections really is? Is it
>> purely a performance trick (providing better rendering times), or does
>> it provide some other kind of advantage to the end user?
>>
>> André Næss
>>
>>
>>
>> Here's the svn diff output:
>>
>> ~/code/myfaces-1.1.5/impl/src/main/java/org/apache/myfaces/application/jsp$
>>
>> svn diff
>> Index: JspStateManagerImpl.java
>> ===================================================================
>> --- JspStateManagerImpl.java    (revision 543859)
>> +++ JspStateManagerImpl.java    (working copy)
>> @@ -41,6 +41,7 @@
>>  import javax.faces.render.ResponseStateManager;
>>  import java.io.*;
>>  import java.util.*;
>> +import java.util.logging.Logger;
>>  import java.util.zip.GZIPInputStream;
>>  import java.util.zip.GZIPOutputStream;
>>
>> @@ -611,7 +612,7 @@
>>
>>          // old views will be hold as soft references which will be
>> removed by
>>          // the garbage collector if free memory is low
>> -        private transient Map _oldSerializedViews = null;
>> +//        private transient Map _oldSerializedViews = null;
>>
>>          public synchronized void add(FacesContext context, Object
>> state) {
>>              Object key = new SerializedViewKey(context);
>> @@ -623,10 +624,10 @@
>>              int views = getNumberOfViewsInSession(context);
>>              while (_keys.size() > views) {
>>                  key = _keys.remove(0);
>> -                Object oldView = _serializedViews.remove(key);
>> -                if (oldView != null) {
>> -                    getOldSerializedViewsMap().put(key, oldView);
>> -                }
>> +                _serializedViews.remove(key);
>> +//                if (oldView != null) {
>> +//                    getOldSerializedViewsMap().put(key, oldView);
>> +//                }
>>              }
>>          }
>>
>> @@ -663,19 +664,19 @@
>>          /**
>>           * @return old serialized views map
>>           */
>> -        protected Map getOldSerializedViewsMap() {
>> -            if (_oldSerializedViews == null) {
>> -                _oldSerializedViews = new ReferenceMap();
>> -            }
>> -            return _oldSerializedViews;
>> -        }
>> +//        protected Map getOldSerializedViewsMap() {
>> +//            if (_oldSerializedViews == null) {
>> +//                _oldSerializedViews = new ReferenceMap();
>> +//            }
>> +//            return _oldSerializedViews;
>> +//        }
>>
>>          public Object get(Integer sequence, String viewId) {
>>              Object key = new SerializedViewKey(viewId, sequence);
>>              Object value = _serializedViews.get(key);
>> -            if (value == null) {
>> -                value = getOldSerializedViewsMap().get(key);
>> -            }
>> +//            if (value == null) {
>> +//                value = getOldSerializedViewsMap().get(key);
>> +//            }
>>              return value;
>>          }
>>      }
>>
>>
>
>

Reply via email to