You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2013/11/12 14:00:03 UTC

svn commit: r1541041 - in /tomcat/trunk/java/org/apache/jasper: ./ compiler/ resources/ servlet/

Author: markt
Date: Tue Nov 12 13:00:03 2013
New Revision: 1541041

URL: http://svn.apache.org/r1541041
Log:
Replace TldLocationsCache with the new TldCache that also caches the contents of the TLDs. This is the next step in the refactoring of TLD handling.

Added:
    tomcat/trunk/java/org/apache/jasper/compiler/TldCache.java   (with props)
Removed:
    tomcat/trunk/java/org/apache/jasper/compiler/TldLocationsCache.java
Modified:
    tomcat/trunk/java/org/apache/jasper/EmbeddedServletOptions.java
    tomcat/trunk/java/org/apache/jasper/JspC.java
    tomcat/trunk/java/org/apache/jasper/JspCompilationContext.java
    tomcat/trunk/java/org/apache/jasper/Options.java
    tomcat/trunk/java/org/apache/jasper/resources/LocalStrings.properties
    tomcat/trunk/java/org/apache/jasper/servlet/JasperInitializer.java

Modified: tomcat/trunk/java/org/apache/jasper/EmbeddedServletOptions.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/EmbeddedServletOptions.java?rev=1541041&r1=1541040&r2=1541041&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/EmbeddedServletOptions.java (original)
+++ tomcat/trunk/java/org/apache/jasper/EmbeddedServletOptions.java Tue Nov 12 13:00:03 2013
@@ -29,7 +29,7 @@ import javax.servlet.jsp.tagext.TagLibra
 import org.apache.jasper.compiler.JspConfig;
 import org.apache.jasper.compiler.Localizer;
 import org.apache.jasper.compiler.TagPluginManager;
-import org.apache.jasper.compiler.TldLocationsCache;
+import org.apache.jasper.compiler.TldCache;
 import org.apache.jasper.xmlparser.ParserUtils;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
@@ -146,9 +146,9 @@ public final class EmbeddedServletOption
     private String compilerClassName = null;
 
     /**
-     * Cache for the TLD locations
+     * Cache for the TLD URIs, resource paths and parsed files.
      */
-    private TldLocationsCache tldLocationsCache = null;
+    private TldCache tldCache = null;
 
     /**
      * Jsp config information
@@ -378,12 +378,12 @@ public final class EmbeddedServletOption
     }
 
     @Override
-    public TldLocationsCache getTldLocationsCache() {
-        return tldLocationsCache;
+    public TldCache getTldCache() {
+        return tldCache;
     }
 
-    public void setTldLocationsCache( TldLocationsCache tldC ) {
-        tldLocationsCache = tldC;
+    public void setTldCache(TldCache tldCache) {
+        this.tldCache = tldCache;
     }
 
     @Override
@@ -748,7 +748,7 @@ public final class EmbeddedServletOption
 
         // Setup the global Tag Libraries location cache for this
         // web-application.
-        tldLocationsCache = TldLocationsCache.getInstance(context);
+        tldCache = TldCache.getInstance(context);
 
         // Setup the jsp config info for this web app.
         jspConfig = new JspConfig(context);

Modified: tomcat/trunk/java/org/apache/jasper/JspC.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/JspC.java?rev=1541041&r1=1541040&r2=1541041&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/JspC.java (original)
+++ tomcat/trunk/java/org/apache/jasper/JspC.java Tue Nov 12 13:00:03 2013
@@ -49,7 +49,7 @@ import org.apache.jasper.compiler.JspCon
 import org.apache.jasper.compiler.JspRuntimeContext;
 import org.apache.jasper.compiler.Localizer;
 import org.apache.jasper.compiler.TagPluginManager;
-import org.apache.jasper.compiler.TldLocationsCache;
+import org.apache.jasper.compiler.TldCache;
 import org.apache.jasper.servlet.JspCServletContext;
 import org.apache.jasper.servlet.TldScanner;
 import org.apache.juli.logging.Log;
@@ -234,7 +234,7 @@ public class JspC extends Task implement
     /**
      * Cache for the TLD locations
      */
-    protected TldLocationsCache tldLocationsCache = null;
+    protected TldCache tldCache = null;
 
     protected JspConfig jspConfig = null;
     protected TagPluginManager tagPluginManager = null;
@@ -705,8 +705,8 @@ public class JspC extends Task implement
      * {@inheritDoc}
      */
     @Override
