Very interesting, I think the patch would be even better if the caching policy was configurable. ie, LRU, MRU, FIFO etc. To take it beyond that, the cache should be configurable per context (see last note).

your test case is a little out there, (who would map .html to cached JSPs:) but it proves your point.

I'm a +0 on this patch as I am currently not involved in the jasper code, but would be willing (+1) to help out creating a more complete patch, as I think Yarick brings out a great point.

In a shared hosted environments, means that one user could take up almost all memory space for JSP pages that are never accessed.

I'd like some thoughts from the Jasper developers.

thanks
Filip


Yaroslav Sokolov wrote:
Hi,

I have found, that once loaded jsp-servlets are never unloaded.

To test I just configured tomcat to process *.html files by JspServlet
and then traversed jdk documentation. The result was not very exciting -
after browsing ~ 150 pages tomcat cried "java.lang.OutOfMemoryError: Java heap space"
and started not to work...

So maybe it would be not a bad idea to try to keep in memeory just some fixed
number of jsp-servlets ?

I have written a sample implementation of such a policy, but it is not very elegant as internal structure containing jsp-servlets, it seems, was not designed for such actions...

Regards,
Yarick.
------------------------------------------------------------------------

diff -rdu 
apache-tomcat-5.5.15-src-orig/jasper/jasper2/src/share/org/apache/jasper/EmbeddedServletOptions.java
 
apache-tomcat-5.5.15-src-patched/jasper/jasper2/src/share/org/apache/jasper/EmbeddedServletOptions.java
--- 
apache-tomcat-5.5.15-src-orig/jasper/jasper2/src/share/org/apache/jasper/EmbeddedServletOptions.java
        2006-01-03 10:14:04.000000000 +0100
+++ 
apache-tomcat-5.5.15-src-patched/jasper/jasper2/src/share/org/apache/jasper/EmbeddedServletOptions.java
     2006-02-21 13:26:44.984221000 +0100
@@ -174,6 +174,17 @@
      */
     private boolean xpoweredBy;
+ /**
+     * The maxim number of loaded jsps per web-application. If there are more
+     * jsps loaded, they will be unloaded.
+     */
+    private int maxLoadedJsps = 20;
+
+    /**
+     * How often it is tryed to unload jsps (in seconds)
+     */
+    private int jspUnloadTestInterval = 4;
+
     public String getProperty(String name ) {
         return settings.getProperty( name );
     }
@@ -355,6 +366,14 @@
         return null;
     }
+ public int getMaxLoadedJsps() {
+        return maxLoadedJsps;
+    }
+
+    public int getJspUnloadTestInterval() {
+        return jspUnloadTestInterval;
+    }
+
     /**
      * Create an EmbeddedServletOptions object using data available from
* ServletConfig and ServletContext. @@ -636,6 +655,40 @@
             }
         }
+ String maxLoadedJsps = config.getInitParameter("maxLoadedJsps");
+        if (maxLoadedJsps != null) {
+            try {
+                this.maxLoadedJsps = Integer.parseInt(maxLoadedJsps);
+                if (this.maxLoadedJsps <= 0) {
+                    this.maxLoadedJsps = 20;
+                    if (log.isWarnEnabled()) {
+                        log.warn(Localizer.getMessage("jsp.warning.maxLoadedJsps", 
""+this.maxLoadedJsps));
+                    }
+                }
+            } catch(NumberFormatException ex) {
+                if (log.isWarnEnabled()) {
+                    log.warn(Localizer.getMessage("jsp.warning.maxLoadedJsps", 
""+this.maxLoadedJsps));
+                }
+            }
+        }
+
+        String jspUnloadTestInterval = 
config.getInitParameter("jspUnloadTestInterval");
+        if (jspUnloadTestInterval != null) {
+            try {
+                this.jspUnloadTestInterval = 
Integer.parseInt(jspUnloadTestInterval);
+                if (this.jspUnloadTestInterval <= 0) {
+                    this.jspUnloadTestInterval = 4;
+                    if (log.isWarnEnabled()) {
+                        
log.warn(Localizer.getMessage("jsp.warning.jspUnloadTestInterval",""+this.jspUnloadTestInterval));
+                    }
+                }
+            } catch(NumberFormatException ex) {
+                if (log.isWarnEnabled()) {
+                    
log.warn(Localizer.getMessage("jsp.warning.jspUnloadTestInterval",""+this.jspUnloadTestInterval));
+                }
+            }
+        }
+
         // Setup the global Tag Libraries location cache for this
         // web-application.
         tldLocationsCache = new TldLocationsCache(context);
diff -rdu 
apache-tomcat-5.5.15-src-orig/jasper/jasper2/src/share/org/apache/jasper/JspC.java
 
apache-tomcat-5.5.15-src-patched/jasper/jasper2/src/share/org/apache/jasper/JspC.java
--- 
apache-tomcat-5.5.15-src-orig/jasper/jasper2/src/share/org/apache/jasper/JspC.java
  2006-01-03 10:14:04.000000000 +0100
+++ 
apache-tomcat-5.5.15-src-patched/jasper/jasper2/src/share/org/apache/jasper/JspC.java
       2006-02-21 13:27:07.568387400 +0100
@@ -448,6 +448,14 @@
         return cache;
     }
+ public int getMaxLoadedJsps() {
+        return 0;
+    }
+
+    public int getJspUnloadTestInterval() {
+        return 0;
+    }
+
     /**
      * Background compilation check intervals in seconds
      */
