ICEfaces
  1. ICEfaces
  2. ICE-5809

Optimize Portlet-request memory use when using extended request scope (high memory use under portlet load test)

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.8.2-EE-GA_P01
    • Fix Version/s: 1.8.3, 1.8.2-EE-GA_P02
    • Component/s: Framework
    • Labels:
      None
    • Environment:
      ICEfaces, Liferay, load test

      Description

      View Objects have been observed to leak under specific circumstances.

        Activity

        Hide
        Ted Goddard added a comment - - edited

        SessionDispatcher is a singleton attached to MainServlet.

        It had a direct reference to a map of sessionBoundServers which belonged to a variety of sessions.

        This has been replaced with a WeakHashMap, so if the session goes away, we do not keep the reference.

        A key was also added to the session itself, because without that the WeakHashMap would not keep the objects and they would likely go away immediately.

        SendUpdatedViews was modified so that globally accumulated updates would not hold a reference to the MainSessionBoundServlet beyond the session lifetime.

        Show
        Ted Goddard added a comment - - edited SessionDispatcher is a singleton attached to MainServlet. It had a direct reference to a map of sessionBoundServers which belonged to a variety of sessions. This has been replaced with a WeakHashMap, so if the session goes away, we do not keep the reference. A key was also added to the session itself, because without that the WeakHashMap would not keep the objects and they would likely go away immediately. SendUpdatedViews was modified so that globally accumulated updates would not hold a reference to the MainSessionBoundServlet beyond the session lifetime.
        Hide
        Ted Goddard added a comment -

        — core/src/com/icesoft/faces/webapp/http/servlet/SessionDispatcher.java (revision 21691)
        +++ core/src/com/icesoft/faces/webapp/http/servlet/SessionDispatcher.java (working copy)
        @@ -44,6 +44,7 @@
        import java.util.Collection;
        import java.util.Date;
        import java.util.HashMap;
        +import java.util.WeakHashMap;
        import java.util.HashSet;
        import java.util.Iterator;
        import java.util.List;
        @@ -67,7 +68,7 @@
        //ICE-3073 - manage sessions with this structure
        private final static Map SessionMonitors = new HashMap();

        • private final Map sessionBoundServers = new HashMap();
          + private final Map sessionBoundServers = new WeakHashMap();
          private final Map activeRequests = new HashMap();
          private final String sessionIdDelimiter;

        @@ -122,11 +123,15 @@

        synchronized (sessionBoundServers) {
        if (!sessionBoundServers.containsKey(id)) {

        • sessionBoundServers.put(id, this.newServer(session, monitor, new Authorization() {
          + PseudoServlet pservlet = this.newServer(session, monitor, new Authorization() {
          public boolean isUserInRole(String role) { return inRole(id, role); }
        • }));
          + });
          + //add to the session so that our WeakHashMap does not ignore the
          + //reference
          + session.setAttribute(id + ":sessionboundserver", pservlet);
          + sessionBoundServers.put(id, pservlet);
          }
          }
          }
          Index: core/src/com/icesoft/faces/webapp/http/core/SendUpdatedViews.java
          ===================================================================
            • core/src/com/icesoft/faces/webapp/http/core/SendUpdatedViews.java (revision 21691)
              +++ core/src/com/icesoft/faces/webapp/http/core/SendUpdatedViews.java (working copy)
              @@ -44,6 +44,7 @@
              import java.util.Collection;
              import java.util.HashSet;
              import java.util.Iterator;
              +import java.lang.ref.WeakReference;

        public class SendUpdatedViews implements Server, Runnable {
        private static final Runnable NOOP = new Runnable() {
        @@ -69,6 +70,7 @@
        private final MonitorRunner monitorRunner;
        private long responseTimeoutTime;
        private Server activeServer;
        + private WeakReference pageTestReference;

        public SendUpdatedViews(String sessionID, final Collection synchronouslyUpdatedViews, final ViewQueue allUpdatedViews, final MonitorRunner monitorRunner, Configuration configuration, final PageTest pageTest) {
        this.timeoutInterval = configuration.getAttributeAsLong("blockingConnectionTimeout", 90000);
        @@ -76,6 +78,7 @@
        this.allUpdatedViews = allUpdatedViews;
        this.synchronouslyUpdatedViews = synchronouslyUpdatedViews;
        this.monitorRunner = monitorRunner;
        + this.pageTestReference = new WeakReference(pageTest);

        allUpdatedViews.onPut(new Runnable() {
        public void run() {
        @@ -107,9 +110,12 @@
        //after first request switch to blocking server
        activeServer = blockingServer;

        + //will throw NullPointerException if the session expires
        + //and this service method is somehow invoked
        + PageTest pageTester = (PageTest) pageTestReference.get();
        //ICE-3493 – test if there was a previous request made for loading the page,
        //if not assume that this request was meant for a failed cluster node

        • if (pageTest.isLoaded()) {
          + if (pageTester.isLoaded()) { //page was loaded from this node, so use the blocking server super.service(request); }

          else {

        Show
        Ted Goddard added a comment - — core/src/com/icesoft/faces/webapp/http/servlet/SessionDispatcher.java (revision 21691) +++ core/src/com/icesoft/faces/webapp/http/servlet/SessionDispatcher.java (working copy) @@ -44,6 +44,7 @@ import java.util.Collection; import java.util.Date; import java.util.HashMap; +import java.util.WeakHashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -67,7 +68,7 @@ // ICE-3073 - manage sessions with this structure private final static Map SessionMonitors = new HashMap(); private final Map sessionBoundServers = new HashMap(); + private final Map sessionBoundServers = new WeakHashMap(); private final Map activeRequests = new HashMap(); private final String sessionIdDelimiter; @@ -122,11 +123,15 @@ synchronized (sessionBoundServers) { if (!sessionBoundServers.containsKey(id)) { sessionBoundServers.put(id, this.newServer(session, monitor, new Authorization() { + PseudoServlet pservlet = this.newServer(session, monitor, new Authorization() { public boolean isUserInRole(String role) { return inRole(id, role); } })); + }); + //add to the session so that our WeakHashMap does not ignore the + //reference + session.setAttribute(id + ":sessionboundserver", pservlet); + sessionBoundServers.put(id, pservlet); } } } Index: core/src/com/icesoft/faces/webapp/http/core/SendUpdatedViews.java =================================================================== core/src/com/icesoft/faces/webapp/http/core/SendUpdatedViews.java (revision 21691) +++ core/src/com/icesoft/faces/webapp/http/core/SendUpdatedViews.java (working copy) @@ -44,6 +44,7 @@ import java.util.Collection; import java.util.HashSet; import java.util.Iterator; +import java.lang.ref.WeakReference; public class SendUpdatedViews implements Server, Runnable { private static final Runnable NOOP = new Runnable() { @@ -69,6 +70,7 @@ private final MonitorRunner monitorRunner; private long responseTimeoutTime; private Server activeServer; + private WeakReference pageTestReference; public SendUpdatedViews(String sessionID, final Collection synchronouslyUpdatedViews, final ViewQueue allUpdatedViews, final MonitorRunner monitorRunner, Configuration configuration, final PageTest pageTest) { this.timeoutInterval = configuration.getAttributeAsLong("blockingConnectionTimeout", 90000); @@ -76,6 +78,7 @@ this.allUpdatedViews = allUpdatedViews; this.synchronouslyUpdatedViews = synchronouslyUpdatedViews; this.monitorRunner = monitorRunner; + this.pageTestReference = new WeakReference(pageTest); allUpdatedViews.onPut(new Runnable() { public void run() { @@ -107,9 +110,12 @@ //after first request switch to blocking server activeServer = blockingServer; + //will throw NullPointerException if the session expires + //and this service method is somehow invoked + PageTest pageTester = (PageTest) pageTestReference.get(); // ICE-3493 – test if there was a previous request made for loading the page, //if not assume that this request was meant for a failed cluster node if (pageTest.isLoaded()) { + if (pageTester.isLoaded()) { //page was loaded from this node, so use the blocking server super.service(request); } else {
        Hide
        Ted Goddard added a comment -

        Another fix may be to modify MonitorRunner so that the Runnable instances are removed from the ArrayList prior to run() so that the shutdown() is not necessary for this cleanup.

        public MonitorRunner(final long interval) {
        try {
        Thread thread = new Thread("Monitor Runner") {
        public void run() {
        while (run) {
        try {
        Thread.sleep(interval);
        Iterator i = new ArrayList(monitors).iterator();
        while (i.hasNext()) {
        Runnable monitor = (Runnable) i.next();
        try

        { monitor.run(); }

        catch (Throwable t)

        { log.warn("Failed to run monitor: " + monitor); }

        }
        } catch (InterruptedException e)

        { //do nothing }

        }
        }
        };
        thread.setDaemon(true);
        thread.start();
        } catch (Exception e)

        { log.error("Unable to initialize Monitor Runner ", e); }

        }

        Show
        Ted Goddard added a comment - Another fix may be to modify MonitorRunner so that the Runnable instances are removed from the ArrayList prior to run() so that the shutdown() is not necessary for this cleanup. public MonitorRunner(final long interval) { try { Thread thread = new Thread("Monitor Runner") { public void run() { while (run) { try { Thread.sleep(interval); Iterator i = new ArrayList(monitors).iterator(); while (i.hasNext()) { Runnable monitor = (Runnable) i.next(); try { monitor.run(); } catch (Throwable t) { log.warn("Failed to run monitor: " + monitor); } } } catch (InterruptedException e) { //do nothing } } } }; thread.setDaemon(true); thread.start(); } catch (Exception e) { log.error("Unable to initialize Monitor Runner ", e); } }
        Hide
        Ted Goddard added a comment -

        Entries are apparently removed from the map via:

        private void sessionDestroy(final String sessionId)

        { sessionBoundServers.remove(sessionId); }

        However, if the sessionId is slightly modified or sessionDestroy() is not called as expected, the map entries will remain.

        Show
        Ted Goddard added a comment - Entries are apparently removed from the map via: private void sessionDestroy(final String sessionId) { sessionBoundServers.remove(sessionId); } However, if the sessionId is slightly modified or sessionDestroy() is not called as expected, the map entries will remain.
        Hide
        Ted Goddard added a comment - - edited

        ASCII-art from the Eclipse memory analyzer (use monospaced font and a wide window to view):

        A large number of SendUpdatedViews are referenced in the monitors array:

        Class Name | Ref. Objects | Shallow Heap | Ref. Shallow Heap | Retained Heap
        ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
        com.icesoft.util.MonitorRunner$1 @ 0xfffffffe6cd92500 Monitor Runner Thread | 2,561 | 184 | 430,248 | 3,720
        '- this$0 com.icesoft.util.MonitorRunner @ 0xfffffffe6cd18f40 | 2,561 | 32 | 430,248 | 9,672
        '- monitors java.util.ArrayList @ 0xfffffffe6cd18f60 | 2,561 | 40 | 430,248 | 9,640
        '- elementData java.lang.Object[679] @ 0xfffffffe91963ae0 | 2,561 | 5,456 | 430,248 | 9,600

        • [12] com.icesoft.faces.webapp.http.core.SendUpdatedViews @ 0xfffffffe88a97840
        51 80 8,568 272
        • [465] com.icesoft.faces.webapp.http.core.SendUpdatedViews @ 0xfffffffefd6f3ef0
        27 80 4,536 272
        '- activeServer com.icesoft.faces.webapp.http.core.SendUpdatedViews$5 @ 0xfffffffefd6f3fe8 27 48 4,536 72
        '- val$pageTest com.icesoft.faces.webapp.http.servlet.MainSessionBoundServlet @ 0xfffffffefd6f31d0 27 88 4,536 2,645,568
        '- views java.util.Collections$SynchronizedMap @ 0xfffffffefd6f33a0 27 56 4,536 838,808
        ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

        views held by MainServlet pageTest:

        Class Name | Ref. Objects | Shallow Heap | Ref. Shallow Heap | Retained Heap
        -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
        [2] class com.icesoft.faces.util.event.servlet.ContextEventRepeater @ 0xfffffffe3d832058 | 1,438 | 96 | 241,584 | 239,584
        '- servletContextConfiguration com.icesoft.faces.webapp.http.servlet.ServletContextConfiguration @ 0xfffffffe6d980a30 | 1,438 | 32 | 241,584 | 32
        '- context weblogic.servlet.internal.WebAppServletContext @ 0xfffffffe614ce790 | 1,438 | 392 | 241,584 | 565,480
        '- attributes weblogic.servlet.internal.AttributesMap @ 0xfffffffe61789098 | 1,438 | 32 | 241,584 | 4,816
        '- attributes java.util.concurrent.ConcurrentHashMap @ 0xfffffffe617890b8 | 1,438 | 72 | 241,584 | 4,784
        '- segments java.util.concurrent.ConcurrentHashMap$Segment[16] @ 0xfffffffe61789100 | 1,438 | 152 | 241,584 | 4,712
        '- [4] java.util.concurrent.ConcurrentHashMap$Segment @ 0xfffffffe617898f0 | 1,438 | 48 | 241,584 | 1,160
        '- table java.util.concurrent.ConcurrentHashMap$HashEntry[2] @ 0xfffffffe6d73d008 | 1,438 | 40 | 241,584 | 1,064
        '- [1] java.util.concurrent.ConcurrentHashMap$HashEntry @ 0xfffffffe6d73d030 | 1,438 | 48 | 241,584 | 48
        '- value com.icesoft.faces.webapp.http.servlet.MainServlet$3 @ 0xfffffffe6cd18c28 | 1,438 | 80 | 241,584 | 219,182,600
        '- sessionBoundServers java.util.HashMap @ 0xfffffffe6cd18c78 | 1,438 | 64 | 241,584 | 219,180,256
        '- table java.util.HashMap$Entry[1024] @ 0xfffffffe7a21b410 | 1,438 | 8,216 | 241,584 | 219,180,192

        • [796] java.util.HashMap$Entry @ 0xffffffff01bd9108
        58 48 9,744 11,769,192
         
        • next java.util.HashMap$Entry @ 0xfffffffe803db5b0
        51 48 8,568 10,412,960
         
        • value com.icesoft.faces.webapp.http.servlet.MainSessionBoundServlet @ 0xffffffff019d02e0
        7 88 1,176 1,356,184
          '- views java.util.Collections$SynchronizedMap @ 0xffffffff019d0548 7 56 1,176 632
        -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
        Show
        Ted Goddard added a comment - - edited ASCII-art from the Eclipse memory analyzer (use monospaced font and a wide window to view): A large number of SendUpdatedViews are referenced in the monitors array: Class Name | Ref. Objects | Shallow Heap | Ref. Shallow Heap | Retained Heap --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- com.icesoft.util.MonitorRunner$1 @ 0xfffffffe6cd92500 Monitor Runner Thread | 2,561 | 184 | 430,248 | 3,720 '- this$0 com.icesoft.util.MonitorRunner @ 0xfffffffe6cd18f40 | 2,561 | 32 | 430,248 | 9,672 '- monitors java.util.ArrayList @ 0xfffffffe6cd18f60 | 2,561 | 40 | 430,248 | 9,640 '- elementData java.lang.Object [679] @ 0xfffffffe91963ae0 | 2,561 | 5,456 | 430,248 | 9,600 [12] com.icesoft.faces.webapp.http.core.SendUpdatedViews @ 0xfffffffe88a97840 51 80 8,568 272 [465] com.icesoft.faces.webapp.http.core.SendUpdatedViews @ 0xfffffffefd6f3ef0 27 80 4,536 272 '- activeServer com.icesoft.faces.webapp.http.core.SendUpdatedViews$5 @ 0xfffffffefd6f3fe8 27 48 4,536 72 '- val$pageTest com.icesoft.faces.webapp.http.servlet.MainSessionBoundServlet @ 0xfffffffefd6f31d0 27 88 4,536 2,645,568 '- views java.util.Collections$SynchronizedMap @ 0xfffffffefd6f33a0 27 56 4,536 838,808 --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- views held by MainServlet pageTest: Class Name | Ref. Objects | Shallow Heap | Ref. Shallow Heap | Retained Heap ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- [2] class com.icesoft.faces.util.event.servlet.ContextEventRepeater @ 0xfffffffe3d832058 | 1,438 | 96 | 241,584 | 239,584 '- servletContextConfiguration com.icesoft.faces.webapp.http.servlet.ServletContextConfiguration @ 0xfffffffe6d980a30 | 1,438 | 32 | 241,584 | 32 '- context weblogic.servlet.internal.WebAppServletContext @ 0xfffffffe614ce790 | 1,438 | 392 | 241,584 | 565,480 '- attributes weblogic.servlet.internal.AttributesMap @ 0xfffffffe61789098 | 1,438 | 32 | 241,584 | 4,816 '- attributes java.util.concurrent.ConcurrentHashMap @ 0xfffffffe617890b8 | 1,438 | 72 | 241,584 | 4,784 '- segments java.util.concurrent.ConcurrentHashMap$Segment [16] @ 0xfffffffe61789100 | 1,438 | 152 | 241,584 | 4,712 '- [4] java.util.concurrent.ConcurrentHashMap$Segment @ 0xfffffffe617898f0 | 1,438 | 48 | 241,584 | 1,160 '- table java.util.concurrent.ConcurrentHashMap$HashEntry [2] @ 0xfffffffe6d73d008 | 1,438 | 40 | 241,584 | 1,064 '- [1] java.util.concurrent.ConcurrentHashMap$HashEntry @ 0xfffffffe6d73d030 | 1,438 | 48 | 241,584 | 48 '- value com.icesoft.faces.webapp.http.servlet.MainServlet$3 @ 0xfffffffe6cd18c28 | 1,438 | 80 | 241,584 | 219,182,600 '- sessionBoundServers java.util.HashMap @ 0xfffffffe6cd18c78 | 1,438 | 64 | 241,584 | 219,180,256 '- table java.util.HashMap$Entry [1024] @ 0xfffffffe7a21b410 | 1,438 | 8,216 | 241,584 | 219,180,192 [796] java.util.HashMap$Entry @ 0xffffffff01bd9108 58 48 9,744 11,769,192   next java.util.HashMap$Entry @ 0xfffffffe803db5b0 51 48 8,568 10,412,960   value com.icesoft.faces.webapp.http.servlet.MainSessionBoundServlet @ 0xffffffff019d02e0 7 88 1,176 1,356,184   '- views java.util.Collections$SynchronizedMap @ 0xffffffff019d0548 7 56 1,176 632 -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
        Hide
        Deryk Sinotte added a comment -

        We have client branches that have most of these issues patched. We first need to audit the patches, add any that aren't already on the trunk, and re-test.

        Show
        Deryk Sinotte added a comment - We have client branches that have most of these issues patched. We first need to audit the patches, add any that aren't already on the trunk, and re-test.
        Hide
        Deryk Sinotte added a comment -

        Waiting on audit and analysis of relevant customer patches with include memory fixes for portlets.

        Show
        Deryk Sinotte added a comment - Waiting on audit and analysis of relevant customer patches with include memory fixes for portlets.
        Hide
        Deryk Sinotte added a comment -

        As a result of the work done in ICE-6197 to merge the changes from two customer patch branches, the various fixes for these issues have now been migrated back to the trunk.

        Show
        Deryk Sinotte added a comment - As a result of the work done in ICE-6197 to merge the changes from two customer patch branches, the various fixes for these issues have now been migrated back to the trunk.

          People

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

            Dates

            • Created:
              Updated:
              Resolved: