geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Matt Hogstrom <m...@hogstrom.org>
Subject Re: svn commit: r409112 - EJB Performance Improvement of 6x (did I miss something ?)!
Date Wed, 24 May 2006 10:30:41 GMT
All,

I have committed a bug fix to improve performance of DayTrader for EJB transactions that has

improved our performance from 500 TPS to 3200 TPS for PingServlet2Session (*a 6x improvement
in EJB 
performance!*).  This also includes an uncommitted (and independent) fix to OpenEJB to improve
EJB 
copy performance.

This change required me to add the concurrent jar to the MANIFEST classpath on the server.jar.
 I 
worked with DJencks on crafting the beast but wanted Dain, et all, to take a peak and make
sure that 
I didn't miss something.

This closes a significant performance gap for Geronimo when compared to other OpenSource and

Commercial AppServers :)

Cheers,

Matt

hogstrom@apache.org wrote:
> Author: hogstrom
> Date: Wed May 24 03:23:45 2006
> New Revision: 409112
> 
> URL: http://svn.apache.org/viewvc?rev=409112&view=rev
> Log:
> BUG: This fix addresses an issue in RMIClassLoaderSpiImpl where a codebase is optionally
passed for certain copy operations.  When profiling DayTrader our performance for PingServlet2Session
was extremely poor relative to JBoss and WebSphere.  Our throughput on a 2-way 3.0 Ghz system
was roughly 500 tps compared to 5400 and 4500 repsepctively.  (It appears that JBoss is not
honoring pass-by-value semantics properly; thus the significantly higher throughput).  
> 
> I chased the degraded performance to RMIClassLoaderSPIImpl where we were spending nearly
70% of the system's time in normalizeCodebase for less than 1/8 of the requests being processed.
 Here is some profiling data to show the degredation:
