You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by mg...@apache.org on 2014/03/13 10:06:58 UTC
[5/9] git commit: WICKET-5527 Inefficient
DefaultPageStore.SerializedPagesCache
WICKET-5527 Inefficient DefaultPageStore.SerializedPagesCache
Add tests and fix bugs in the new PerSessionPageStore impl
Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/1c31627e
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/1c31627e
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/1c31627e
Branch: refs/heads/master
Commit: 1c31627e4bfc167bc6e2e29bd642991d6b5a1dcb
Parents: 25016d1
Author: Martin Tzvetanov Grigorov <mg...@apache.org>
Authored: Tue Mar 11 12:17:55 2014 +0200
Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
Committed: Tue Mar 11 12:17:55 2014 +0200
----------------------------------------------------------------------
.../wicket/pageStore/DefaultPageStore.java | 46 ++++---
.../wicket/pageStore/PerSessionPageStore.java | 52 +++++---
.../wicket/pageStore/AbstractPageStoreTest.java | 131 +++++++++++++++++++
.../wicket/pageStore/DefaultPageStoreTest.java | 31 +++++
.../apache/wicket/pageStore/NoopDataStore.java | 61 +++++++++
.../pageStore/PerSessionPageStoreTest.java | 53 ++++++++
6 files changed, 336 insertions(+), 38 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/wicket/blob/1c31627e/wicket-core/src/main/java/org/apache/wicket/pageStore/DefaultPageStore.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/pageStore/DefaultPageStore.java b/wicket-core/src/main/java/org/apache/wicket/pageStore/DefaultPageStore.java
index 393fac3..e574f37 100644
--- a/wicket-core/src/main/java/org/apache/wicket/pageStore/DefaultPageStore.java
+++ b/wicket-core/src/main/java/org/apache/wicket/pageStore/DefaultPageStore.java
@@ -303,18 +303,19 @@ public class DefaultPageStore extends AbstractCachingPageStore<DefaultPageStore.
*/
static class SerializedPagesCache implements SecondLevelPageCache<String, Integer, SerializedPage>
{
- private final int size;
+ private final int maxSize;
private final ConcurrentLinkedDeque<SoftReference<SerializedPage>> cache;
/**
- * Construct.
+ * Constructor.
*
- * @param size
+ * @param maxSize
+ * The maximum number of entries to cache
*/
- public SerializedPagesCache(final int size)
+ public SerializedPagesCache(final int maxSize)
{
- this.size = size;
+ this.maxSize = maxSize;
cache = new ConcurrentLinkedDeque<>();
}
@@ -327,17 +328,18 @@ public class DefaultPageStore extends AbstractCachingPageStore<DefaultPageStore.
@Override
public SerializedPage removePage(final String sessionId, final Integer pageId)
{
- if (size > 0)
+ if (maxSize > 0)
{
Args.notNull(sessionId, "sessionId");
Args.notNull(pageId, "pageId");
+ SerializedPage sample = new SerializedPage(sessionId, pageId, null);
+
for (Iterator<SoftReference<SerializedPage>> i = cache.iterator(); i.hasNext();)
{
SoftReference<SerializedPage> ref = i.next();
SerializedPage entry = ref.get();
- if (entry != null && entry.getPageId() == pageId &&
- entry.getSessionId().equals(sessionId))
+ if (sample.equals(entry))
{
i.remove();
return entry;
@@ -356,7 +358,7 @@ public class DefaultPageStore extends AbstractCachingPageStore<DefaultPageStore.
@Override
public void removePages(String sessionId)
{
- if (size > 0)
+ if (maxSize > 0)
{
Args.notNull(sessionId, "sessionId");
@@ -385,17 +387,18 @@ public class DefaultPageStore extends AbstractCachingPageStore<DefaultPageStore.
public SerializedPage getPage(String sessionId, Integer pageId)
{
SerializedPage result = null;
- if (size > 0)
+ if (maxSize > 0)
{
Args.notNull(sessionId, "sessionId");
Args.notNull(pageId, "pageId");
+ SerializedPage sample = new SerializedPage(sessionId, pageId, null);
+
for (Iterator<SoftReference<SerializedPage>> i = cache.iterator(); i.hasNext();)
{
SoftReference<SerializedPage> ref = i.next();
SerializedPage entry = ref.get();
- if (entry != null && entry.getPageId() == pageId &&
- entry.getSessionId().equals(sessionId))
+ if (sample.equals(entry))
{
i.remove();
result = entry;
@@ -406,7 +409,7 @@ public class DefaultPageStore extends AbstractCachingPageStore<DefaultPageStore.
if (result != null)
{
// move to top
- storePage(sessionId, pageId, result);
+ internalStore(result);
}
}
return result;
@@ -421,7 +424,7 @@ public class DefaultPageStore extends AbstractCachingPageStore<DefaultPageStore.
@Override
public void storePage(String sessionId, Integer pageId, SerializedPage page)
{
- if (size > 0)
+ if (maxSize > 0)
{
Args.notNull(sessionId, "sessionId");
Args.notNull(pageId, "pageId");
@@ -438,11 +441,16 @@ public class DefaultPageStore extends AbstractCachingPageStore<DefaultPageStore.
}
}
- cache.add(new SoftReference<>(page));
- while (cache.size() > size)
- {
- cache.remove(0);
- }
+ internalStore(page);
+ }
+ }
+
+ private void internalStore(SerializedPage page)
+ {
+ cache.push(new SoftReference<>(page));
+ while (cache.size() > maxSize)
+ {
+ cache.pollLast();
}
}
http://git-wip-us.apache.org/repos/asf/wicket/blob/1c31627e/wicket-core/src/main/java/org/apache/wicket/pageStore/PerSessionPageStore.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/pageStore/PerSessionPageStore.java b/wicket-core/src/main/java/org/apache/wicket/pageStore/PerSessionPageStore.java
index 8d0c631..eaff550 100644
--- a/wicket-core/src/main/java/org/apache/wicket/pageStore/PerSessionPageStore.java
+++ b/wicket-core/src/main/java/org/apache/wicket/pageStore/PerSessionPageStore.java
@@ -18,6 +18,7 @@ package org.apache.wicket.pageStore;
import java.lang.ref.SoftReference;
import java.util.Comparator;
+import java.util.Iterator;
import java.util.Map;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ConcurrentSkipListMap;
@@ -25,7 +26,6 @@ import java.util.concurrent.ConcurrentSkipListMap;
import org.apache.wicket.page.IManageablePage;
import org.apache.wicket.serialize.ISerializer;
import org.apache.wicket.util.lang.Args;
-import org.apache.wicket.util.time.Time;
/**
* A page store that uses a SecondLevelPageCache with the last N used page instances
@@ -75,6 +75,8 @@ public class PerSessionPageStore extends AbstractCachingPageStore<IManageablePag
}
/**
+ * An implementation of SecondLevelPageCache that stores the last used N live page instances
+ * per http session.
*/
protected static class PagesCache implements SecondLevelPageCache<String, Integer, IManageablePage>
{
@@ -92,7 +94,7 @@ public class PerSessionPageStore extends AbstractCachingPageStore<IManageablePag
/**
* The last time this page has been used/accessed.
*/
- private Time accessTime;
+ private long accessTime;
private PageValue(IManageablePage page)
{
@@ -102,7 +104,7 @@ public class PerSessionPageStore extends AbstractCachingPageStore<IManageablePag
private PageValue(int pageId)
{
this.pageId = pageId;
- this.accessTime = Time.now();
+ this.accessTime = System.nanoTime();
}
@Override
@@ -114,7 +116,6 @@ public class PerSessionPageStore extends AbstractCachingPageStore<IManageablePag
PageValue pageValue = (PageValue) o;
return pageId == pageValue.pageId;
-
}
@Override
@@ -129,7 +130,7 @@ public class PerSessionPageStore extends AbstractCachingPageStore<IManageablePag
@Override
public int compare(PageValue p1, PageValue p2)
{
- return p1.accessTime.compareTo(p2.accessTime);
+ return Long.valueOf(p1.accessTime).compareTo(p2.accessTime);
}
}
@@ -173,11 +174,17 @@ public class PerSessionPageStore extends AbstractCachingPageStore<IManageablePag
ConcurrentMap<PageValue, IManageablePage> pages = pagesPerSession.get();
if (pages != null)
{
- PageValue pv = new PageValue(pageId);
- IManageablePage page = pages.remove(pv);
- if (page != null)
+ PageValue sample = new PageValue(pageId);
+ Iterator<Map.Entry<PageValue, IManageablePage>> iterator = pages.entrySet().iterator();
+ while (iterator.hasNext())
{
- result = page;
+ Map.Entry<PageValue, IManageablePage> entry = iterator.next();
+ if (sample.equals(entry.getKey()))
+ {
+ result = entry.getValue();
+ iterator.remove();
+ break;
+ }
}
}
}
@@ -231,14 +238,19 @@ public class PerSessionPageStore extends AbstractCachingPageStore<IManageablePag
ConcurrentSkipListMap<PageValue, IManageablePage> pages = pagesPerSession.get();
if (pages != null)
{
- PageValue pv = new PageValue(pageId);
- Map.Entry<PageValue, IManageablePage> entry = pages.ceilingEntry(pv);
-
- if (entry != null)
+ PageValue sample = new PageValue(pageId);
+ Iterator<Map.Entry<PageValue,IManageablePage>> iterator = pages.entrySet().iterator();
+ while (iterator.hasNext())
{
- // touch the entry
- entry.getKey().accessTime = Time.now();
- result = entry.getValue();
+ Map.Entry<PageValue, IManageablePage> entry = iterator.next();
+
+ if (sample.equals(entry.getKey()))
+ {
+ // touch the entry
+ entry.getKey().accessTime = System.nanoTime();
+ result = entry.getValue();
+ break;
+ }
}
}
}
@@ -286,13 +298,15 @@ public class PerSessionPageStore extends AbstractCachingPageStore<IManageablePag
if (pages != null)
{
+ removePage(sessionId, pageId);
+
+ PageValue pv = new PageValue(page);
+ pages.put(pv, page);
+
while (pages.size() > maxEntriesPerSession)
{
pages.pollFirstEntry();
}
-
- PageValue pv = new PageValue(page);
- pages.put(pv, page);
}
}
}
http://git-wip-us.apache.org/repos/asf/wicket/blob/1c31627e/wicket-core/src/test/java/org/apache/wicket/pageStore/AbstractPageStoreTest.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/pageStore/AbstractPageStoreTest.java b/wicket-core/src/test/java/org/apache/wicket/pageStore/AbstractPageStoreTest.java
new file mode 100644
index 0000000..42cce39
--- /dev/null
+++ b/wicket-core/src/test/java/org/apache/wicket/pageStore/AbstractPageStoreTest.java
@@ -0,0 +1,131 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.pageStore;
+
+import org.apache.wicket.MockPage;
+import org.apache.wicket.serialize.ISerializer;
+import org.apache.wicket.serialize.java.JavaSerializer;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public abstract class AbstractPageStoreTest extends Assert
+{
+ protected final String sessionId = "1234567890";
+ protected final int pageId = 123;
+ protected final ISerializer serializer = new JavaSerializer(DefaultPageStore.class.getName());
+ protected final IDataStore dataStore = new NoopDataStore();
+ protected int maxEntries = 1;
+ protected IPageStore pageStore = null;
+
+ @Before
+ public void before()
+ {
+ pageStore = createPageStore(serializer, dataStore, maxEntries);
+ }
+
+ protected abstract IPageStore createPageStore(ISerializer serializer, IDataStore dataStore, int maxEntries);
+
+ @After
+ public void after()
+ {
+ if (pageStore != null)
+ {
+ pageStore.destroy();
+ pageStore = null;
+ }
+ }
+
+ /**
+ * Assert that a stored page is available to be read
+ */
+ @Test
+ public void storePage()
+ {
+ pageStore.storePage(sessionId, new MockPage(pageId));
+
+ assertNotNull(pageStore.getPage(sessionId, pageId));
+ }
+
+ /**
+ * Assert that storing a page twice won't keep two entries
+ */
+ @Test
+ public void storePage2()
+ {
+ int maxEntries = 10;
+
+ pageStore = createPageStore(serializer, dataStore, maxEntries);
+
+ pageStore.storePage(sessionId, new MockPage(pageId));
+ pageStore.storePage(sessionId, new MockPage(pageId));
+
+ assertNotNull(pageStore.getPage(sessionId, pageId));
+
+ pageStore.removePage(sessionId, pageId);
+
+ assertNull(pageStore.getPage(sessionId, pageId));
+ }
+
+ @Test
+ public void removePage()
+ {
+ pageStore.storePage(sessionId, new MockPage(pageId));
+
+ assertNotNull(pageStore.getPage(sessionId, pageId));
+
+ pageStore.removePage(sessionId, pageId);
+
+ assertNull(pageStore.getPage(sessionId, pageId));
+ }
+
+ /**
+ * Verify that at most {@code maxEntries} per session can be put in the cache
+ */
+ @Test
+ public void maxSizeSameSession()
+ {
+ pageStore.storePage(sessionId, new MockPage(pageId));
+
+ assertNotNull(pageStore.getPage(sessionId, pageId));
+
+ int pageId2 = 234;
+ pageStore.storePage(sessionId, new MockPage(pageId2));
+ assertNull(pageStore.getPage(sessionId, pageId));
+ assertNotNull(pageStore.getPage(sessionId, pageId2));
+ }
+
+ /**
+ * Verify that it is OK to store more pages than {@code maxEntries}
+ * if they are in different sessions
+ */
+ @Test
+ public void maxSizeDifferentSessions()
+ {
+ String sessionId2 = "0987654321";
+
+ pageStore.storePage(sessionId, new MockPage(pageId));
+
+ assertNotNull(pageStore.getPage(sessionId, pageId));
+
+ pageStore.storePage(sessionId2, new MockPage(pageId));
+
+ assertNull(pageStore.getPage(sessionId, pageId));
+ assertNotNull(pageStore.getPage(sessionId2, pageId));
+ }
+}
http://git-wip-us.apache.org/repos/asf/wicket/blob/1c31627e/wicket-core/src/test/java/org/apache/wicket/pageStore/DefaultPageStoreTest.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/pageStore/DefaultPageStoreTest.java b/wicket-core/src/test/java/org/apache/wicket/pageStore/DefaultPageStoreTest.java
new file mode 100644
index 0000000..2ffa376
--- /dev/null
+++ b/wicket-core/src/test/java/org/apache/wicket/pageStore/DefaultPageStoreTest.java
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.pageStore;
+
+import org.apache.wicket.serialize.ISerializer;
+
+/**
+ * Tests for DefaultPageStore
+ */
+public class DefaultPageStoreTest extends AbstractPageStoreTest
+{
+ @Override
+ protected IPageStore createPageStore(ISerializer serializer, IDataStore dataStore, int maxEntries)
+ {
+ return new DefaultPageStore(serializer, dataStore, maxEntries);
+ }
+}
http://git-wip-us.apache.org/repos/asf/wicket/blob/1c31627e/wicket-core/src/test/java/org/apache/wicket/pageStore/NoopDataStore.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/pageStore/NoopDataStore.java b/wicket-core/src/test/java/org/apache/wicket/pageStore/NoopDataStore.java
new file mode 100644
index 0000000..0cadaee
--- /dev/null
+++ b/wicket-core/src/test/java/org/apache/wicket/pageStore/NoopDataStore.java
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.pageStore;
+
+/**
+ * An implementation of IDataStore that does nothing
+ */
+public class NoopDataStore implements IDataStore
+{
+ @Override
+ public byte[] getData(String sessionId, int id)
+ {
+ return null;
+ }
+
+ @Override
+ public void removeData(String sessionId, int id)
+ {
+ }
+
+ @Override
+ public void removeData(String sessionId)
+ {
+ }
+
+ @Override
+ public void storeData(String sessionId, int id, byte[] data)
+ {
+ }
+
+ @Override
+ public void destroy()
+ {
+ }
+
+ @Override
+ public boolean isReplicated()
+ {
+ return false;
+ }
+
+ @Override
+ public boolean canBeAsynchronous()
+ {
+ return false;
+ }
+}
http://git-wip-us.apache.org/repos/asf/wicket/blob/1c31627e/wicket-core/src/test/java/org/apache/wicket/pageStore/PerSessionPageStoreTest.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/pageStore/PerSessionPageStoreTest.java b/wicket-core/src/test/java/org/apache/wicket/pageStore/PerSessionPageStoreTest.java
new file mode 100644
index 0000000..0cd4a2a
--- /dev/null
+++ b/wicket-core/src/test/java/org/apache/wicket/pageStore/PerSessionPageStoreTest.java
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.pageStore;
+
+import org.apache.wicket.MockPage;
+import org.apache.wicket.serialize.ISerializer;
+import org.junit.Test;
+
+/**
+ * Tests for PerSessionPageStore
+ */
+public class PerSessionPageStoreTest extends AbstractPageStoreTest
+{
+ @Override
+ protected IPageStore createPageStore(ISerializer serializer, IDataStore dataStore, int maxEntries)
+ {
+ return new PerSessionPageStore(serializer, dataStore, maxEntries);
+ }
+
+ /**
+ * Verify that it is OK to store more pages than {@code maxEntries}
+ * if they are in different sessions
+ */
+ @Test
+ @Override
+ public void maxSizeDifferentSessions()
+ {
+ String sessionId2 = "0987654321";
+
+ pageStore.storePage(sessionId, new MockPage(pageId));
+
+ assertNotNull(pageStore.getPage(sessionId, pageId));
+
+ pageStore.storePage(sessionId2, new MockPage(pageId));
+
+ assertNotNull(pageStore.getPage(sessionId, pageId));
+ assertNotNull(pageStore.getPage(sessionId2, pageId));
+ }
+}