You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by iv...@apache.org on 2010/11/14 08:19:10 UTC
svn commit: r1034948 - in /wicket/branches/wicket-1.4.x/wicket/src:
main/java/org/apache/wicket/PageMap.java
main/java/org/apache/wicket/Session.java
test/java/org/apache/wicket/PageMapTest.java
Author: ivaynberg
Date: Sun Nov 14 07:19:10 2010
New Revision: 1034948
URL: http://svn.apache.org/viewvc?rev=1034948&view=rev
Log:
Issue: WICKET-3108
Modified:
wicket/branches/wicket-1.4.x/wicket/src/main/java/org/apache/wicket/PageMap.java
wicket/branches/wicket-1.4.x/wicket/src/main/java/org/apache/wicket/Session.java
wicket/branches/wicket-1.4.x/wicket/src/test/java/org/apache/wicket/PageMapTest.java
Modified: wicket/branches/wicket-1.4.x/wicket/src/main/java/org/apache/wicket/PageMap.java
URL: http://svn.apache.org/viewvc/wicket/branches/wicket-1.4.x/wicket/src/main/java/org/apache/wicket/PageMap.java?rev=1034948&r1=1034947&r2=1034948&view=diff
==============================================================================
--- wicket/branches/wicket-1.4.x/wicket/src/main/java/org/apache/wicket/PageMap.java (original)
+++ wicket/branches/wicket-1.4.x/wicket/src/main/java/org/apache/wicket/PageMap.java Sun Nov 14 07:19:10 2010
@@ -346,11 +346,14 @@ public abstract class PageMap implements
}
/**
- *
+ * Marking this PageMap as the most recently used only if it isn't already removed from session.
*/
protected final void dirty()
{
- Session.get().dirtyPageMap(this);
+ if (Session.get().getPageMaps().contains(this))
+ {
+ Session.get().dirtyPageMap(this);
+ }
}
/**
Modified: wicket/branches/wicket-1.4.x/wicket/src/main/java/org/apache/wicket/Session.java
URL: http://svn.apache.org/viewvc/wicket/branches/wicket-1.4.x/wicket/src/main/java/org/apache/wicket/Session.java?rev=1034948&r1=1034947&r2=1034948&view=diff
==============================================================================
--- wicket/branches/wicket-1.4.x/wicket/src/main/java/org/apache/wicket/Session.java (original)
+++ wicket/branches/wicket-1.4.x/wicket/src/main/java/org/apache/wicket/Session.java Sun Nov 14 07:19:10 2010
@@ -19,6 +19,7 @@ package org.apache.wicket;
import java.io.Serializable;
import java.util.ArrayList;
import java.util.Collections;
+import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
@@ -351,8 +352,8 @@ public abstract class Session implements
*/
private transient Map<String, Object> temporarySessionAttributes;
- /** A linked list for last used pagemap queue */
- private final LinkedList/* <IPageMap> */<IPageMap> usedPageMaps = new LinkedList<IPageMap>();
+ /** A linked list for last used pagemap names queue */
+ private final LinkedList<String> usedPageMapNames = new LinkedList<String>();
/**
* Constructor. Note that {@link RequestCycle} is not available until this constructor returns.
@@ -705,7 +706,7 @@ public abstract class Session implements
IPageMap pageMap = pageMapForName(pageMapName, pageMapName == PageMap.DEFAULT_NAME);
if (pageMap != null)
{
- synchronized (usedPageMaps) // get a lock so be sure that only one
+ synchronized (usedPageMapNames) // get a lock so be sure that only one
// is made
{
if (pageMapsUsedInRequest == null)
@@ -809,10 +810,24 @@ public abstract class Session implements
list.add((IPageMap)getAttribute(attribute));
}
}
+ Collections.sort(list, new LruComparator());
return list;
}
/**
+ * Sorting page maps respecting the least recently used sequence.
+ */
+ private class LruComparator implements Comparator<IPageMap>
+ {
+ public int compare(IPageMap pg1, IPageMap pg2)
+ {
+ Integer pg1Index = usedPageMapNames.indexOf(pg1.getName());
+ Integer pg2Index = usedPageMapNames.indexOf(pg2.getName());
+ return pg1Index.compareTo(pg2Index);
+ }
+ }
+
+ /**
* @return Size of this session, including all the pagemaps it contains
*/
public final long getSizeInBytes()
@@ -919,18 +934,26 @@ public abstract class Session implements
{
// Check that session doesn't have too many page maps already, if so, evict
final int maxPageMaps = getApplication().getSessionSettings().getMaxPageMaps();
- synchronized (usedPageMaps)
+ synchronized (usedPageMapNames)
{
- while (usedPageMaps.size() >= maxPageMaps)
+ List<IPageMap> usedPageMaps = getPageMaps();
+ int excessPagemaps = (usedPageMaps.size() + 1) - maxPageMaps;/*
+ * plus 1 meaning the new
+ * one we are about to add
+ */
+ if (excessPagemaps > 0)
{
- IPageMap pm = usedPageMaps.getFirst();
- pm.remove();
+ for (int i = 0; i < excessPagemaps; i++)
+ {
+ usedPageMaps.get(i).remove();
+ }
}
}
// Create new page map
final IPageMap pageMap = getSessionStore().createPageMap(name);
setAttribute(attributeForPageMapName(name), pageMap);
+ // marking it as the most recently used
dirtyPageMap(pageMap);
dirty();
return pageMap;
@@ -967,9 +990,9 @@ public abstract class Session implements
pagemapMetaData.pageMapNames.remove(pageMap.getName());
}
- synchronized (usedPageMaps)
+ synchronized (usedPageMapNames)
{
- usedPageMaps.remove(pageMap);
+ usedPageMapNames.remove(pageMap.getName());
}
// the page map also needs to be removed from the dirty objects list or
@@ -1147,7 +1170,7 @@ public abstract class Session implements
* Name of page map
* @return Session attribute holding page map
*/
- private final String attributeForPageMapName(final String pageMapName)
+ private static final String attributeForPageMapName(final String pageMapName)
{
return pageMapAttributePrefix + pageMapName;
}
@@ -1362,10 +1385,10 @@ public abstract class Session implements
return;
}
- synchronized (usedPageMaps)
+ synchronized (usedPageMapNames)
{
- usedPageMaps.remove(map);
- usedPageMaps.addLast(map);
+ usedPageMapNames.remove(map.getName());
+ usedPageMapNames.addLast(map.getName());
}
List<IClusterable> dirtyObjects = getDirtyObjectsList();
Modified: wicket/branches/wicket-1.4.x/wicket/src/test/java/org/apache/wicket/PageMapTest.java
URL: http://svn.apache.org/viewvc/wicket/branches/wicket-1.4.x/wicket/src/test/java/org/apache/wicket/PageMapTest.java?rev=1034948&r1=1034947&r2=1034948&view=diff
==============================================================================
--- wicket/branches/wicket-1.4.x/wicket/src/test/java/org/apache/wicket/PageMapTest.java (original)
+++ wicket/branches/wicket-1.4.x/wicket/src/test/java/org/apache/wicket/PageMapTest.java Sun Nov 14 07:19:10 2010
@@ -16,23 +16,43 @@
*/
package org.apache.wicket;
+import java.io.Serializable;
+import java.util.ArrayList;
+
+import junit.framework.TestCase;
+
import org.apache.wicket.markup.IMarkupResourceStreamProvider;
import org.apache.wicket.markup.html.WebPage;
import org.apache.wicket.markup.html.link.Link;
+import org.apache.wicket.model.Model;
+import org.apache.wicket.protocol.http.SecondLevelCacheSessionStore;
import org.apache.wicket.protocol.http.WebRequestCycle;
+import org.apache.wicket.protocol.http.pagestore.DiskPageStore;
import org.apache.wicket.protocol.http.request.WebRequestCodingStrategy;
+import org.apache.wicket.session.ISessionStore;
+import org.apache.wicket.util.lang.Objects;
import org.apache.wicket.util.resource.IResourceStream;
import org.apache.wicket.util.resource.StringResourceStream;
+import org.apache.wicket.util.tester.WicketTester;
/**
* https://issues.apache.org/jira/browse/WICKET-3108
*/
-public class PageMapTest extends WicketTestCase
+public class PageMapTest extends TestCase
{
private static final String TO_REMOVE_PAGE_MAP = "test-0";
private static final String WORKING_PAGE_MAP = "test-1";
// just to have an test limit, can be any value
private static final int MAX_PAGE_MAPS = 2;
+ private static final long BIG_OBJECT_SIZE = 10000;
+
+ private WicketTester tester;
+
+ @Override
+ protected void setUp() throws Exception
+ {
+ tester = new WicketTester();
+ }
/**
* Making sure that the TO_REMOVE_PAGEMAP doesn't get back to session after a call to
@@ -58,7 +78,6 @@ public class PageMapTest extends WicketT
}
}
-
/**
* Requesting (maxPageMaps + 1) pages on distinct pagemaps
*/
@@ -87,6 +106,72 @@ public class PageMapTest extends WicketT
assertTrue(MAX_PAGE_MAPS >= tester.getWicketSession().getPageMaps().size());
}
+ /**
+ * Making sure that the PageMap don't get serialized with the session. If this test is failing,
+ * please look for any possible memory leak and than increase the BIG_OBJECT_SIZE.
+ *
+ * @see https://issues.apache.org/jira/browse/WICKET-3160
+ */
+ public void testPagemapIsNotReferencedBySession()
+ {
+ tester = new WicketTester(new WicketTester.DummyWebApplication()
+ {
+ @Override
+ protected ISessionStore newSessionStore()
+ {
+ return new SecondLevelCacheSessionStore(this, new DiskPageStore());
+ }
+ });
+ BigObject bigObject = new BigObject();
+ long bigObjectSize = Objects.sizeof(bigObject);
+ TestPage testPage = new TestPage();
+ testPage.setDefaultModel(new Model(bigObject));
+ tester.startPage(testPage);
+ long sessionSize = Objects.sizeof(tester.getWicketSession());
+ assertTrue(sessionSize < bigObjectSize);
+ }
+
+ public static class BigObject implements Serializable
+ {
+ private final ArrayList list = new ArrayList();
+ {
+ for (int i = 0; i < BIG_OBJECT_SIZE; i++)
+ {
+ list.add(Byte.MIN_VALUE);
+ }
+ }
+ }
+
+ /**
+ * Making sure that the getPageMaps return the correctly LRU page maps sequence
+ */
+ public void testLruCache()
+ {
+ for (int i = 0; i < MAX_PAGE_MAPS; i++)
+ {
+ tester.getWicketSession().newPageMap(Integer.toString(i));
+ }
+ Integer cacheIndex = 0;
+ for (IPageMap pageMap : tester.getWicketSession().getPageMaps())
+ {
+ assertEquals(cacheIndex++, Integer.valueOf(pageMap.getName()));
+ }
+ }
+
+ /**
+ * Making sure that the dirty page map logic is marking the the page map as the last used.
+ */
+ public void testLruCacheOnDirtyPageMap()
+ {
+ goToPage(TestPage.class, "0");
+ goToPage(TestPage.class, "1");
+ goToPage(TestPage.class, "2");
+ goToPage(TestPage.class, "1");
+ assertEquals(3, tester.getWicketSession().getPageMaps().size());
+ assertEquals("0", tester.getWicketSession().getPageMaps().get(0).getName());
+ assertEquals("1", tester.getWicketSession().getPageMaps().get(2).getName());
+ }
+
private void goToPage(Class pageClass, String pageMap)
{
WebRequestCycle cycle = tester.setupRequestAndResponse();