diff -rdu 
apache-tomcat-5.5.15-src-orig/jasper/jasper2/src/share/org/apache/jasper/Options.java
 
apache-tomcat-5.5.15-src-patched/jasper/jasper2/src/share/org/apache/jasper/Options.java
--- 
apache-tomcat-5.5.15-src-orig/jasper/jasper2/src/share/org/apache/jasper/Options.java
       2006-01-03 10:14:04.000000000 +0100
+++ 
apache-tomcat-5.5.15-src-patched/jasper/jasper2/src/share/org/apache/jasper/Options.java
    2006-02-21 13:27:24.210173000 +0100
@@ -184,5 +184,16 @@
      * @return the Map(String uri, TreeNode tld) instance.
      */
     public Map getCache();
+
+    /**
+     * The maxim number of loaded jsps per web-application. If there are more
+     * jsps loaded, they will be unloaded.
+     */
+    public int getMaxLoadedJsps();
+
+    /**
+     * How often it is tryed to unload jsps (in seconds)
+     */
+    public int getJspUnloadTestInterval();
}
diff -rdu 
apache-tomcat-5.5.15-src-orig/jasper/jasper2/src/share/org/apache/jasper/compiler/JspRuntimeContext.java
 
apache-tomcat-5.5.15-src-patched/jasper/jasper2/src/share/org/apache/jasper/compiler/JspRuntimeContext.java
--- 
apache-tomcat-5.5.15-src-orig/jasper/jasper2/src/share/org/apache/jasper/compiler/JspRuntimeContext.java
    2006-01-03 10:14:02.000000000 +0100
+++ 
apache-tomcat-5.5.15-src-patched/jasper/jasper2/src/share/org/apache/jasper/compiler/JspRuntimeContext.java
 2006-02-21 13:16:29.004201800 +0100
@@ -25,10 +25,7 @@
 import java.security.PermissionCollection;
 import java.security.Policy;
 import java.security.cert.Certificate;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.Map;
+import java.util.*; // yarick: java.util.HashMap -> java.util.*
import javax.servlet.ServletContext;
 import javax.servlet.jsp.JspFactory;
@@ -148,8 +145,8 @@
     /**
      * Maps JSP pages to their JspServletWrapper's
      */
-    private Map jsps = Collections.synchronizedMap( new HashMap());
- + private Map jsps = Collections.synchronizedMap( new LinkedHashMap( 16, 0.75f, true ) ); // yarick: HashMap -> LinkedHashMap
+
/**
      * The background thread.
@@ -192,6 +189,21 @@
     }
/**
+     * Get an already existing JspServletWrapper and increases services count.
+     *
+     * @param jspUri JSP URI
+     * @return JspServletWrapper for JSP
+     */
+    public JspServletWrapper getWrapperAndIncService(String jspUri) {
+        JspServletWrapper jswr;
+        synchronized( jsps ) {
+            jswr = (JspServletWrapper) jsps.get(jspUri);
+            if( null != jswr )  jswr.incService();
+        }
+        return jswr;
+    }
+
+    /**
      * Remove a  JspServletWrapper.
      *
      * @param jspUri JSP URI of JspServletWrapper to remove
@@ -521,4 +533,28 @@
} +// { inserted by Yarick
+
+    /** must be called from synchronized by [EMAIL PROTECTED] 
org.apache.jasper.servlet.JspServlet} context */
+    public JspServletWrapper getJspForUnload()
+    {
+       int MAX_UNLOADABLE_JSPS = 10;
+       if( jsps.size() > MAX_UNLOADABLE_JSPS ) {
+           JspServletWrapper jsw = null;
+           synchronized( jsps ) {
+               Iterator it = jsps.entrySet().iterator();
+               for( int rest_jsps=jsps.size(); rest_jsps > MAX_UNLOADABLE_JSPS 
&& it.hasNext(); --rest_jsps ) {
+                   jsw = (JspServletWrapper) ( (Map.Entry) it.next() 
).getValue();
+                   if( jsw.getExecutingServicesCount() == 0 && 
!jsw.isTagFile() ) {
+                       it.remove();
+                       return jsw;
+                   }
+               }
+           }
+       }
+       return null;
+    }
+
+// } inserted by Yarick
+
 }
