[GitHub] [logging-log4j2] carterkozak commented on a change in pull request #542: LOG4J2-2785: Added support for pattern Layout to abbreviate logger or class names except n rightmost words

[GitBox <gi...@apache.org>]

carterkozak commented on a change in pull request #542:
URL: https://github.com/apache/logging-log4j2/pull/542#discussion_r682718072



##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/NameAbbreviator.java
##########
@@ -365,4 +372,51 @@ public void abbreviate(final String original, final StringBuilder destination) {
             }
         }
     }
+
+    /**
+     * <p>Specialized abbreviator that shortens all words to the first char except the indicated number of rightmost words.
+     * To select this abbreviator, use pattern <code>1.n*</code> where n (&gt; 0) is the number of rightmost words to leave unchanged.</p>
+     *
+     * By example for input <code>org.apache.logging.log4j.core.pattern.NameAbbreviator</code>:
+     * <pre>
+     * 1.1*     =&gt;   o.a.l.l.c.p.NameAbbreviator
+     * 1.2*     =&gt;   o.a.l.l.c.pattern.NameAbbreviator
+     * 1.3*     =&gt;   o.a.l.l.core.pattern.NameAbbreviator
+     * ..
+     * 1.999*   =&gt;   org.apache.logging.log4j.core.pattern.NameAbbreviator
+     * </pre>
+     * @since 2.x
+     */
+    static class DynamicWordAbbreviator extends NameAbbreviator {
+
+        /** Right-most number of words (at least one) that will not be abbreviated. */
+        private final int rightWordCount;
+
+        static DynamicWordAbbreviator create(String pattern) {
+            if (pattern != null) {
+                Matcher matcher = Pattern.compile("1\\.([1-9][0-9]*)\\*").matcher(pattern);
+                if (matcher.matches()) {
+                    return new DynamicWordAbbreviator(Integer.parseInt(matcher.group(1)));
+                }
+            }
+            return null;
+        }
+
+        private DynamicWordAbbreviator(int rightWordCount) {
+            this.rightWordCount = rightWordCount;
+        }
+
+        @Override
+        public void abbreviate(String original, StringBuilder destination) {
+            if (original != null && destination != null) {
+                String[] words = original.split("\\.");
+                for (int i = 0; i < words.length - rightWordCount; i++) {
+                    words[i] = words[i].substring(0, 1);
+                }
+                destination.append(String.join(".", words));

Review comment:
       This looks very expensive. String.split uses regex and allocates a new string for each segment. Substring allocates another string per abbreviated segment, and the final String.join creates a string representation of the data that's added to the StringBuilder. I think allocation can be entirely avoided here by tacking indexes, and writing directly to the Destination buffer.
   
   It could be interesting to run the benchmarks comparing `1.1*` to the existing `1.`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org