Repository: knox
Updated Branches:
  refs/heads/master 42f8ec75e -> 501fd7e66


KNOX-608: Improve Knox read and write performance by tuning buffer sizes.


Project: http://git-wip-us.apache.org/repos/asf/knox/repo
Commit: http://git-wip-us.apache.org/repos/asf/knox/commit/501fd7e6
Tree: http://git-wip-us.apache.org/repos/asf/knox/tree/501fd7e6
Diff: http://git-wip-us.apache.org/repos/asf/knox/diff/501fd7e6

Branch: refs/heads/master
Commit: 501fd7e660d15ea92705ccd69bebb3342c885fab
Parents: 42f8ec7
Author: Kevin Minder <kevin.min...@hortonworks.com>
Authored: Fri Oct 16 15:49:51 2015 -0400
Committer: Kevin Minder <kevin.min...@hortonworks.com>
Committed: Fri Oct 16 15:55:40 2015 -0400

----------------------------------------------------------------------
 .../gateway/i18n/messages/MessagesInvoker.java  |  20 +--
 .../i18n/resources/ResourcesInvoker.java        |  30 ++--
 .../filter/rewrite/impl/UrlRewriteResponse.java |   4 +-
 .../security/impl/DefaultCryptoService.java     |  26 +++-
 .../topology/impl/DefaultTopologyService.java   |   4 +-
 .../gateway/dispatch/DefaultDispatch.java       |   1 -
 .../gateway/dispatch/InputStreamEntity.java     | 152 +++++++++++++++++++
 .../hadoop/gateway/util/urltemplate/Parser.java |  28 ++--
 8 files changed, 216 insertions(+), 49 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/knox/blob/501fd7e6/gateway-i18n/src/main/java/org/apache/hadoop/gateway/i18n/messages/MessagesInvoker.java
----------------------------------------------------------------------
diff --git 
a/gateway-i18n/src/main/java/org/apache/hadoop/gateway/i18n/messages/MessagesInvoker.java
 
b/gateway-i18n/src/main/java/org/apache/hadoop/gateway/i18n/messages/MessagesInvoker.java
index 21ed6ea..bc461a5 100644
--- 
a/gateway-i18n/src/main/java/org/apache/hadoop/gateway/i18n/messages/MessagesInvoker.java
+++ 
b/gateway-i18n/src/main/java/org/apache/hadoop/gateway/i18n/messages/MessagesInvoker.java
@@ -42,7 +42,7 @@ public class MessagesInvoker extends ResourcesInvoker 
implements InvocationHandl
   }
 
   @Override