diff -rdu 
apache-tomcat-5.5.15-src-orig/jasper/jasper2/src/share/org/apache/jasper/servlet/JspServlet.java
 
apache-tomcat-5.5.15-src-patched/jasper/jasper2/src/share/org/apache/jasper/servlet/JspServlet.java
--- 
apache-tomcat-5.5.15-src-orig/jasper/jasper2/src/share/org/apache/jasper/servlet/JspServlet.java
    2006-01-03 10:14:02.000000000 +0100
+++ 
apache-tomcat-5.5.15-src-patched/jasper/jasper2/src/share/org/apache/jasper/servlet/JspServlet.java
 2006-02-21 13:37:08.060784200 +0100
@@ -17,8 +17,9 @@
 package org.apache.jasper.servlet;
import java.io.IOException;
+import java.io.File;
 import java.lang.reflect.Constructor;
-import java.util.Enumeration;
+import java.util.*;
import javax.servlet.ServletConfig;
 import javax.servlet.ServletContext;
@@ -97,8 +98,10 @@
             options = new EmbeddedServletOptions(config, context);
         }
         rctxt = new JspRuntimeContext(context, options);
- - if (log.isDebugEnabled()) {
+
+       startUnloadJspsThread();  // inserted by yarick
+
+       if (log.isDebugEnabled()) {
             log.debug(Localizer.getMessage("jsp.message.scratch.dir.is",
                     options.getScratchDir().toString()));
             
log.debug(Localizer.getMessage("jsp.message.dont.modify.servlets"));
@@ -278,7 +281,7 @@
         if (log.isDebugEnabled()) {
             log.debug("JspServlet.destroy()");
         }
-
+       stopUnloadJspsThread();  // inserted by yarick
         rctxt.destroy();
     }
@@ -290,29 +293,99 @@
                                 Throwable exception, boolean precompile)
         throws ServletException, IOException {
- JspServletWrapper wrapper =
-            (JspServletWrapper) rctxt.getWrapper(jspUri);
-        if (wrapper == null) {
-            synchronized(this) {
-                wrapper = (JspServletWrapper) rctxt.getWrapper(jspUri);
-                if (wrapper == null) {
-                    // Check if the requested JSP page exists, to avoid
-                    // creating unnecessary directories and files.
-                    if (null == context.getResource(jspUri)) {
-                        response.sendError(HttpServletResponse.SC_NOT_FOUND,
-                                           jspUri);
-                        return;
+       JspServletWrapper wrapper = null;
+        try {
+            wrapper = (JspServletWrapper) 
rctxt.getWrapperAndIncService(jspUri);
+            if( null == wrapper )
+                synchronized(this) {
+                    wrapper = (JspServletWrapper) 
rctxt.getWrapperAndIncService(jspUri);
+                    if (wrapper == null) {
+                        // Check if the requested JSP page exists, to avoid
+                        // creating unnecessary directories and files.
+                        if (null == context.getResource(jspUri)) {
+                            
response.sendError(HttpServletResponse.SC_NOT_FOUND,
+                                               jspUri);
+                            return;
+                        }
+                        boolean isErrorPage = exception != null;
+                        wrapper = new JspServletWrapper(config, options, 
jspUri,
+                                                        isErrorPage, rctxt);
+                        rctxt.addWrapper(jspUri,wrapper);
                     }
-                    boolean isErrorPage = exception != null;
-                    wrapper = new JspServletWrapper(config, options, jspUri,
-                                                    isErrorPage, rctxt);
-                    rctxt.addWrapper(jspUri,wrapper);
+                    wrapper.incService();
+                }
+
+            /* dances around making it not possible to call servlet.init() 
before
+             * previous copy of servlet is unloaded ( called method 
servlet.destroy() )
+             */
+            String _destroyingUri = destroyingUri;
+            if( wrapper.getJspUri().equals( _destroyingUri ) )
+                synchronized( _destroyingUri )  {
+                    if( _destroyingUri == destroyingUri )
+                        _destroyingUri.wait();
                 }
+
+            wrapper.service(request, response, precompile);
+
+        } catch( InterruptedException ignore ) {}
+        finally {  // inserted by yarick
+           synchronized(this)  {
+                if( null != wrapper ) wrapper.decService();
             }
-        }
+       }
+    }
- wrapper.service(request, response, precompile);
+// { inserted by yarick
+    private Thread unloadThread;
+    private String destroyingUri;
+ protected void startUnloadJspsThread() {
+        String pathToWebApp = context.getRealPath("/");
+        if( pathToWebApp.endsWith( File.separator ) )
+            pathToWebApp = pathToWebApp.substring( 0, pathToWebApp.length()-1 
);
+
+        unloadThread = new Thread( "jspUnloader ["+pathToWebApp.substring( 
pathToWebApp.lastIndexOf( File.separatorChar ) )+( "/" )+"]" ) {
+           public void run()  {  runUnloadJspsThread();  }
+       };
+       unloadThread.setDaemon( true );
+       unloadThread.start();
+    }
+
+    protected void stopUnloadJspsThread() {
+       if( null != unloadThread ) {
+           unloadThread.interrupt();
+            try { unloadThread.join( 10 * 1000 ); } // wait maximum 10 seconds
+            catch( InterruptedException ignore ) {}
+            unloadThread = null;
+       }
     }
+ protected void runUnloadJspsThread() {
+       try {
+           while( !Thread.currentThread().isInterrupted() ) {
+               JspServletWrapper jsw;
+               synchronized( this ) {
+                   jsw = rctxt.getJspForUnload();
+                   if( null != jsw )  destroyingUri = new String( 
jsw.getJspUri() );
+                }
+
+               if( null == jsw )
+                    Thread.sleep( options.getJspUnloadTestInterval() * 1000 );
+                else
+                    /* dances around making it not possible to call 
servlet.init() before
+                     * previous copy of servlet is unloaded ( called method 
servlet.destroy() )
+                     */
+                    synchronized( destroyingUri ) {
+                        try {
+                              jsw.destroy();
+                        } finally {
+                            String prev_destroyingUri = destroyingUri;
+                            destroyingUri = null;
+                            prev_destroyingUri.notifyAll();
+                        }
+                    }
+            }
+       } catch( InterruptedException exit ) {}
+    }
+// } inserted by yarick
 }
diff -rdu 
apache-tomcat-5.5.15-src-orig/jasper/jasper2/src/share/org/apache/jasper/servlet/JspServletWrapper.java
 
apache-tomcat-5.5.15-src-patched/jasper/jasper2/src/share/org/apache/jasper/servlet/JspServletWrapper.java
--- 
apache-tomcat-5.5.15-src-orig/jasper/jasper2/src/share/org/apache/jasper/servlet/JspServletWrapper.java
     2006-01-03 10:14:04.000000000 +0100
+++ 
apache-tomcat-5.5.15-src-patched/jasper/jasper2/src/share/org/apache/jasper/servlet/JspServletWrapper.java
  2006-02-21 13:24:43.999843400 +0100
@@ -86,6 +86,7 @@
     private JasperException compileException;
     private long servletClassLastModifiedTime;
     private long lastModificationTest = 0L;
+    private int  executingServicesCount; // yarick
/*
      * JspServletWrapper for JSP pages.
@@ -397,6 +398,10 @@
         }
     }
+ synchronized public void incService() { ++executingServicesCount; } // yarick: inserted accounting of 'service'
+    synchronized public void decService()  {  --executingServicesCount;  }  // 
yarick: inserted accounting of 'service'
+    String getJspUri()                     {  return jspUri;  }  // yarick: 
inserted
+
     public void destroy() {
         if (theServlet != null) {
             theServlet.destroy();
@@ -416,6 +421,9 @@
         this.lastModificationTest = lastModificationTest;
     }
+ /** @return Returns count of currently executing of method [EMAIL PROTECTED] #service} */
+    public int  getExecutingServicesCount() { return executingServicesCount; } 
// yarick
+
     /**
      * <p>Attempts to construct a JasperException that contains helpful 
information
      * about what went wrong. Uses the JSP compiler system to translate the 
line

------------------------------------------------------------------------

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to