> 
>     =======   =====   =====   =====      =======      ======= ==========
>      Parent    4610    0.71   70.54   1157864861 115775210210   28) J:org/apache/geronimo/system/rmi/RMIClassLoaderSpiImpl.loadClass(Ljava/lang/
> 
>        Self    4610    0.71   70.54   1157864861 115775210210 [29]  J:org/apache/geronimo/system/rmi/RMIClassLoaderSpiImpl.normalizeCodebase(Lja
> 
>       Child   50550    0.34   61.97    555213244 101708338940   31) J:org/apache/geronimo/system/rmi/RMIClassLoaderSpiImpl.updateCodebase(Ljava/
>       Child    4584    0.04    4.40     65515147   7224581400   30) J:org/apache/geronimo/system/rmi/RMIClassLoaderSpiImpl.normalizeURL(Ljava/ne
>       Child   55136    0.46    2.71    758942891   4446465936   42) J:java/net/URL.<init>(Ljava/net/URL;Ljava/lang/String;Ljava/net/URLStreamHan
     
>       Child  234350    0.39    0.59    631910516    970672051   36) J:java/lang/String.indexOf(I)I
> 
> Note that of the 4610 requests were a subset of a total of 32000 transactions.  
> 
> 
> Here is the  normalizeURL information:
> 
>     =======   =====   =====   =====      =======      ======= ==========
>      Parent   50524    0.38   59.74    629510012  98041028811   30) J:org/apache/geronimo/system/rmi/RMIClassLoaderSpiImpl.normalizeURL(Ljava/ne
> 
>        Self   50524    0.38   59.74    629510012  98041028811 [32]  J:java/io/File.toURI()Ljava/net/URI;
> 
>       Child   50524    0.23   55.43    376226211  90981520312   33) J:java/net/URI.<init>(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;
>       Child   50524    0.12    3.14    189098774   5152641459   54) J:java/io/File.isDirectory()Z
>       Child   50524    0.17    0.39    274629658    646476978  192) J:java/io/File.slashify(Ljava/lang/String;Z)Ljava/lang/String;
>       Child   50524    0.14    0.29    224357024    473009995  210) J:java/io/File.<init>(Ljava/lang/String;)V
>       Child   50524    0.05    0.05     88736649     88736649  386) J:java/lang/String.startsWith(Ljava/lang/String;I)Z
>       Child   50524    0.04    0.04     69133406     69133406  408) J:java/io/Win32FileSystem.resolve(Ljava/io/File;)Ljava/lang/String;
> 
> I added a new method to inercept and cache calls to normalizeCodebase where I simply
return a cached result.  This has resulted in a 6x+ performance improvement where we have
gone from 500 tps to 3200+.   
> 
> 
> Modified:
>     geronimo/branches/1.1/assemblies/j2ee-jetty-server/project.xml
>     geronimo/branches/1.1/assemblies/j2ee-tomcat-server/project.xml
>     geronimo/branches/1.1/assemblies/minimal-jetty-server/project.xml
>     geronimo/branches/1.1/assemblies/minimal-tomcat-server/project.xml
>     geronimo/branches/1.1/assemblies/zzzzj2ee-installer/project.xml
>     geronimo/branches/1.1/configs/client-system/project.properties
>     geronimo/branches/1.1/configs/client-system/project.xml
>     geronimo/branches/1.1/configs/j2ee-system/project.properties
>     geronimo/branches/1.1/configs/j2ee-system/project.xml
>     geronimo/branches/1.1/configs/rmi-naming/project.xml
>     geronimo/branches/1.1/modules/system/src/java/org/apache/geronimo/system/rmi/RMIClassLoaderSpiImpl.java
> 
> Modified: geronimo/branches/1.1/assemblies/j2ee-jetty-server/project.xml
> URL: http://svn.apache.org/viewvc/geronimo/branches/1.1/assemblies/j2ee-jetty-server/project.xml?rev=409112&r1=409111&r2=409112&view=diff
> ==============================================================================
> --- geronimo/branches/1.1/assemblies/j2ee-jetty-server/project.xml (original)
> +++ geronimo/branches/1.1/assemblies/j2ee-jetty-server/project.xml Wed May 24 03:23:45
2006
> @@ -760,6 +760,9 @@
>              <groupId>concurrent</groupId>
>              <artifactId>concurrent</artifactId>
>              <version>${concurrent_version}</version>
> +            <properties>
> +                <geronimo.assemble>library</geronimo.assemble>
> +            </properties>
>          </dependency>
>  
>          <dependency>
> 
> Modified: geronimo/branches/1.1/assemblies/j2ee-tomcat-server/project.xml
> URL: http://svn.apache.org/viewvc/geronimo/branches/1.1/assemblies/j2ee-tomcat-server/project.xml?rev=409112&r1=409111&r2=409112&view=diff
> ==============================================================================
> --- geronimo/branches/1.1/assemblies/j2ee-tomcat-server/project.xml (original)
> +++ geronimo/branches/1.1/assemblies/j2ee-tomcat-server/project.xml Wed May 24 03:23:45
2006
> @@ -761,6 +761,9 @@
>              <groupId>concurrent</groupId>
>              <artifactId>concurrent</artifactId>
>              <version>${concurrent_version}</version>
> +            <properties>
> +                <geronimo.assemble>library</geronimo.assemble>
> +            </properties>
>          </dependency>
>  
>          <dependency>
> 
> Modified: geronimo/branches/1.1/assemblies/minimal-jetty-server/project.xml
> URL: http://svn.apache.org/viewvc/geronimo/branches/1.1/assemblies/minimal-jetty-server/project.xml?rev=409112&r1=409111&r2=409112&view=diff
> ==============================================================================
> --- geronimo/branches/1.1/assemblies/minimal-jetty-server/project.xml (original)
> +++ geronimo/branches/1.1/assemblies/minimal-jetty-server/project.xml Wed May 24 03:23:45
2006
> @@ -446,6 +446,9 @@
>              <groupId>concurrent</groupId>
>              <artifactId>concurrent</artifactId>
>              <version>${concurrent_version}</version>
> +            <properties>
> +                <geronimo.assemble>library</geronimo.assemble>
> +            </properties>
>          </dependency>
>  
>  <!-- geronimo jars -->
> 
> Modified: geronimo/branches/1.1/assemblies/minimal-tomcat-server/project.xml
> URL: http://svn.apache.org/viewvc/geronimo/branches/1.1/assemblies/minimal-tomcat-server/project.xml?rev=409112&r1=409111&r2=409112&view=diff
> ==============================================================================
> --- geronimo/branches/1.1/assemblies/minimal-tomcat-server/project.xml (original)
> +++ geronimo/branches/1.1/assemblies/minimal-tomcat-server/project.xml Wed May 24 03:23:45
2006
> @@ -446,6 +446,9 @@
>              <groupId>concurrent</groupId>
>              <artifactId>concurrent</artifactId>
>              <version>${concurrent_version}</version>
> +            <properties>
> +                <geronimo.assemble>library</geronimo.assemble>
> +            </properties>
>          </dependency>
>  
>  <!-- geronimo jars -->
> 
> Modified: geronimo/branches/1.1/assemblies/zzzzj2ee-installer/project.xml
> URL: http://svn.apache.org/viewvc/geronimo/branches/1.1/assemblies/zzzzj2ee-installer/project.xml?rev=409112&r1=409111&r2=409112&view=diff
> ==============================================================================
> --- geronimo/branches/1.1/assemblies/zzzzj2ee-installer/project.xml (original)
> +++ geronimo/branches/1.1/assemblies/zzzzj2ee-installer/project.xml Wed May 24 03:23:45
2006
> @@ -951,6 +951,9 @@
>              <groupId>concurrent</groupId>
>              <artifactId>concurrent</artifactId>
>              <version>${concurrent_version}</version>
> +            <properties>
> +                <geronimo.assemble>library</geronimo.assemble>
> +            </properties>
>          </dependency>
>  
>          <dependency>
> 
> Modified: geronimo/branches/1.1/configs/client-system/project.properties
> URL: http://svn.apache.org/viewvc/geronimo/branches/1.1/configs/client-system/project.properties?rev=409112&r1=409111&r2=409112&view=diff
> ==============================================================================
> --- geronimo/branches/1.1/configs/client-system/project.properties (original)
> +++ geronimo/branches/1.1/configs/client-system/project.properties Wed May 24 03:23:45
2006
> @@ -34,6 +34,7 @@
>      ../lib/cglib-nodep-${cglib_version}.jar \
>      ../lib/commons-cli-${commons_cli_version}.jar \
>      ../lib/commons-logging-${commons_logging_version}.jar \
> +    ../lib/cooncurrent-${concurrent_version}.jar \
>      ../lib/log4j-${log4j_version}.jar \
>      ../lib/mx4j-${mx4j_version}.jar \
>      ../lib/mx4j-remote-${mx4j_version}.jar \
> 
> Modified: geronimo/branches/1.1/configs/client-system/project.xml
> URL: http://svn.apache.org/viewvc/geronimo/branches/1.1/configs/client-system/project.xml?rev=409112&r1=409111&r2=409112&view=diff
> ==============================================================================
> --- geronimo/branches/1.1/configs/client-system/project.xml (original)
> +++ geronimo/branches/1.1/configs/client-system/project.xml Wed May 24 03:23:45 2006
> @@ -76,6 +76,14 @@
>               </properties>
>          </dependency>
>          <dependency>
> +            <groupId>concurrent</groupId>
> +            <artifactId>concurrent</artifactId>
> +            <version>${concurrent_version}</version>
> +            <properties>
> +                <geronimo.dependency>true</geronimo.dependency>
> +            </properties>
> +        </dependency>
> +        <dependency>
>              <groupId>commons-cli</groupId>
>              <artifactId>commons-cli</artifactId>
>              <version>${commons_cli_version}</version>
> 
> Modified: geronimo/branches/1.1/configs/j2ee-system/project.properties
> URL: http://svn.apache.org/viewvc/geronimo/branches/1.1/configs/j2ee-system/project.properties?rev=409112&r1=409111&r2=409112&view=diff
> ==============================================================================
> --- geronimo/branches/1.1/configs/j2ee-system/project.properties (original)
> +++ geronimo/branches/1.1/configs/j2ee-system/project.properties Wed May 24 03:23:45
2006
> @@ -32,6 +32,7 @@
>      ../lib/geronimo-system-${geronimo_version}.jar \
>      ../lib/geronimo-util-${geronimo_version}.jar \
>      ../lib/cglib-nodep-${cglib_version}.jar \
> +    ../lib/concurrent-${concurrent_version}.jar \
>      ../lib/commons-cli-${commons_cli_version}.jar \
>      ../lib/commons-logging-${commons_logging_version}.jar \
>      ../lib/log4j-${log4j_version}.jar \
> @@ -44,4 +45,4 @@
>  
>  geronimo.packaging.mainClass=org.apache.geronimo.system.main.Daemon
>  geronimo.packaging.endorsedDirs=lib/endorsed
> -geronimo.packaging.extensionDirs=lib/ext
> \ No newline at end of file
> +geronimo.packaging.extensionDirs=lib/ext
> 
> Modified: geronimo/branches/1.1/configs/j2ee-system/project.xml
> URL: http://svn.apache.org/viewvc/geronimo/branches/1.1/configs/j2ee-system/project.xml?rev=409112&r1=409111&r2=409112&view=diff
> ==============================================================================
> --- geronimo/branches/1.1/configs/j2ee-system/project.xml (original)
> +++ geronimo/branches/1.1/configs/j2ee-system/project.xml Wed May 24 03:23:45 2006
> @@ -76,6 +76,14 @@
>              </properties>
>          </dependency>
>          <dependency>
> +            <groupId>concurrent</groupId>
> +            <artifactId>concurrent</artifactId>
> +            <version>${concurrent_version}</version>
> +            <properties>
> +                <geronimo.dependency>true</geronimo.dependency>
> +            </properties>
> +        </dependency>
> +        <dependency>
>              <groupId>commons-cli</groupId>
>              <artifactId>commons-cli</artifactId>
>              <version>${commons_cli_version}</version>
> 
> Modified: geronimo/branches/1.1/configs/rmi-naming/project.xml
> URL: http://svn.apache.org/viewvc/geronimo/branches/1.1/configs/rmi-naming/project.xml?rev=409112&r1=409111&r2=409112&view=diff
> ==============================================================================
> --- geronimo/branches/1.1/configs/rmi-naming/project.xml (original)
> +++ geronimo/branches/1.1/configs/rmi-naming/project.xml Wed May 24 03:23:45 2006
> @@ -277,9 +277,6 @@
>              <groupId>concurrent</groupId>
>              <artifactId>concurrent</artifactId>
>              <version>${concurrent_version}</version>
> -            <properties>
> -                 <geronimo.dependency>true</geronimo.dependency>
> -             </properties>
>          </dependency>
>      <!--transaction module requires tranql for cache class -->
>  <!-- TODO this is false.  tranql should not be needed here -->
> 
> Modified: geronimo/branches/1.1/modules/system/src/java/org/apache/geronimo/system/rmi/RMIClassLoaderSpiImpl.java
> URL: http://svn.apache.org/viewvc/geronimo/branches/1.1/modules/system/src/java/org/apache/geronimo/system/rmi/RMIClassLoaderSpiImpl.java?rev=409112&r1=409111&r2=409112&view=diff
> ==============================================================================
> --- geronimo/branches/1.1/modules/system/src/java/org/apache/geronimo/system/rmi/RMIClassLoaderSpiImpl.java
(original)
> +++ geronimo/branches/1.1/modules/system/src/java/org/apache/geronimo/system/rmi/RMIClassLoaderSpiImpl.java
Wed May 24 03:23:45 2006
> @@ -26,6 +26,7 @@
>  
>  import java.rmi.server.RMIClassLoader;
>  import java.rmi.server.RMIClassLoaderSpi;
> +import EDU.oswego.cs.dl.util.concurrent.ConcurrentReaderHashMap;
>  
>  /**
>   * An implementation of {@link RMIClassLoaderSpi} which provides normilzation
> @@ -37,12 +38,16 @@
>      extends RMIClassLoaderSpi
>  {
>      private RMIClassLoaderSpi delegate = RMIClassLoader.getDefaultProviderInstance();
> -    
> +
> +    //TODO: Not sure of the best initial size.  Starting with 100 which should be reasonable.
> +    private ConcurrentReaderHashMap cachedCodebases = new ConcurrentReaderHashMap(100,
0.75F);
> +
> +
>      public Class loadClass(String codebase, String name, ClassLoader defaultLoader)
>          throws MalformedURLException, ClassNotFoundException
>      {
>          if (codebase != null) {
> -            codebase = normalizeCodebase(codebase);
> +            codebase = getNormalizedCodebase(codebase);
>          }
>          
>          return delegate.loadClass(codebase, name, defaultLoader);
> @@ -52,7 +57,7 @@
>          throws MalformedURLException, ClassNotFoundException
>      {
>          if (codebase != null) {
> -            codebase = normalizeCodebase(codebase);
> +            codebase = getNormalizedCodebase(codebase);
>          }
>          
>          return delegate.loadProxyClass(codebase, interfaces, defaultLoader);
> @@ -62,7 +67,7 @@
>          throws MalformedURLException
>      {
>          if (codebase != null) {
> -            codebase = normalizeCodebase(codebase);
> +            codebase = getNormalizedCodebase(codebase);
>          }
>          
>          return delegate.getClassLoader(codebase);
> @@ -89,13 +94,37 @@
>          
>          return delegate.getClassAnnotation(type);
>      }
> -    
> +
> +    /**
> +     * Uses a ConcurrentReaderHashmap to save the contents of previous parses.
> +     *
> +     * @param codebase
> +     * @return
> +     * @throws MalformedURLException
> +     */
> +    private String getNormalizedCodebase(String codebase)
> +            throws MalformedURLException {
> +        String cachedCodebase = (String)cachedCodebases.get(codebase);
> +        if (cachedCodebase != null)
> +            return cachedCodebase;
> +
> +        String normalizedCodebase = normalizeCodebase(codebase);
> +        String oldValue = (String)cachedCodebases.put(codebase, normalizedCodebase);
> +
> +        // If there was a previous value remove the one we just added to make sure the
> +        // cache doesn't grow.
> +        if (oldValue != null) {
> +            cachedCodebases.remove(codebase);
> +        }
> +        return normalizedCodebase;  // We can use the oldValue
> +    }
> +
> +
>      static String normalizeCodebase(String input)
>          throws MalformedURLException
>      {
>          assert input != null;
> -        // System.out.println("Input codebase: " + input);
> -        
> +
>          StringBuffer codebase = new StringBuffer();
>          StringBuffer working = new StringBuffer();
>          StringTokenizer stok = new StringTokenizer(input, " \t\n\r\f", true);
> @@ -169,4 +198,4 @@
>      public interface ClassLoaderServerAware {
>          public URL[] getClassLoaderServerURLs();
>      }
> -        }
> +}
> 
> 
> 
> 
> 

Mime
View raw message