-  public Object invoke( Object proxy, Method method, Object[] args ) throws 
Throwable {
+  public Object invoke( final Object proxy, final Method method, final 
Object[] args ) throws Throwable {
     String message = null;
     MessageLevel level = getLevel( method );
     if( logger.isLoggable( level ) ) {
@@ -61,7 +61,7 @@ public class MessagesInvoker extends ResourcesInvoker 
implements InvocationHandl
     return message;
   }
 
-  private String getCode( Method method ) {
+  private final String getCode( final Method method ) {
     String code = null;
     Message anno = method.getAnnotation( Message.class );
     if( anno != null ) {
@@ -73,8 +73,8 @@ public class MessagesInvoker extends ResourcesInvoker 
implements InvocationHandl
     return code;
   }
 
-  private static StackTrace getStackTraceAnno( Method method, int param ) {
-    Annotation[] annos = method.getParameterAnnotations()[ param ];
+  private final static StackTrace getStackTraceAnno( final Method method, 
final int param ) {
+    final Annotation[] annos = method.getParameterAnnotations()[ param ];
     for( Annotation anno: annos ) {
       if( anno instanceof StackTrace ) {
         return (StackTrace)anno;
@@ -83,7 +83,7 @@ public class MessagesInvoker extends ResourcesInvoker 
implements InvocationHandl
     return null;
   }
 
-  private static Throwable findLoggableThrowable( MessageLogger logger, Method 
method, Object[] args ) {
+  private final static Throwable findLoggableThrowable( final MessageLogger 
logger, final Method method, final Object[] args ) {
     Throwable throwable = null;
     if( args != null ) {
       for( int i=0; i<args.length; i++ ) {
@@ -102,7 +102,7 @@ public class MessagesInvoker extends ResourcesInvoker 
implements InvocationHandl
     return throwable;
   }
 
-  protected String getAnnotationPattern( Method method ) {
+  protected String getAnnotationPattern( final Method method ) {
     String pattern = null;
     Message anno = method.getAnnotation( Message.class );
     if( anno != null ) {
@@ -111,7 +111,7 @@ public class MessagesInvoker extends ResourcesInvoker 
implements InvocationHandl
     return pattern;
   }
 
-  private static final MessageLevel getLevel( Method method ) {
+  private static final MessageLevel getLevel( final Method method ) {
     MessageLevel level;
     Message anno = method.getAnnotation( Message.class );
     if( anno == null ) {
@@ -122,7 +122,7 @@ public class MessagesInvoker extends ResourcesInvoker 
implements InvocationHandl
     return level;
   }
 
-  private static String calcCodePattern( Class<?> clazz, Messages anno ) {
+  private static String calcCodePattern( final Class<?> clazz, final Messages 
anno ) {
     String pattern = anno.codes();
     if( Messages.DEFAULT_CODES.equals( pattern ) ) {
       pattern = clazz.getCanonicalName().replace( '.', '/' );
@@ -144,7 +144,7 @@ public class MessagesInvoker extends ResourcesInvoker 
implements InvocationHandl
     return bundle;
   }
 
-  private static String calcLoggerName( Class<?> clazz, Messages anno ) {
+  private final static String calcLoggerName( final Class<?> clazz, final 
Messages anno ) {
     String logger = null;
     if( anno != null ) {
       logger = anno.logger();
@@ -162,7 +162,7 @@ public class MessagesInvoker extends ResourcesInvoker 
implements InvocationHandl
     return bundle;
   }
 
-  private static MessageLogger getLogger( Class<?> clazz, Messages anno, 
MessageLoggerFactory loggers ) {
+  private final static MessageLogger getLogger( final Class<?> clazz, final 
Messages anno, final MessageLoggerFactory loggers ) {
     return loggers.getLogger( calcLoggerName( clazz, anno ) );
   }
 

http://git-wip-us.apache.org/repos/asf/knox/blob/501fd7e6/gateway-i18n/src/main/java/org/apache/hadoop/gateway/i18n/resources/ResourcesInvoker.java
----------------------------------------------------------------------
diff --git 
a/gateway-i18n/src/main/java/org/apache/hadoop/gateway/i18n/resources/ResourcesInvoker.java
 
b/gateway-i18n/src/main/java/org/apache/hadoop/gateway/i18n/resources/ResourcesInvoker.java
index 01c2ad8..049afc3 100644
--- 
a/gateway-i18n/src/main/java/org/apache/hadoop/gateway/i18n/resources/ResourcesInvoker.java
+++ 
b/gateway-i18n/src/main/java/org/apache/hadoop/gateway/i18n/resources/ResourcesInvoker.java
@@ -49,17 +49,17 @@ public class ResourcesInvoker implements InvocationHandler {
   }
 
   @Override
-  public Object invoke( Object proxy, Method method, Object[] args ) throws 
Throwable {
+  public Object invoke( final Object proxy, final Method method, final 
Object[] args ) throws Throwable {
     return getText( method, args );
   }
 
-  protected String getText( Method method, Object[] args ) {
+  protected final String getText( final Method method, final Object[] args ) {
     String pattern = getPattern( method );
     String text = MessageFormat.format( pattern, args );
     return text;
   }
 
-  protected String getPattern( Method method ) {
+  protected final String getPattern( final Method method ) {
     String pattern = getBundlePattern( method );
     if( pattern == null ) {
       pattern = getAnnotationPattern( method );
@@ -70,28 +70,28 @@ public class ResourcesInvoker implements InvocationHandler {
     return pattern;
   }
 
-  protected String getAnnotationPattern( Method method ) {
+  protected String getAnnotationPattern( final Method method ) {
     String pattern = null;
-    Resource anno = method.getAnnotation( Resource.class );
+    final Resource anno = method.getAnnotation( Resource.class );
     if( anno != null ) {
       pattern = anno.text();
     }
     return pattern;
   }
 
-  protected String getBundlePattern( final Method method ) {
+  protected final String getBundlePattern( final Method method ) {
     String pattern = null;
-    ResourceBundle bundle = findBundle();
+    final ResourceBundle bundle = findBundle();
     if( bundle != null && bundle.containsKey( method.getName() ) ) {
       pattern = bundle.getString( method.getName() );
     }
     return pattern;
   }
 
-  protected static String getDefaultPattern( Method method ) {
-    String prefix = method.getName();
+  protected final static String getDefaultPattern( final Method method ) {
+    final String prefix = method.getName();
     String suffix;
-    int params = method.getParameterTypes().length;
+    final int params = method.getParameterTypes().length;
     switch( params ) {
       case( 0 )  : suffix = ""; break;
       case( 1 )  : suffix = "(\"{0}\")"; break;
@@ -109,7 +109,7 @@ public class ResourcesInvoker implements InvocationHandler {
     return prefix + suffix;
   }
 
-  private static String createDefaultPatternSuffix( int size ) {
+  private final static String createDefaultPatternSuffix( final int size ) {
     StringBuilder builder = new StringBuilder( 1 + size*7 );
     builder.append( "(" );
     for( int i=0; i<size; i++ ) {
@@ -123,9 +123,9 @@ public class ResourcesInvoker implements InvocationHandler {
 
   }
 
-  private static String calcBundleName( Class<?> clazz ) {
+  private final static String calcBundleName( final Class<?> clazz ) {
     String bundle = null;
-    Resources anno = clazz.getAnnotation( Resources.class );
+    final Resources anno = clazz.getAnnotation( Resources.class );
     if( anno != null ) {
       bundle = anno.bundle();
       if( Resources.DEFAULT_BUNDLE.equals( bundle ) ) {
@@ -143,8 +143,8 @@ public class ResourcesInvoker implements InvocationHandler {
     return bundleName;
   }
 
-  protected ResourceBundle findBundle() {
-    Locale locale = Locale.getDefault();
+  protected final ResourceBundle findBundle() {
+    final Locale locale = Locale.getDefault();
     ResourceBundle bundle = bundles.get( locale );
     if( bundle == MISSING_BUNDLE ) {
       bundle = null;

http://git-wip-us.apache.org/repos/asf/knox/blob/501fd7e6/gateway-provider-rewrite/src/main/java/org/apache/hadoop/gateway/filter/rewrite/impl/UrlRewriteResponse.java
----------------------------------------------------------------------
diff --git 
a/gateway-provider-rewrite/src/main/java/org/apache/hadoop/gateway/filter/rewrite/impl/UrlRewriteResponse.java
 
b/gateway-provider-rewrite/src/main/java/org/apache/hadoop/gateway/filter/rewrite/impl/UrlRewriteResponse.java
index ef61ae1..6898620 100644
--- 
a/gateway-provider-rewrite/src/main/java/org/apache/hadoop/gateway/filter/rewrite/impl/UrlRewriteResponse.java
+++ 
b/gateway-provider-rewrite/src/main/java/org/apache/hadoop/gateway/filter/rewrite/impl/UrlRewriteResponse.java
@@ -64,7 +64,9 @@ public class UrlRewriteResponse extends 
GatewayResponseWrapper implements Params
 
   private static final UrlRewriteMessages LOG = MessagesFactory.get( 
UrlRewriteMessages.class );
 
-  private static final int STREAM_BUFFER_SIZE = 4096;
+  // An 8K buffer better matches the underlying buffer sizes.
+  // Testing with 16K made no appreciable difference.
+  private static final int STREAM_BUFFER_SIZE = 8 * 1024;
 
   private static final Set<String> IGNORE_HEADER_NAMES = new HashSet<String>();
   static {

http://git-wip-us.apache.org/repos/asf/knox/blob/501fd7e6/gateway-server/src/main/java/org/apache/hadoop/gateway/services/security/impl/DefaultCryptoService.java
----------------------------------------------------------------------
diff --git 
a/gateway-server/src/main/java/org/apache/hadoop/gateway/services/security/impl/DefaultCryptoService.java
 
b/gateway-server/src/main/java/org/apache/hadoop/gateway/services/security/impl/DefaultCryptoService.java
index b3c9d97..0700417 100644
--- 
a/gateway-server/src/main/java/org/apache/hadoop/gateway/services/security/impl/DefaultCryptoService.java
+++ 
b/gateway-server/src/main/java/org/apache/hadoop/gateway/services/security/impl/DefaultCryptoService.java
@@ -24,6 +24,7 @@ import java.security.NoSuchAlgorithmException;
 import java.security.PrivateKey;
 import java.security.Signature;
 import java.security.SignatureException;
+import java.util.HashMap;
 import java.util.Map;
 
 import org.apache.hadoop.gateway.GatewayMessages;
@@ -42,6 +43,7 @@ public class DefaultCryptoService implements CryptoService {
 
   private AliasService as = null;
   private KeystoreService ks = null;
+  private HashMap<String,AESEncryptor> encryptorCache = new 
HashMap<String,AESEncryptor>();
 
   public void setKeystoreService(KeystoreService ks) {
     this.ks = ks;
@@ -91,10 +93,8 @@ public class DefaultCryptoService implements CryptoService {
       e2.printStackTrace();
     }
     if (password != null) {
-      AESEncryptor aes = null;
       try {
-        aes = new AESEncryptor(new String(password));
-        return aes.encrypt(clear);
+        return getEncryptor(clusterName,password).encrypt( clear );
       } catch (NoSuchAlgorithmException e1) {
         LOG.failedToEncryptPasswordForCluster( clusterName, e1 );
       } catch (InvalidKeyException e) {
@@ -118,13 +118,11 @@ public class DefaultCryptoService implements 
CryptoService {
 
   @Override
   public byte[] decryptForCluster(String clusterName, String alias, byte[] 
cipherText, byte[] iv, byte[] salt) {
-    char[] password = null;
     try {
-      password = as.getPasswordFromAliasForCluster(clusterName, alias);
+      final char[] password = as.getPasswordFromAliasForCluster(clusterName, 
alias);
       if (password != null) {
-        AESEncryptor aes = new AESEncryptor(new String(password));
         try {
-          return aes.decrypt(salt, iv, cipherText);
+          return getEncryptor(clusterName,password ).decrypt( salt, iv, 
cipherText);
         } catch (Exception e) {
           LOG.failedToDecryptPasswordForCluster( clusterName, e );
         }
@@ -187,4 +185,18 @@ public class DefaultCryptoService implements CryptoService 
{
     }
     return null;
   }
+
+  // The assumption here is that lock contention will be less of a performance 
issue than the cost of object creation.
+  // We have seen via profiling that AESEncryptor instantiation is very 
expensive.
+  private final AESEncryptor getEncryptor( final String clusterName, final 
char[] password ) {
+    synchronized( encryptorCache ) {
+      AESEncryptor encryptor = encryptorCache.get( clusterName );
+      if( encryptor == null ) {
+        encryptor = new AESEncryptor( String.valueOf( password ) );
+        encryptorCache.put( clusterName, encryptor );
+      }
+      return encryptor;
+    }
+  }
+
 }

http://git-wip-us.apache.org/repos/asf/knox/blob/501fd7e6/gateway-server/src/main/java/org/apache/hadoop/gateway/services/topology/impl/DefaultTopologyService.java
----------------------------------------------------------------------
diff --git 
a/gateway-server/src/main/java/org/apache/hadoop/gateway/services/topology/impl/DefaultTopologyService.java
 
b/gateway-server/src/main/java/org/apache/hadoop/gateway/services/topology/impl/DefaultTopologyService.java
index ecfbcd9..8f2cb12 100644
--- 
a/gateway-server/src/main/java/org/apache/hadoop/gateway/services/topology/impl/DefaultTopologyService.java
+++ 
b/gateway-server/src/main/java/org/apache/hadoop/gateway/services/topology/impl/DefaultTopologyService.java
@@ -222,7 +222,9 @@ public class DefaultTopologyService
   }
 
   private void initListener(File directory) throws IOException, SAXException {
-    initListener(new FileAlterationMonitor(1000L), directory);
+    // Increasing the monitoring interval to 5 seconds as profiling has shown
+    // this is rather expensive in terms of generated garbage objects.
+    initListener(new FileAlterationMonitor(5000L), directory);
   }
 
   private Map<File, Topology> loadTopologies(File directory) {

http://git-wip-us.apache.org/repos/asf/knox/blob/501fd7e6/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/DefaultDispatch.java
----------------------------------------------------------------------
diff --git 
a/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/DefaultDispatch.java
 
b/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/DefaultDispatch.java
index c6aa1ea..6cd9461 100644
--- 
a/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/DefaultDispatch.java
+++ 
b/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/DefaultDispatch.java
@@ -38,7 +38,6 @@ import org.apache.http.client.methods.HttpPost;
 import org.apache.http.client.methods.HttpPut;
 import org.apache.http.client.methods.HttpUriRequest;
 import org.apache.http.entity.ContentType;
-import org.apache.http.entity.InputStreamEntity;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;

http://git-wip-us.apache.org/repos/asf/knox/blob/501fd7e6/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/InputStreamEntity.java
----------------------------------------------------------------------
diff --git 
a/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/InputStreamEntity.java
 
b/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/InputStreamEntity.java
new file mode 100644
index 0000000..7cde693
--- /dev/null
+++ 
b/gateway-spi/src/main/java/org/apache/hadoop/gateway/dispatch/InputStreamEntity.java
@@ -0,0 +1,152 @@
+/*
+ * ====================================================================
+ * 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.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+package org.apache.hadoop.gateway.dispatch;
+
+import org.apache.http.annotation.NotThreadSafe;
+import org.apache.http.entity.AbstractHttpEntity;
+import org.apache.http.entity.ContentType;
+import org.apache.http.util.Args;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+/**
+ * A streamed, non-repeatable entity that obtains its content froman {@link 
InputStream}.
+ * Copied from HttpClient source in order to increase buffer size.
+ */
+@NotThreadSafe
+public class InputStreamEntity extends AbstractHttpEntity {
+
+  // A value of 16K results in a significant performance improvement due to 
better HTTP chunking on the wire.
+  protected static final int OUTPUT_BUFFER_SIZE = 16 * 1024;
+  private final InputStream content;
+  private final long length;
+
+  /**
+   * Creates an entity with an unknown length.
+   * Equivalent to {@code new InputStreamEntity(instream, -1)}.
+   *
+   * @param instream input stream
+   * @throws IllegalArgumentException if {@code instream} is {@code null}
+   */
+  public InputStreamEntity( final InputStream instream ) {
+    this( instream, -1 );
+  }
+
+  /**
+   * Creates an entity with a specified content length.
+   *
+   * @param instream input stream
+   * @param length   of the input stream, {@code -1} if unknown
+   * @throws IllegalArgumentException if {@code instream} is {@code null}
+   */
+  public InputStreamEntity( final InputStream instream, final long length ) {
+    this( instream, length, null );
+  }
+
+  /**
+   * Creates an entity with a content type and unknown length.
+   * Equivalent to {@code new InputStreamEntity(instream, -1, contentType)}.
+   *
+   * @param instream    input stream
+   * @param contentType content type
+   * @throws IllegalArgumentException if {@code instream} is {@code null}
+   */
+  public InputStreamEntity( final InputStream instream, final ContentType 
contentType ) {
+    this( instream, -1, contentType );
+  }
+
+  /**
+   * @param instream    input stream
+   * @param length      of the input stream, {@code -1} if unknown
+   * @param contentType for specifying the {@code Content-Type} header, may be 
{@code null}
+   * @throws IllegalArgumentException if {@code instream} is {@code null}
+   */
+  public InputStreamEntity( final InputStream instream, final long length, 
final ContentType contentType ) {
+    super();
+    this.content = Args.notNull( instream, "Source input stream" );
+    this.length = length;
+    if( contentType != null ) {
+      setContentType( contentType.toString() );
+    }
+  }
+
+  public boolean isRepeatable() {
+    return false;
+  }
+
+  /**
+   * @return the content length or {@code -1} if unknown
+   */
+  public long getContentLength() {
+    return this.length;
+  }
+
+  public InputStream getContent() throws IOException {
+    return this.content;
+  }
+
+  /**
+   * Writes bytes from the {@code InputStream} this entity was constructed
+   * with to an {@code OutputStream}.  The content length
+   * determines how many bytes are written.  If the length is unknown ({@code 
-1}), the
+   * stream will be completely consumed (to the end of the stream).
+   */
+  public void writeTo( final OutputStream outstream ) throws IOException {
+    Args.notNull( outstream, "Output stream" );
+    final InputStream instream = this.content;
+    try {
+      final byte[] buffer = new byte[ OUTPUT_BUFFER_SIZE ];
+      int l;
+      if( this.length < 0 ) {
+        // consume until EOF
+        while( ( l = instream.read( buffer ) ) != -1 ) {
+          outstream.write( buffer, 0, l );
+        }
+      } else {
+        // consume no more than length
+        long remaining = this.length;
+        while( remaining > 0 ) {
+          l = instream.read( buffer, 0, (int)Math.min( OUTPUT_BUFFER_SIZE, 
remaining ) );
+          if( l == -1 ) {
+            break;
+          }
+          outstream.write( buffer, 0, l );
+          remaining -= l;
+        }
+      }
+    } finally {
+      instream.close();
+    }
+  }
+
+  public boolean isStreaming() {
+    return true;
+  }
+
+}

http://git-wip-us.apache.org/repos/asf/knox/blob/501fd7e6/gateway-util-urltemplate/src/main/java/org/apache/hadoop/gateway/util/urltemplate/Parser.java
----------------------------------------------------------------------
diff --git 
a/gateway-util-urltemplate/src/main/java/org/apache/hadoop/gateway/util/urltemplate/Parser.java
 
b/gateway-util-urltemplate/src/main/java/org/apache/hadoop/gateway/util/urltemplate/Parser.java
index 99f4806..8bf3586 100644
--- 
a/gateway-util-urltemplate/src/main/java/org/apache/hadoop/gateway/util/urltemplate/Parser.java
+++ 
b/gateway-util-urltemplate/src/main/java/org/apache/hadoop/gateway/util/urltemplate/Parser.java
@@ -111,7 +111,7 @@ public class Parser {
     return builder.build();
   }
 
-  private void fixNakedAuthority() {
+  private final void fixNakedAuthority() {
     if( builder.getHashScheme() &&
         !builder.getHasAuthority() &&
         !builder.getIsAbsolute() &&
@@ -119,7 +119,7 @@ public class Parser {
         ( builder.getPath().size() == 1 ) &&
         !builder.getHasQuery() &&
         !builder.getHasFragment() ) {
-      Scheme scheme = builder.getScheme();
+      final Scheme scheme = builder.getScheme();
       builder.setHasScheme( false );
       builder.setHost( makeTokenSingular( scheme.getToken() ) );
       Path path = builder.getPath().remove( 0 );
@@ -128,8 +128,8 @@ public class Parser {
     }
   }
 
-  private Token makeTokenSingular( Token token ) {
-    String effectivePattern = token.getEffectivePattern();
+  private final Token makeTokenSingular( Token token ) {
+    final String effectivePattern = token.getEffectivePattern();
     if( Segment.GLOB_PATTERN.equals( effectivePattern ) ) {
       token = new Token( token.getParameterName(), token.getOriginalPattern(), 
Segment.STAR_PATTERN );
     }
@@ -205,18 +205,18 @@ public class Parser {
     }
   }
 
-  private void consumePathToken( String token ) {
+  private final void consumePathToken( final String token ) {
     if( token != null ) {
-      StringTokenizer tokenizer = new StringTokenizer( token, "/" );
+      final StringTokenizer tokenizer = new StringTokenizer( token, "/" );
       while( tokenizer.hasMoreTokens() ) {
         consumePathSegment( tokenizer.nextToken() );
       }
     }
   }
 
-  private void consumePathSegment( String token ) {
+  private final void consumePathSegment( final String token ) {
     if( token != null ) {
-      Token t = parseTemplateToken( token, Segment.GLOB_PATTERN );
+      final Token t = parseTemplateToken( token, Segment.GLOB_PATTERN );
       builder.addPath( t );
     }
   }
@@ -281,16 +281,16 @@ public class Parser {
     }
   }
 
-  static Token parseTemplateToken( String s, String defaultEffectivePattern ) {
+  static final Token parseTemplateToken( final String s, final String 
defaultEffectivePattern ) {
     String paramName, actualPattern, effectivePattern;
-    int l = s.length();
+    final int l = s.length();
     // If the token isn't the empty string, then
     if( l > 0 ) {
-      int b = ( s.charAt( 0 ) == TEMPLATE_OPEN_MARKUP ? 1 : -1 );
-      int e = ( s.charAt( l-1 ) == TEMPLATE_CLOSE_MARKUP ? l-1 : -1 );
+      final int b = ( s.charAt( 0 ) == TEMPLATE_OPEN_MARKUP ? 1 : -1 );
+      final int e = ( s.charAt( l-1 ) == TEMPLATE_CLOSE_MARKUP ? l-1 : -1 );
       // If this is a parameter template, ie {...}
       if( ( b > 0 ) && ( e > 0 ) && ( e > b ) ) {
-        int i = s.indexOf( NAME_PATTERN_SEPARATOR, b );
+        final int i = s.indexOf( NAME_PATTERN_SEPARATOR, b );
         // If this is an anonymous template
         if( i < 0 ) {
           paramName = s.substring( b, e );
@@ -318,7 +318,7 @@ public class Parser {
       actualPattern = s;
       effectivePattern = actualPattern;
     }
-    Token token = new Token( paramName, actualPattern, effectivePattern );
+    final Token token = new Token( paramName, actualPattern, effectivePattern 
);
     return token;
   }
 

Reply via email to