-    public TldLocationsCache getTldLocationsCache() {
-        return tldLocationsCache;
+    public TldCache getTldCache() {
+        return tldCache;
     }
 
     /**
@@ -1432,8 +1432,9 @@ public class JspC extends Task implement
         } catch (SAXException e) {
             throw new JasperException(e);
         }
-        tldLocationsCache = new TldLocationsCache(scanner.getUriTldResourcePathMap());
-        context.setAttribute(TldLocationsCache.KEY, tldLocationsCache);
+        tldCache = new TldCache(scanner.getUriTldResourcePathMap(),
+                scanner.getTldResourcePathTaglibXmlMap());
+        context.setAttribute(TldCache.SERVLET_CONTEXT_ATTRIBUTE_NAME, tldCache);
         rctxt = new JspRuntimeContext(context, this);
         jspConfig = new JspConfig(context);
         tagPluginManager = new TagPluginManager(context);

Modified: tomcat/trunk/java/org/apache/jasper/JspCompilationContext.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/JspCompilationContext.java?rev=1541041&r1=1541040&r2=1541041&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/JspCompilationContext.java (original)
+++ tomcat/trunk/java/org/apache/jasper/JspCompilationContext.java Tue Nov 12 13:00:03 2013
@@ -565,7 +565,7 @@ public class JspCompilationContext {
      */
     public TldLocation getTldLocation(String uri) {
         TldLocation location =
-                getOptions().getTldLocationsCache().getLocation(uri);
+                getOptions().getTldCache().getLocation(uri);
         return location;
     }
 

Modified: tomcat/trunk/java/org/apache/jasper/Options.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/Options.java?rev=1541041&r1=1541040&r2=1541041&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/Options.java (original)
+++ tomcat/trunk/java/org/apache/jasper/Options.java Tue Nov 12 13:00:03 2013
@@ -14,7 +14,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 package org.apache.jasper;
 
 import java.io.File;
@@ -24,7 +23,7 @@ import javax.servlet.jsp.tagext.TagLibra
 
 import org.apache.jasper.compiler.JspConfig;
 import org.apache.jasper.compiler.TagPluginManager;
-import org.apache.jasper.compiler.TldLocationsCache;
+import org.apache.jasper.compiler.TldCache;
 
 /**
  * A class to hold all init parameters specific to the JSP engine.
@@ -142,9 +141,8 @@ public interface Options {
     public String getCompilerClassName();
 
     /**
-     * The cache for the location of the TLD's
-     * for the various tag libraries 'exposed'
-     * by the web application.
+     * The cache that maps URIs, resource paths and parsed TLD files for the
+     * various tag libraries 'exposed' by the web application.
      * A tag library is 'exposed' either explicitly in
      * web.xml or implicitly via the uri tag in the TLD
      * of a taglib deployed in a jar file (WEB-INF/lib).
@@ -152,7 +150,7 @@ public interface Options {
      * @return the instance of the TldLocationsCache
      * for the web-application.
      */
-    public TldLocationsCache getTldLocationsCache();
+    public TldCache getTldCache();
 
     /**
      * Java platform encoding to generate the JSP

Added: tomcat/trunk/java/org/apache/jasper/compiler/TldCache.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/TldCache.java?rev=1541041&view=auto
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/TldCache.java (added)
+++ tomcat/trunk/java/org/apache/jasper/compiler/TldCache.java Tue Nov 12 13:00:03 2013
@@ -0,0 +1,85 @@
+/*
+ * 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.jasper.compiler;
+
+import java.io.IOException;
+import java.net.URL;
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.servlet.ServletContext;
+
+import org.apache.tomcat.util.descriptor.tld.TaglibXml;
+import org.apache.tomcat.util.descriptor.tld.TldResourcePath;
+
+/**
+ * This class caches parsed instances of TLD files to remove the need for the
+ * same TLD to be parsed for each JSP that references it. It does not protect
+ * against multiple threads processing the same, new TLD but it does ensure that
+ * each all threads will use the same TLD object after parsing.
+ */
+public class TldCache {
+
+    public static final String SERVLET_CONTEXT_ATTRIBUTE_NAME =
+            TldCache.class.getName();
+
+    private final Map<String, TldResourcePath> uriTldResourcePathMap = new HashMap<>();
+    private final Map<TldResourcePath, TaglibXml> tldResourcePathTaglibXmlMap = new HashMap<>();
+
+
+    public static TldCache getInstance(ServletContext servletContext) {
+        if (servletContext == null) {
+            throw new IllegalArgumentException(Localizer.getMessage(
+                    "org.apache.jasper.compiler.TldCache.servletContextNull"));
+        }
+        return (TldCache) servletContext.getAttribute(SERVLET_CONTEXT_ATTRIBUTE_NAME);
+    }
+
+
+    public TldCache(Map<String, TldResourcePath> uriTldResourcePathMap,
+            Map<TldResourcePath, TaglibXml> tldResourcePathTaglibXmlMap) {
+        this.uriTldResourcePathMap.putAll(uriTldResourcePathMap);
+        this.tldResourcePathTaglibXmlMap.putAll(tldResourcePathTaglibXmlMap);
+    }
+
+
+    /**
+     * This method is a temporary bridge between the old TLD handling code and
+     * the new. It will be removed shortly, hopefully in the next wave of
+     * refactoring.
+     */
+    @Deprecated
+    public TldLocation getLocation(String uri) {
+        TldResourcePath tldResourcePath = uriTldResourcePathMap.get(uri);
+        if (tldResourcePath == null) {
+            return null;
+        }
+        URL url = tldResourcePath.getUrl();
+        String entryName = tldResourcePath.getEntryName();
+        TldLocation tldLocation;
+        if (entryName == null) {
+            tldLocation = new TldLocation(url.toExternalForm());
+        } else {
+            try {
+                tldLocation = new TldLocation(entryName, url);
+            } catch (IOException ioe) {
+                throw new IllegalArgumentException(ioe);
+            }
+        }
+        return tldLocation;
+    }
+}

Propchange: tomcat/trunk/java/org/apache/jasper/compiler/TldCache.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: tomcat/trunk/java/org/apache/jasper/resources/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/resources/LocalStrings.properties?rev=1541041&r1=1541040&r2=1541041&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/resources/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/jasper/resources/LocalStrings.properties Tue Nov 12 13:00:03 2013
@@ -397,6 +397,9 @@ jsp.tldCache.noTldSummary=At least one J
 #ELInterpreter
 jsp.error.el_interpreter_class.instantiation=Failed to load or instantiate ELInterpreter class [{0}]
 
+org.apache.jasper.compiler.TldCache.servletContextNull=The provided SevletContext was null
+
 org.apache.jasper.servlet.JasperInitializer.onStartup=Initializing Jasper for context [{0}]
 org.apache.jasper.servlet.TldScanner.webxmlSkip=Skipping load of TLD for URI {1} from resource path {0} as it has already been defined in <jsp-config>
 org.apache.jasper.servlet.TldScanner.webxmlAdd=Loading TLD for URI {1} from resource path {0}
+

Modified: tomcat/trunk/java/org/apache/jasper/servlet/JasperInitializer.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/servlet/JasperInitializer.java?rev=1541041&r1=1541040&r2=1541041&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/servlet/JasperInitializer.java (original)
+++ tomcat/trunk/java/org/apache/jasper/servlet/JasperInitializer.java Tue Nov 12 13:00:03 2013
@@ -17,7 +17,6 @@
 package org.apache.jasper.servlet;
 
 import java.io.IOException;
-import java.util.Map;
 import java.util.Set;
 
 import javax.servlet.ServletContainerInitializer;
@@ -25,10 +24,9 @@ import javax.servlet.ServletContext;
 import javax.servlet.ServletException;
 
 import org.apache.jasper.compiler.Localizer;
-import org.apache.jasper.compiler.TldLocationsCache;
+import org.apache.jasper.compiler.TldCache;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
-import org.apache.tomcat.util.descriptor.tld.TldResourcePath;
 import org.xml.sax.SAXException;
 
 /**
@@ -64,11 +62,8 @@ public class JasperInitializer implement
             context.addListener(listener);
         }
 
-        Map<String, TldResourcePath> taglibMap = scanner.getUriTldResourcePathMap();
-        try {
-            context.setAttribute(TldLocationsCache.KEY, new TldLocationsCache(taglibMap));
-        } catch (IOException ioe) {
-            throw new ServletException(ioe);
-        }
+        context.setAttribute(TldCache.SERVLET_CONTEXT_ATTRIBUTE_NAME,
+                new TldCache(scanner.getUriTldResourcePathMap(),
+                        scanner.getTldResourcePathTaglibXmlMap()));
     }
 }



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: TldLocation

Posted by Mark Thomas <ma...@apache.org>.
On 13/11/2013 08:04, Jeremy Boynes wrote:
> On Nov 12, 2013, at 8:53 AM, Mark Thomas <ma...@apache.org> wrote:

>> 
>> I was planning on completing the removal of TldLocation. I'm 
>> currently working my way through understanding what that means.
> 
> I'd started by trying to remove the parsing code from 
> TagLibraryInfoImpl, making a thin decorator for a TaglibXml that 
> could be shared (given the TLIImpls are page/compilation specific
> due to both prefix and the reference to other TLIs).

I was thinking along the same lines.

> IIRC, that worked for tags but there was a coupling somewhere in
> the TagFileProcessor call used to populate the TagInfo for a tag
> file. I was trying to figure out why
> ctxt.setTagFileJarResource(path, jar); was needed in before calling
> it and how #46471 had been fixed.

I hadn't got this far but good to have the pointer for when I do.

> In investigating, I was also tempted to try and separate the 
> directive and load processing (visitors) in TagFileProcessor and
> see if there was a way to only do that once.

That is probably more than I am likely to do.

> I should have more time to contribute over the next couple of
> weeks and could pick this up again if you've not got too far.

Great.

I'm currently looking at tracking changes so we can avoid re-scanning
the TLDs unless we have to. The tricky part is how this interacts with
the new resources implementation. The scanner returns URLs to TLDs but
the new resources implementation could map a resource with a higher
priority to the same location and with just the URL you have no way of
knowing this. You need to know the path to the resource in the web
application. I'm experimenting with having the scanner report the web
application path as well as the URL. I'm not entirely happy with what
I have at the moment. I'm thinking about a new JarScannerResult type
that encapsulates webapp path and URL but I haven't convinced myself
that is the right way to go yet.

Mark


> 
> Cheers Jeremy
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


TldLocation

Posted by Jeremy Boynes <jb...@apache.org>.
On Nov 12, 2013, at 8:53 AM, Mark Thomas <ma...@apache.org> wrote:

> On 12/11/2013 15:27, Jeremy Boynes wrote:
>> On Nov 12, 2013, at 5:00 AM, markt@apache.org wrote:
>> 
>>> Author: markt Date: Tue Nov 12 13:00:03 2013 New Revision:
>>> 1541041
>>> 
>>> URL: http://svn.apache.org/r1541041 Log: Replace
>>> TldLocationsCache with the new TldCache that also caches the
>>> contents of the TLDs. This is the next step in the refactoring of
>>> TLD handling.
>> 
>> How far do you intend to go with these changes? I was just starting
>> to get back to them and don't want to conflict.
> 
> I was planning on completing the removal of TldLocation. I'm currently
> working my way through understanding what that means.

I'd started by trying to remove the parsing code from TagLibraryInfoImpl, making a thin decorator for a TaglibXml that could be shared (given the TLIImpls are page/compilation specific due to both prefix and the reference to other TLIs). IIRC, that worked for tags but there was a coupling somewhere in the TagFileProcessor call used to populate the TagInfo for a tag file. I was trying to figure out why
            ctxt.setTagFileJarResource(path, jar);
was needed in before calling it and how #46471 had been fixed.

In investigating, I was also tempted to try and separate the directive and load processing (visitors) in TagFileProcessor and see if there was a way to only do that once.

I should have more time to contribute over the next couple of weeks and could pick this up again if you've not got too far.

Cheers
Jeremy


Re: svn commit: r1541041 - in /tomcat/trunk/java/org/apache/jasper: ./ compiler/ resources/ servlet/

Posted by Mark Thomas <ma...@apache.org>.
On 12/11/2013 15:27, Jeremy Boynes wrote:
> On Nov 12, 2013, at 5:00 AM, markt@apache.org wrote:
> 
>> Author: markt Date: Tue Nov 12 13:00:03 2013 New Revision:
>> 1541041
>> 
>> URL: http://svn.apache.org/r1541041 Log: Replace
>> TldLocationsCache with the new TldCache that also caches the
>> contents of the TLDs. This is the next step in the refactoring of
>> TLD handling.
> 
> How far do you intend to go with these changes? I was just starting
> to get back to them and don't want to conflict.

I was planning on completing the removal of TldLocation. I'm currently
working my way through understanding what that means.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1541041 - in /tomcat/trunk/java/org/apache/jasper: ./ compiler/ resources/ servlet/

Posted by Jeremy Boynes <jb...@apache.org>.
On Nov 12, 2013, at 5:00 AM, markt@apache.org wrote:

> Author: markt
> Date: Tue Nov 12 13:00:03 2013
> New Revision: 1541041
> 
> URL: http://svn.apache.org/r1541041
> Log:
> Replace TldLocationsCache with the new TldCache that also caches the contents of the TLDs. This is the next step in the refactoring of TLD handling.

How far do you intend to go with these changes? I was just starting to get back to them and don't want to conflict.
Thanks
Jeremy