You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by rp...@apache.org on 2016/03/16 14:38:41 UTC

[24/50] logging-log4j2 git commit: Re-implement CompositeFilter to be backed by a Filter[] instead of an ArrayList for performance. Internally, we now traverse the array as a primitive with [] instead of using a new Iterator each time the filters

Re-implement CompositeFilter to be backed by a Filter[] instead of an
ArrayList<Filter> for performance. Internally, we now traverse the array
as a primitive with [] instead of using a new Iterator each time the
filters need to be traversed. Note: I have a use case where the internal
list was always empty, which meant an iterator was always created, when
for example, Logger.isTraceEnabled() was called. After a couple of
million times calling isTraceEnabled(), it adds up. Now, the array is
not traversed since it is since of length 0. This is means one less
object generated per call to isTraceEnabled() and thrown away (the
iterator) when a config has an empty Filters section (because child in
it is commented out).

Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/3761dba6
Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/3761dba6
Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/3761dba6

Branch: refs/heads/LOG4J2-1278-gc-free-logger
Commit: 3761dba60f3f498f8691f59e772a2cb4d9be2928
Parents: b74ece8
Author: ggregory <gg...@apache.org>
Authored: Fri Mar 11 17:05:21 2016 -0800
Committer: ggregory <gg...@apache.org>
Committed: Fri Mar 11 17:05:21 2016 -0800

----------------------------------------------------------------------
 .../log4j/core/filter/CompositeFilter.java      |  76 ++++----
 .../log4j/core/util/ObjectArrayIterator.java    | 178 +++++++++++++++++++
 .../log4j/core/CustomLevelsWithFiltersTest.java |   6 +-
 3 files changed, 223 insertions(+), 37 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/3761dba6/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/CompositeFilter.java
----------------------------------------------------------------------
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/CompositeFilter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/CompositeFilter.java
index 5fe084c..7a6efbc 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/CompositeFilter.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/CompositeFilter.java
@@ -16,9 +16,7 @@
  */
 package org.apache.logging.log4j.core.filter;
 
-import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 
@@ -32,6 +30,7 @@ import org.apache.logging.log4j.core.config.Node;
 import org.apache.logging.log4j.core.config.plugins.Plugin;
 import org.apache.logging.log4j.core.config.plugins.PluginElement;
 import org.apache.logging.log4j.core.config.plugins.PluginFactory;
+import org.apache.logging.log4j.core.util.ObjectArrayIterator;
 import org.apache.logging.log4j.message.Message;
 
 /**
@@ -40,18 +39,15 @@ import org.apache.logging.log4j.message.Message;
 @Plugin(name = "filters", category = Node.CATEGORY, printObject = true)
 public final class CompositeFilter extends AbstractLifeCycle implements Iterable<Filter>, Filter {
 
-    private final List<Filter> filters;
+    private static final Filter[] EMPTY_FILTERS = new Filter[0];
+    private final Filter[] filters;
 
     private CompositeFilter() {
-        this.filters = new ArrayList<>();
+        this.filters = EMPTY_FILTERS;
     }
 
-    private CompositeFilter(final List<Filter> filters) {
-        if (filters == null) {
-            this.filters = Collections.unmodifiableList(new ArrayList<Filter>());
-            return;
-        }
-        this.filters = Collections.unmodifiableList(filters);
+    private CompositeFilter(final Filter[] filters) {
+        this.filters = filters == null ? EMPTY_FILTERS : filters;
     }
 
     public CompositeFilter addFilter(final Filter filter) {
@@ -59,9 +55,9 @@ public final class CompositeFilter extends AbstractLifeCycle implements Iterable
             // null does nothing
             return this;
         }
-        final List<Filter> filterList = new ArrayList<>(this.filters);
-        filterList.add(filter);
-        return new CompositeFilter(Collections.unmodifiableList(filterList));
+        final Filter[] copy = Arrays.copyOf(this.filters, this.filters.length + 1);
+        copy[this.filters.length] = filter;
+        return new CompositeFilter(copy);
     }
 
     public CompositeFilter removeFilter(final Filter filter) {
@@ -69,17 +65,31 @@ public final class CompositeFilter extends AbstractLifeCycle implements Iterable
             // null does nothing
             return this;
         }
-        final List<Filter> filterList = new ArrayList<>(this.filters);
+        // This is not a great implementation but simpler than copying Apache Commons 
+        // Lang ArrayUtils.removeElement() and associated bits (MutableInt),
+        // which is OK since removing a filter should not be on the critical path.
+        final List<Filter> filterList = Arrays.asList(this.filters);
         filterList.remove(filter);
-        return new CompositeFilter(Collections.unmodifiableList(filterList));
+        return new CompositeFilter(filterList.toArray(new Filter[this.filters.length - 1]));
     }
 
     @Override
     public Iterator<Filter> iterator() {
-        return filters.iterator();
+        return new ObjectArrayIterator<>(filters);
     }
 
+    /**
+     * Gets a new list over the internal filter array.
+     * 
+     * @return a new list over the internal filter array
+     * @deprecated Use {@link #getFiltersArray()}
+     */
+    @Deprecated
     public List<Filter> getFilters() {
+        return Arrays.asList(filters);
+    }
+
+    public Filter[] getFiltersArray() {
         return filters;
     }
 
