From: markt Date: Wed, 17 Nov 2010 10:52:58 +0000 (+0000) Subject: Session manager performance X-Git-Url: https://git.internetallee.de/?a=commitdiff_plain;h=5ffd25e3d42e95f4fc3c6f67d7f5ed2a62e820ed;p=tomcat7.0 Session manager performance Focused on Windows. No need for DataInputStream, so remove it. Ensure randomIS is consistent with devRandomSource including when devRandomSource is changed whilst Manager is started Further reduce scope of syncs git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1035975 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/java/org/apache/catalina/session/ManagerBase.java b/java/org/apache/catalina/session/ManagerBase.java index f6c35ec8b..f85d618ee 100644 --- a/java/org/apache/catalina/session/ManagerBase.java +++ b/java/org/apache/catalina/session/ManagerBase.java @@ -22,10 +22,10 @@ package org.apache.catalina.session; import java.beans.PropertyChangeEvent; import java.beans.PropertyChangeListener; import java.beans.PropertyChangeSupport; -import java.io.DataInputStream; import java.io.File; import java.io.FileInputStream; import java.io.IOException; +import java.io.InputStream; import java.lang.reflect.Method; import java.security.AccessController; import java.security.MessageDigest; @@ -73,9 +73,9 @@ public abstract class ManagerBase extends LifecycleMBeanBase // ----------------------------------------------------- Instance Variables - protected DataInputStream randomIS = null; - protected String devRandomSource = "/dev/urandom"; - protected boolean devRandomSourceIsValid = true; + protected volatile InputStream randomIS = null; + protected volatile String devRandomSource = "/dev/urandom"; + protected volatile boolean devRandomSourceIsValid = true; /** * The default message digest algorithm to use if we cannot use @@ -149,7 +149,7 @@ public abstract class ManagerBase extends LifecycleMBeanBase /** * A random number generator to use when generating session identifiers. */ - protected Random random = null; + protected volatile Random random = null; /** @@ -237,39 +237,33 @@ public abstract class ManagerBase extends LifecycleMBeanBase // ------------------------------------------------------------- Security classes - private class PrivilegedSetRandomFile implements PrivilegedAction { + private class PrivilegedSetRandomFile implements PrivilegedAction { public PrivilegedSetRandomFile(String s) { devRandomSource = s; } @Override - public DataInputStream run(){ - devRandomSourceIsValid = true; + public Void run(){ try { - File f=new File( devRandomSource ); - if( ! f.exists() ) { + File f = new File(devRandomSource); + if (!f.exists()) { devRandomSourceIsValid = false; + closeRandomFile(); return null; } - randomIS= new DataInputStream( new FileInputStream(f)); - randomIS.readLong(); + InputStream is = new FileInputStream(f); + is.read(); if( log.isDebugEnabled() ) log.debug( "Opening " + devRandomSource ); - return randomIS; + randomIS = is; + devRandomSourceIsValid = true; } catch (IOException ex){ log.warn("Error reading " + devRandomSource, ex); - if (randomIS != null) { - try { - randomIS.close(); - } catch (Exception e) { - log.warn("Failed to close randomIS."); - } - } devRandomSourceIsValid = false; - randomIS=null; - return null; - } + closeRandomFile(); + } + return null; } } @@ -558,36 +552,30 @@ public abstract class ManagerBase extends LifecycleMBeanBase * visible on the first call to getSession ( like in the first JSP ) * - so use it if available. */ - public void setRandomFile( String s ) { - // Assume the source is valid until proved otherwise - devRandomSourceIsValid = true; + public synchronized void setRandomFile( String s ) { // as a hack, you can use a static file - and generate the same // session ids ( good for strange debugging ) if (Globals.IS_SECURITY_ENABLED){ - randomIS = AccessController.doPrivileged(new PrivilegedSetRandomFile(s)); + AccessController.doPrivileged(new PrivilegedSetRandomFile(s)); } else { try{ - devRandomSource=s; - File f=new File( devRandomSource ); + devRandomSource = s; + File f = new File(devRandomSource); if (!f.exists()) { devRandomSourceIsValid = false; + closeRandomFile(); return; } - randomIS= new DataInputStream( new FileInputStream(f)); - randomIS.readLong(); + InputStream is = new FileInputStream(f); + is.read(); if( log.isDebugEnabled() ) log.debug( "Opening " + devRandomSource ); + randomIS = is; + devRandomSourceIsValid = true; } catch( IOException ex ) { log.warn("Error reading " + devRandomSource, ex); - if (randomIS != null) { - try { - randomIS.close(); - } catch (Exception e) { - log.warn("Failed to close randomIS."); - } - } devRandomSourceIsValid = false; - randomIS=null; + closeRandomFile(); } } } @@ -597,6 +585,17 @@ public abstract class ManagerBase extends LifecycleMBeanBase } + protected synchronized void closeRandomFile() { + if (randomIS != null) { + try { + randomIS.close(); + } catch (Exception e) { + log.warn("Failed to close randomIS."); + } + randomIS = null; + } + } + /** * Return the random number generator instance we should use for * generating session identifiers. If there is no such generator @@ -604,35 +603,39 @@ public abstract class ManagerBase extends LifecycleMBeanBase */ public Random getRandom() { if (this.random == null) { - // Calculate the new random number generator seed - long seed = System.currentTimeMillis(); - long t1 = seed; - char entropy[] = getEntropy().toCharArray(); - for (int i = 0; i < entropy.length; i++) { - long update = ((byte) entropy[i]) << ((i % 8) * 8); - seed ^= update; - } - try { - // Construct and seed a new random number generator - Class clazz = Class.forName(randomClass); - this.random = (Random) clazz.newInstance(); - this.random.setSeed(seed); - } catch (Exception e) { - // Fall back to the simple case - log.error(sm.getString("managerBase.random", randomClass), - e); - this.random = new java.util.Random(); - this.random.setSeed(seed); - } - if(log.isDebugEnabled()) { - long t2=System.currentTimeMillis(); - if( (t2-t1) > 100 ) - log.debug(sm.getString("managerBase.seeding", randomClass) + " " + (t2-t1)); + synchronized (this) { + if (this.random == null) { + // Calculate the new random number generator seed + long seed = System.currentTimeMillis(); + long t1 = seed; + char entropy[] = getEntropy().toCharArray(); + for (int i = 0; i < entropy.length; i++) { + long update = ((byte) entropy[i]) << ((i % 8) * 8); + seed ^= update; + } + try { + // Construct and seed a new random number generator + Class clazz = Class.forName(randomClass); + this.random = (Random) clazz.newInstance(); + this.random.setSeed(seed); + } catch (Exception e) { + // Fall back to the simple case + log.error(sm.getString("managerBase.random", + randomClass), e); + this.random = new java.util.Random(); + this.random.setSeed(seed); + } + if(log.isDebugEnabled()) { + long t2=System.currentTimeMillis(); + if( (t2-t1) > 100 ) + log.debug(sm.getString("managerBase.seeding", + randomClass) + " " + (t2-t1)); + } + } } } - return (this.random); - + return this.random; } @@ -766,16 +769,7 @@ public abstract class ManagerBase extends LifecycleMBeanBase @Override protected void destroyInternal() throws LifecycleException { - - if (randomIS!=null) { - try { - randomIS.close(); - } catch (IOException ioe) { - log.warn("Failed to close randomIS."); - } - randomIS=null; - } - + closeRandomFile(); super.destroyInternal(); } @@ -959,14 +953,25 @@ public abstract class ManagerBase extends LifecycleMBeanBase } - protected synchronized void getRandomBytes(byte bytes[]) { + protected void getRandomBytes(byte bytes[]) { // Generate a byte array containing a session identifier if (devRandomSourceIsValid && randomIS == null) { - setRandomFile(devRandomSource); + synchronized (this) { + if (devRandomSourceIsValid && randomIS == null) { + setRandomFile(devRandomSource); + } + } } if (randomIS != null) { try { - int len = randomIS.read(bytes); + // If randomIS is set to null by a call to setRandomFile that + // fails, the resulting NPE will trigger a fall-back to + // getRandom() + int len; + synchronized (randomIS) { + // FileInputStream is not thread safe on all platforms + len = randomIS.read(bytes); + } if (len == bytes.length) { return; } @@ -976,14 +981,7 @@ public abstract class ManagerBase extends LifecycleMBeanBase // Ignore } devRandomSourceIsValid = false; - - try { - randomIS.close(); - } catch (Exception e) { - log.warn("Failed to close randomIS."); - } - - randomIS = null; + closeRandomFile(); } getRandom().nextBytes(bytes); } diff --git a/test/org/apache/catalina/session/Benchmarks.java b/test/org/apache/catalina/session/Benchmarks.java index 2ff964faf..f6a9dbdfd 100644 --- a/test/org/apache/catalina/session/Benchmarks.java +++ b/test/org/apache/catalina/session/Benchmarks.java @@ -32,9 +32,9 @@ public class Benchmarks extends TestCase { /* * Results on markt's 4-core Windows dev box * 1 thread - ~270ms - * 2 threads - ~400ms - * 4 threads - ~970ms - * 16 threads - ~4,000ms + * 2 threads - ~350ms + * 4 threads - ~870ms + * 16 threads - ~3,500ms * * Results on markt's 2-core OSX dev box * 1 thread - ~720ms @@ -114,10 +114,10 @@ public class Benchmarks extends TestCase { /* * Results on markt's 4-core Windows dev box - * 1 thread - ~860ms - * 2 threads - ~800ms - * 4 threads - ~1,600ms - * 16 threads - ~6,900ms + * 1 thread - ~670ms + * 2 threads - ~690ms + * 4 threads - ~1,100ms + * 16 threads - ~5,000ms * * Results on markt's 2-core OSX dev box * 1 thread - ~1,500ms