Author: hlship
Date: Thu Sep  8 20:33:57 2011
New Revision: 1166889

URL: http://svn.apache.org/viewvc?rev=1166889&view=rev
Log:
TAP5-1637: Use SoftReferences to cache Page instances, rather than an explicit 
janitor job

Modified:
    
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/SymbolConstants.java
    
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PageSourceImpl.java
    
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java
    
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/services/AppModule.java

Modified: 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/SymbolConstants.java
URL: 
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/SymbolConstants.java?rev=1166889&r1=1166888&r2=1166889&view=diff
==============================================================================
--- 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/SymbolConstants.java
 (original)
+++ 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/SymbolConstants.java
 Thu Sep  8 20:33:57 2011
@@ -313,23 +313,6 @@ public class SymbolConstants
     public static final String CLUSTERED_SESSIONS = 
"tapestry.clustered-sessions";
 
     /**
-     * The interval (in milliseconds) at which the {@link 
org.apache.tapestry5.internal.services.PageSource} checks for
-     * inactive pages that can be discarded. The default is "5 m" (every five 
minutes).
-     *
-     * @since 5.3
-     */
-    public static final String PAGE_SOURCE_CHECK_INTERVAL = 
"tapestry.page-source-check-interval";
-
-    /**
-     * The maximum amount of time, in milliseconds, that an instantiated page 
instance may be kept in memory
-     * before being discarded. The frequency of checks for such pages is 
determined by {@link #PAGE_SOURCE_CHECK_INTERVAL}.
-     * The default is "15 m" (fifteen minutes).
-     *
-     * @since 5.3
-     */
-    public static final String PAGE_SOURCE_ACTIVE_WINDOW = 
"tapestry.page-source-active-window";
-
-    /**
      * The fix for <a 
href="https://issues.apache.org/jira/browse/TAP5-1596";>TAP5-1596</a> means that 
component ids referenced
      * by event handler methods (either the naming convention, or the {@link 
org.apache.tapestry5.annotations.OnEvent} annotation)
      * can cause a page load error if there is no matching component in the 
component's template. Although this is correct behavior,

Modified: 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PageSourceImpl.java
URL: 
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PageSourceImpl.java?rev=1166889&r1=1166888&r2=1166889&view=diff
==============================================================================
--- 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PageSourceImpl.java
 (original)
+++ 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/PageSourceImpl.java
 Thu Sep  8 20:33:57 2011
@@ -14,22 +14,13 @@
 
 package org.apache.tapestry5.internal.services;
 
-import org.apache.tapestry5.SymbolConstants;
 import org.apache.tapestry5.internal.structure.Page;
-import org.apache.tapestry5.ioc.annotations.IntermediateType;
-import org.apache.tapestry5.ioc.annotations.PostInjection;
-import org.apache.tapestry5.ioc.annotations.Symbol;
 import org.apache.tapestry5.ioc.internal.util.CollectionFactory;
-import org.apache.tapestry5.ioc.services.cron.IntervalSchedule;
-import org.apache.tapestry5.ioc.services.cron.PeriodicExecutor;
-import org.apache.tapestry5.ioc.util.TimeInterval;
 import org.apache.tapestry5.services.InvalidationListener;
 import org.apache.tapestry5.services.pageload.ComponentRequestSelectorAnalyzer;
 import org.apache.tapestry5.services.pageload.ComponentResourceSelector;
-import org.slf4j.Logger;
 
-import java.util.Date;
-import java.util.Iterator;
+import java.lang.ref.SoftReference;
 import java.util.Map;
 
 public class PageSourceImpl implements PageSource, InvalidationListener
@@ -38,10 +29,6 @@ public class PageSourceImpl implements P
 
     private final PageLoader pageLoader;
 
-    private final long activeWindow;
-
-    private final Logger logger;
-
     private static final class CachedPageKey
     {
         final String pageName;
@@ -73,91 +60,50 @@ public class PageSourceImpl implements P
         }
     }
 
-    private final Map<CachedPageKey, Page> pageCache = 
CollectionFactory.newConcurrentMap();
+    private final Map<CachedPageKey, SoftReference<Page>> pageCache = 
CollectionFactory.newConcurrentMap();
 
-    public PageSourceImpl(PageLoader pageLoader, 
ComponentRequestSelectorAnalyzer selectorAnalyzer,
-
-                          @Symbol(SymbolConstants.PAGE_SOURCE_ACTIVE_WINDOW)
-                          @IntermediateType(TimeInterval.class)
-                          long activeWindow, Logger logger)
+    public PageSourceImpl(PageLoader pageLoader, 
ComponentRequestSelectorAnalyzer selectorAnalyzer)
     {
         this.pageLoader = pageLoader;
         this.selectorAnalyzer = selectorAnalyzer;
-        this.activeWindow = activeWindow;
-        this.logger = logger;
     }
 
-    @PostInjection
-    public void startJanitor(PeriodicExecutor executor, 
@Symbol(SymbolConstants.PAGE_SOURCE_CHECK_INTERVAL)
-    @IntermediateType(TimeInterval.class)
-    long checkInterval)
+    public void objectWasInvalidated()
     {
-        executor.addJob(new IntervalSchedule(checkInterval), "PagePool 
cleanup", new Runnable()
-        {
-            public void run()
-            {
-                prune();
-            }
-        });
+        clearCache();
     }
 
-    private void prune()
+    public Page getPage(String canonicalPageName)
     {
-        Iterator<Page> iterator = pageCache.values().iterator();
+        ComponentResourceSelector selector = 
selectorAnalyzer.buildSelectorForRequest();
 
-        int count = 0;
+        CachedPageKey key = new CachedPageKey(canonicalPageName, selector);
 
-        long cutoff = System.currentTimeMillis() - activeWindow;
+        // The while loop looks superfluous, but it helps to ensure that the 
Page instance,
+        // with all of its mutable construction-time state, is properly 
published to other
+        // threads (at least, as I understand Brian Goetz's explanation, it 
should be).
 
-        while (iterator.hasNext())
+        while (true)
         {
-            Page page = iterator.next();
+            SoftReference<Page> ref = pageCache.get(key);
 
-            if (page.getLastAttachTime() <= cutoff)
-            {
-                count++;
-                iterator.remove();
+            Page page = ref == null ? null : ref.get();
 
-                logger.info(String.format("Pruned page %s (%s), not accessed 
since %s.",
-                        page.getName(),
-                        page.getSelector().toShortString(),
-                        new Date(page.getLastAttachTime())));
+            if (page != null)
+            {
+                return page;
             }
-        }
-
-        if (count > 0)
-        {
-            logger.info(String.format("Pruned %d page %s.", count, count == 1 
? "instance" : "instances"));
-        }
-    }
-
-    public synchronized void objectWasInvalidated()
-    {
-        pageCache.clear();
-    }
 
-    public Page getPage(String canonicalPageName)
-    {
-        ComponentResourceSelector selector = 
selectorAnalyzer.buildSelectorForRequest();
-
-        CachedPageKey key = new CachedPageKey(canonicalPageName, selector);
-
-        if (!pageCache.containsKey(key))
-        {
             // In rare race conditions, we may see the same page loaded 
multiple times across
             // different threads. The last built one will "evict" the others 
from the page cache,
             // and the earlier ones will be GCed.
 
-            Page page = pageLoader.loadPage(canonicalPageName, selector);
+            page = pageLoader.loadPage(canonicalPageName, selector);
 
-            pageCache.put(key, page);
-        }
-
-        // From good authority (Brian Goetz), this is the best way to ensure 
that the
-        // loaded page, with all of its semi-mutable construction-time state, 
is
-        // properly published.
+            ref = new SoftReference<Page>(page);
 
-        return pageCache.get(key);
+            pageCache.put(key, ref);
+        }
     }
 
     public void clearCache()

Modified: 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java
URL: 
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java?rev=1166889&r1=1166888&r2=1166889&view=diff
==============================================================================
--- 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java
 (original)
+++ 
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java
 Thu Sep  8 20:33:57 2011
@@ -2369,9 +2369,6 @@ public final class TapestryModule
         configuration.add(SymbolConstants.HOSTPORT, 0);
         configuration.add(SymbolConstants.HOSTPORT_SECURE, 0);
 
-        configuration.add(SymbolConstants.PAGE_SOURCE_CHECK_INTERVAL, "5 m");
-        configuration.add(SymbolConstants.PAGE_SOURCE_ACTIVE_WINDOW, "15 m");
-
         configuration.add(SymbolConstants.UNKNOWN_COMPONENT_ID_CHECK_ENABLED, 
true);
 
         configuration.add(SymbolConstants.APPLICATION_FOLDER, "");

Modified: 
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/services/AppModule.java
URL: 
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/services/AppModule.java?rev=1166889&r1=1166888&r2=1166889&view=diff
==============================================================================
--- 
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/services/AppModule.java
 (original)
+++ 
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/services/AppModule.java
 Thu Sep  8 20:33:57 2011
@@ -142,9 +142,6 @@ public class AppModule
         configuration.add(SymbolConstants.SECURE_ENABLED, "true");
 
         configuration.add("app.injected-symbol", "Symbol contributed to 
ApplicationDefaults");
-
-        configuration.add(SymbolConstants.PAGE_SOURCE_ACTIVE_WINDOW, "30s");
-        configuration.add(SymbolConstants.PAGE_SOURCE_CHECK_INTERVAL, "15s");
     }
 
     public static void contributeIgnoredPathsFilter(Configuration<String> 
configuration)


Reply via email to