@@ -89,11 +99,11 @@ public final class CompositeFilter extends AbstractLifeCycle implements Iterable
      * @return whether this composite contains any filters.
      */
     public boolean isEmpty() {
-        return this.filters.isEmpty();
+        return this.filters.length == 0;
     }
 
     public int size() {
-        return filters.size();
+        return filters.length;
     }
 
     @Override
@@ -151,10 +161,10 @@ public final class CompositeFilter extends AbstractLifeCycle implements Iterable
      */
     @Override
     public Result filter(final Logger logger, final Level level, final Marker marker, final String msg,
-                         final Object... params) {
+            final Object... params) {
         Result result = Result.NEUTRAL;
-        for (final Filter filter : filters) {
-            result = filter.filter(logger, level, marker, msg, params);
+        for (int i = 0; i < filters.length; i++) {
+            result = filters[i].filter(logger, level, marker, msg, params);
             if (result == Result.ACCEPT || result == Result.DENY) {
                 return result;
             }
@@ -179,10 +189,10 @@ public final class CompositeFilter extends AbstractLifeCycle implements Iterable
      */
     @Override
     public Result filter(final Logger logger, final Level level, final Marker marker, final Object msg,
-                         final Throwable t) {
+            final Throwable t) {
         Result result = Result.NEUTRAL;
-        for (final Filter filter : filters) {
-            result = filter.filter(logger, level, marker, msg, t);
+        for (int i = 0; i < filters.length; i++) {
+            result = filters[i].filter(logger, level, marker, msg, t);
             if (result == Result.ACCEPT || result == Result.DENY) {
                 return result;
             }
@@ -207,10 +217,10 @@ public final class CompositeFilter extends AbstractLifeCycle implements Iterable
      */
     @Override
     public Result filter(final Logger logger, final Level level, final Marker marker, final Message msg,
-                         final Throwable t) {
+            final Throwable t) {
         Result result = Result.NEUTRAL;
-        for (final Filter filter : filters) {
-            result = filter.filter(logger, level, marker, msg, t);
+        for (int i = 0; i < filters.length; i++) {
+            result = filters[i].filter(logger, level, marker, msg, t);
             if (result == Result.ACCEPT || result == Result.DENY) {
                 return result;
             }
@@ -228,8 +238,8 @@ public final class CompositeFilter extends AbstractLifeCycle implements Iterable
     @Override
     public Result filter(final LogEvent event) {
         Result result = Result.NEUTRAL;
-        for (final Filter filter : filters) {
-            result = filter.filter(event);
+        for (int i = 0; i < filters.length; i++) {
+            result = filters[i].filter(event);
             if (result == Result.ACCEPT || result == Result.DENY) {
                 return result;
             }
@@ -240,13 +250,13 @@ public final class CompositeFilter extends AbstractLifeCycle implements Iterable
     @Override
     public String toString() {
         final StringBuilder sb = new StringBuilder();
-        for (final Filter filter : filters) {
+        for (int i = 0; i < filters.length; i++) {
             if (sb.length() == 0) {
                 sb.append('{');
             } else {
                 sb.append(", ");
             }
-            sb.append(filter.toString());
+            sb.append(filters[i].toString());
         }
         if (sb.length() > 0) {
             sb.append('}');
@@ -263,9 +273,7 @@ public final class CompositeFilter extends AbstractLifeCycle implements Iterable
      */
     @PluginFactory
     public static CompositeFilter createFilters(@PluginElement("Filters") final Filter[] filters) {
-        final List<Filter> filterList = filters == null || filters.length == 0 ?
-            new ArrayList<Filter>() : Arrays.asList(filters);
-        return new CompositeFilter(filterList);
+        return new CompositeFilter(filters);
     }
 
 }

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/3761dba6/log4j-core/src/main/java/org/apache/logging/log4j/core/util/ObjectArrayIterator.java
----------------------------------------------------------------------
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/ObjectArrayIterator.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/ObjectArrayIterator.java
new file mode 100644
index 0000000..9f9897e
--- /dev/null
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/ObjectArrayIterator.java
@@ -0,0 +1,178 @@
+/*
+ * 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.
+ */
+package org.apache.logging.log4j.core.util;
+
+import java.util.Iterator;
+import java.util.NoSuchElementException;
+
+/**
+ * An {@link java.util.Iterator Iterator} over an array of objects.
+ * <p>
+ * This iterator does not support {@link #remove}, as the object array cannot be
+ * structurally modified.
+ * <p>
+ * The iterator implements a {@link #reset} method, allowing the reset of the iterator
+ * back to the start if required.
+ *
+ * @param <E> the type to iterate over
+ * @since 3.0
+ * @version $Id: ObjectArrayIterator.java 1734648 2016-03-11 23:51:22Z ggregory $
+ */
+public class ObjectArrayIterator<E> implements /*Resettable*/ Iterator<E> {
+
+    /** The array */
+    final E[] array;
+    /** The start index to loop from */
+    final int startIndex;
+    /** The end index to loop to */
+    final int endIndex;
+    /** The current iterator index */
+    int index = 0;
+
+    //-------------------------------------------------------------------------
+    /**
+     * Constructs an ObjectArrayIterator that will iterate over the values in the
+     * specified array.
+     *
+     * @param array the array to iterate over
+     * @throws NullPointerException if <code>array</code> is <code>null</code>
+     */
+    public ObjectArrayIterator(final E... array) {
+        this(array, 0, array.length);
+    }
+
+    /**
+     * Constructs an ObjectArrayIterator that will iterate over the values in the
+     * specified array from a specific start index.
+     *
+     * @param array  the array to iterate over
+     * @param start  the index to start iterating at
+     * @throws NullPointerException if <code>array</code> is <code>null</code>
+     * @throws IndexOutOfBoundsException if the start index is out of bounds
+     */
+    public ObjectArrayIterator(final E array[], final int start) {
+        this(array, start, array.length);
+    }
+
+    /**
+     * Construct an ObjectArrayIterator that will iterate over a range of values
+     * in the specified array.
+     *
+     * @param array  the array to iterate over
+     * @param start  the index to start iterating at
+     * @param end  the index (exclusive) to finish iterating at
+     * @throws IndexOutOfBoundsException if the start or end index is out of bounds
+     * @throws IllegalArgumentException if end index is before the start
+     * @throws NullPointerException if <code>array</code> is <code>null</code>
+     */
+    public ObjectArrayIterator(final E array[], final int start, final int end) {
+        super();
+        if (start < 0) {
+            throw new ArrayIndexOutOfBoundsException("Start index must not be less than zero");
+        }
+        if (end > array.length) {
+            throw new ArrayIndexOutOfBoundsException("End index must not be greater than the array length");
+        }
+        if (start > array.length) {
+            throw new ArrayIndexOutOfBoundsException("Start index must not be greater than the array length");
+        }
+        if (end < start) {
+            throw new IllegalArgumentException("End index must not be less than start index");
+        }
+        this.array = array;
+        this.startIndex = start;
+        this.endIndex = end;
+        this.index = start;
+    }
+
+    // Iterator interface
+    //-------------------------------------------------------------------------
+
+    /**
+     * Returns true if there are more elements to return from the array.
+     *
+     * @return true if there is a next element to return
+     */
+    @Override
+    public boolean hasNext() {
+        return this.index < this.endIndex;
+    }
+
+    /**
+     * Returns the next element in the array.
+     *
+     * @return the next element in the array
+     * @throws NoSuchElementException if all the elements in the array
+     *    have already been returned
+     */
+    @Override
+    public E next() {
+        if (hasNext() == false) {
+            throw new NoSuchElementException();
+        }
+        return this.array[this.index++];
+    }
+
+    /**
+     * Throws {@link UnsupportedOperationException}.
+     *
+     * @throws UnsupportedOperationException always
+     */
+    @Override
+    public void remove() {
+        throw new UnsupportedOperationException("remove() method is not supported for an ObjectArrayIterator");
+    }
+
+    // Properties
+    //-------------------------------------------------------------------------
+
+    /**
+     * Gets the array that this iterator is iterating over.
+     *
+     * @return the array this iterator iterates over
+     */
+    public E[] getArray() {
+        return this.array;
+    }
+
+    /**
+     * Gets the start index to loop from.
+     *
+     * @return the start index
+     */
+    public int getStartIndex() {
+        return this.startIndex;
+    }
+
+    /**
+     * Gets the end index to loop to.
+     *
+     * @return the end index
+     */
+    public int getEndIndex() {
+        return this.endIndex;
+    }
+
+    /**
+     * Resets the iterator back to the start index.
+     */
+    //@Override
+    public void reset() {
+        this.index = this.startIndex;
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/3761dba6/log4j-core/src/test/java/org/apache/logging/log4j/core/CustomLevelsWithFiltersTest.java
----------------------------------------------------------------------
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/CustomLevelsWithFiltersTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/CustomLevelsWithFiltersTest.java
index bdb2522..440d5fb 100644
--- a/log4j-core/src/test/java/org/apache/logging/log4j/core/CustomLevelsWithFiltersTest.java
+++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/CustomLevelsWithFiltersTest.java
@@ -59,10 +59,10 @@ public class CustomLevelsWithFiltersTest {
         assertNotNull(appender);
         final CompositeFilter compFilter = (CompositeFilter) appender.getFilter();
         assertNotNull(compFilter);
-        final List<Filter> filterList = compFilter.getFilters();
-        assertNotNull(filterList);
+        final Filter[] filters = compFilter.getFiltersArray();
+        assertNotNull(filters);
         boolean foundLevel = false;
-        for (final Filter filter : filterList) {
+        for (final Filter filter : filters) {
             final ThresholdFilter tFilter = (ThresholdFilter) filter;
             if (infom1Level.equals(tFilter.getLevel())) {
                 foundLevel = true;