ICEfaces
  1. ICEfaces
  2. ICE-6228

Tidying of request attributes is removing request-scoped beans

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.8.2-EE-GA_P01
    • Fix Version/s: 1.8.2-EE-GA_P02
    • Component/s: Framework
    • Labels:
      None
    • Environment:
      ICEfaces 1.8 portlet

      Description

      As part of the changes merged in ICE-6197, there was a fix for live memory consumption where the request map was cleared. While this release references that were causing a memory leak, it also cleared out the request-scoped beans. With extended request scope, this causes problems as the beans are reset each time.

        Activity

        Hide
        Deryk Sinotte added a comment -

        The merge (ICE-6197) from the patched branch included this code in PortletExternalContext.release:

        requestMap.clear();

        This call was one of the main ways we found to significantly reduce memory usage in the load testing. Unfortunately, what we didn't catch at the time is that this clears out the request-scoped beans as well.

        I made some adjustments in the 1.8 trunk after the merge to try and be more selective about what gets cleared out of the requestMap - leaving the beans there and removing request and response references which seemed to be the cause of the memory accumulation. Since then I've been trying to verify that the memory fix is still doing it's job. At this point, it doesn't look good.

        My method is to run the chat portlet then use a simple curl script to simulated a couple of hundred users making a simple request to the portal page that has the chat portlet on it. This is the same strategy we used before to verify the memory usage:

        for index in

        {00..99}

        ; do curl http://localhost:8080/web/guest/chat; done

        Using the Eclipse MAT (Memory Analyzer Tool) to examine the live heap. Specifically, I look at how many Views there are and how much retained memory there is from the View:

        For the EE p_01 official release (before patching), it shows this:

        Class Name | Objects | Shallow Heap | Retained Heap
        ----------------------------------------------------------------------
        com.icesoft.faces.context.View| 201 | 33,768 | 35,474,680
        ----------------------------------------------------------------------

        After all the changes we made on the SF-9412 branch - including the requestMap.clear() call, it shows this:

        Class Name | Objects | Shallow Heap | Retained Heap
        ----------------------------------------------------------------------
        com.icesoft.faces.context.View| 201 | 33,768 | 7,843,752
        ----------------------------------------------------------------------

        So I've been trying to verify that the merge from the patched branch we did along with all the other changes shows the same behaviour. After many different iterations and experiments, I finally cleared everything from the requestMap. The result was:

        Class Name | Objects | Shallow Heap | Retained Heap
        ----------------------------------------------------------------------
        com.icesoft.faces.context.View| 201 | 33,768 | 35,469,520
        ----------------------------------------------------------------------

        So the difference is that the technique I'm currently using to remove items from the request map is in some way markedly different than simply calling clear().

        Show
        Deryk Sinotte added a comment - The merge (ICE-6197) from the patched branch included this code in PortletExternalContext.release: requestMap.clear(); This call was one of the main ways we found to significantly reduce memory usage in the load testing. Unfortunately, what we didn't catch at the time is that this clears out the request-scoped beans as well. I made some adjustments in the 1.8 trunk after the merge to try and be more selective about what gets cleared out of the requestMap - leaving the beans there and removing request and response references which seemed to be the cause of the memory accumulation. Since then I've been trying to verify that the memory fix is still doing it's job. At this point, it doesn't look good. My method is to run the chat portlet then use a simple curl script to simulated a couple of hundred users making a simple request to the portal page that has the chat portlet on it. This is the same strategy we used before to verify the memory usage: for index in {00..99} ; do curl http://localhost:8080/web/guest/chat ; done Using the Eclipse MAT (Memory Analyzer Tool) to examine the live heap. Specifically, I look at how many Views there are and how much retained memory there is from the View: For the EE p_01 official release (before patching), it shows this: Class Name | Objects | Shallow Heap | Retained Heap ---------------------------------------------------------------------- com.icesoft.faces.context.View| 201 | 33,768 | 35,474,680 ---------------------------------------------------------------------- After all the changes we made on the SF-9412 branch - including the requestMap.clear() call, it shows this: Class Name | Objects | Shallow Heap | Retained Heap ---------------------------------------------------------------------- com.icesoft.faces.context.View| 201 | 33,768 | 7,843,752 ---------------------------------------------------------------------- So I've been trying to verify that the merge from the patched branch we did along with all the other changes shows the same behaviour. After many different iterations and experiments, I finally cleared everything from the requestMap. The result was: Class Name | Objects | Shallow Heap | Retained Heap ---------------------------------------------------------------------- com.icesoft.faces.context.View| 201 | 33,768 | 35,469,520 ---------------------------------------------------------------------- So the difference is that the technique I'm currently using to remove items from the request map is in some way markedly different than simply calling clear().
        Hide
        Deryk Sinotte added a comment -

        Turns out the following code is not actually removing the attributes from the underlying collection:

        Iterator keys = requestMap.keySet().iterator();
        while (keys.hasNext()) {
        String key = (String)keys.next();
        if( "javax.portlet.request".equals(key) ||
        "javax.portlet.response".equals(key)){
        keys.remove();

        It may be our extension of HashMap in AbstractCopyingAttributeMap that's not quite right but in any event, I've changed the code so that instead of trying to remove the items of interest directly in the interator, we simply store the key temporarily and then, after the interation, call remove directly. Measuring this approach with MAT shows that we're back to normal and the request scope beans are now left intact.

        Show
        Deryk Sinotte added a comment - Turns out the following code is not actually removing the attributes from the underlying collection: Iterator keys = requestMap.keySet().iterator(); while (keys.hasNext()) { String key = (String)keys.next(); if( "javax.portlet.request".equals(key) || "javax.portlet.response".equals(key)){ keys.remove(); It may be our extension of HashMap in AbstractCopyingAttributeMap that's not quite right but in any event, I've changed the code so that instead of trying to remove the items of interest directly in the interator, we simply store the key temporarily and then, after the interation, call remove directly. Measuring this approach with MAT shows that we're back to normal and the request scope beans are now left intact.
        Hide
        Deryk Sinotte added a comment -

        MAT numbers are now aligned with the values when request.clear() is used and the portlet is still functional as the request-scoped beans are still available.

        Show
        Deryk Sinotte added a comment - MAT numbers are now aligned with the values when request.clear() is used and the portlet is still functional as the request-scoped beans are still available.

          People

          • Assignee:
            Deryk Sinotte
            Reporter:
            Deryk Sinotte
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: