Merge pull request #3716 from eclipse/jetty-9.4.x-log-condense-improvement

Issue #3715 - Improve Log condensePackageString() performance

jetty-jmh/src/main/java/org/eclipse/jetty/util/log/LogCondensePackageStringBenchmark.java

@@ -0,0 +1,64 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.util.log;
+
+import java.util.concurrent.TimeUnit;
+
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Param;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.infra.Blackhole;
+import org.openjdk.jmh.profile.GCProfiler;
+import org.openjdk.jmh.runner.Runner;
+import org.openjdk.jmh.runner.RunnerException;
+import org.openjdk.jmh.runner.options.Options;
+import org.openjdk.jmh.runner.options.OptionsBuilder;
+
+@Fork(value = 5)
+@State(Scope.Benchmark)
+
+@Warmup(iterations = 4, time = 5, timeUnit = TimeUnit.SECONDS)
+@Measurement(iterations = 4, time = 5, timeUnit = TimeUnit.SECONDS)
+public class LogCondensePackageStringBenchmark
+{
+    @Param({"com.acme.Dump",
+            "org.eclipse.jetty.websocket.common.extensions.compress.DeflateFrameExtension$Pool"
+    })
+    String fqClassName;
+
+    @Benchmark
+    public void testCondensePackage(Blackhole blackhole)
+    {
+        blackhole.consume(AbstractLogger.condensePackageString(fqClassName));
+    }
+
+    public static void main(String[] args) throws RunnerException
+    {
+        Options opt = new OptionsBuilder()
+            .include(LogCondensePackageStringBenchmark.class.getSimpleName())
+            .addProfiler(GCProfiler.class)
+            .build();
+
+        new Runner(opt).run();
+    }
+}

jetty-util/src/main/java/org/eclipse/jetty/util/log/AbstractLogger.java

@@ -198,36 +198,62 @@ public abstract class AbstractLogger implements Logger
      *            the fully qualified class name
      * @return the condensed name
      */
+    @SuppressWarnings("Duplicates")
     protected static String condensePackageString(String classname)
     {
-        if(classname == null || classname.isEmpty())
+        if (classname == null || classname.isEmpty())
         {
             return "";
         }
-        // strip non-allowed character
-        String allowed = classname.replaceAll("[^\\w.]", "");
-        int len = allowed.length();
-        // find end of classname (strip empty sections. eg: "org.Foo.")
-        while(allowed.charAt(--len) == '.');
-        String parts[] = allowed.substring(0,len+1).split("\\.");
-        StringBuilder dense = new StringBuilder();
-        for (int i = 0; i < (parts.length - 1); i++)
+
+        int rawLen = classname.length();
+        StringBuilder dense = new StringBuilder(rawLen);
+        boolean foundStart = false;
+        boolean hasPackage = false;
+        int startIdx = -1;
+        int endIdx = -1;
+        for (int i = 0; i < rawLen; i++)
         {
-            String part = parts[i].trim();
-            if(!part.isEmpty())
+            char c = classname.charAt(i);
+            if (!foundStart)
             {
-                dense.append(part.charAt(0));
+                foundStart = Character.isJavaIdentifierStart(c);
+                if (foundStart)
+                {
+                    if (startIdx >= 0)
+                    {
+                        dense.append(classname.charAt(startIdx));
+                        hasPackage = true;
+                    }
+                    startIdx = i;
+                }
+            }
+
+            if (foundStart)
+            {
+                if (!Character.isJavaIdentifierPart(c))
+                {
+                    foundStart = false;
+                }
+                else
+                {
+                    endIdx = i;
+                }
             }
         }
-        if (dense.length() > 0)
+        // append remaining from startIdx
+        if ((startIdx >= 0) && (endIdx >= startIdx))
         {
-            dense.append('.');
+            if (hasPackage)
+            {
+                dense.append('.');
+            }
+            dense.append(classname, startIdx, endIdx + 1);
         }
-        dense.append(parts[parts.length - 1]);
+
         return dense.toString();
     }
 
-
     @Override
     public void debug(String msg, long arg)
     {

jetty-util/src/test/java/org/eclipse/jetty/util/log/LogTest.java

@@ -18,7 +18,6 @@
 
 package org.eclipse.jetty.util.log;
 
-
 import java.util.HashMap;
 import java.util.Map;
 import java.util.stream.Stream;
@@ -101,20 +100,41 @@ public class LogTest
     public static Stream<Arguments> packageCases()
     {
         return Stream.of(
-                Arguments.of(null, ""),
-                Arguments.of("org.eclipse.Foo.\u0000", "oe.Foo"),
-                Arguments.of(".foo", "foo"),
-                Arguments.of(".bar.Foo", "b.Foo"),
-                Arguments.of("org...bar..Foo", "ob.Foo")
+            // null entry
+            Arguments.of(null, ""),
+            // empty entry
+            Arguments.of("", ""),
+            // all whitespace entry
+            Arguments.of("  \t  ", ""),
+            // bad / invalid characters
+            Arguments.of("org.eclipse.Foo.\u0000", "oe.Foo"),
+            Arguments.of("org.eclipse.\u20ac.Euro", "oe\u20ac.Euro"),
+            // bad package segments
+            Arguments.of(".foo", "foo"),
+            Arguments.of(".bar.Foo", "b.Foo"),
+            Arguments.of("org...bar..Foo", "ob.Foo"),
+            Arguments.of("org . . . bar . . Foo ", "ob.Foo"),
+            Arguments.of("org . . . bar . . Foo ", "ob.Foo"),
+            // long-ish classname
+            Arguments.of("org.eclipse.jetty.websocket.common.extensions.compress.DeflateFrameExtension", "oejwcec.DeflateFrameExtension"),
+            // internal class
+            Arguments.of("org.eclipse.jetty.foo.Bar$Internal", "oejf.Bar$Internal")
         );
     }
     
     @ParameterizedTest
     @MethodSource("packageCases")
-    public void testCondensePackage(String input, String expected)
+    public void testCondensePackageViaLogger(String input, String expected)
     {
         StdErrLog log = new StdErrLog();
         StdErrLog logger = (StdErrLog) log.newLogger(input);
         assertThat("log[" + input + "] condenses to name", logger._abbrevname, is(expected));
     }
+
+    @ParameterizedTest
+    @MethodSource("packageCases")
+    public void testCondensePackageDirect(String input, String expected)
+    {
+        assertThat("log[" + input + "] condenses to name", AbstractLogger.condensePackageString(input), is(expected));
+    }
 }