svn commit: r1587102 - /logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/MarkerManager.java

[rg...@apache.org]

Author: rgoers
Date: Sun Apr 13 21:41:04 2014
New Revision: 1587102

URL: http://svn.apache.org/r1587102
Log:
LOG4J2-585 - Change check for Marker to be only on immediate parents

Modified:
    logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/MarkerManager.java

Modified: logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/MarkerManager.java
URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/MarkerManager.java?rev=1587102&r1=1587101&r2=1587102&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/MarkerManager.java (original)
+++ logging/log4j/log4j2/trunk/log4j-api/src/main/java/org/apache/logging/log4j/MarkerManager.java Sun Apr 13 21:41:04 2014
@@ -90,17 +90,20 @@ public final class MarkerManager {
             if (parent == null) {
                 throw new IllegalArgumentException("A parent marker must be specified");
             }
+            // It is not strictly necessary to copy the variable here but it should perform better than
+            // Accessing a volatile variable multiple times.
+            Marker[] localParents = this.parents;
             // Don't add a parent that is already in the hierarchy.
-            if (parents != null && (this.isInstanceOf(parent) || parent.isInstanceOf(this))) {
+            if (localParents != null && (contains(parent, localParents) || parent.isInstanceOf(this))) {
                 return this;
             }
-            int size = parents == null ? 1 : parents.length + 1;
+            int size = localParents == null ? 1 : localParents.length + 1;
             Marker[] markers = new Marker[size];
-            if (parents != null) {
-                System.arraycopy(parents, 0, markers, 0, parents.length);
+            if (localParents != null) {
+                System.arraycopy(localParents, 0, markers, 0, localParents.length);
             }
             markers[size - 1] = parent;
-            parents = markers;
+            this.parents = markers;
             return this;
         }
 
@@ -109,22 +112,23 @@ public final class MarkerManager {
             if (parent == null) {
                 throw new IllegalArgumentException("A parent marker must be specified");
             }
-            if (parents == null) {
+            Marker[] localParents = this.parents;
+            if (localParents == null) {
                 return false;
             }
-            if (parents.length == 1) {
-                if (parents[0].equals(parent)) {
+            if (localParents.length == 1) {
+                if (localParents[0].equals(parent)) {
                     parents = null;
                     return true;
                 }
                 return false;
             }
             int index = 0;
-            Marker[] markers = new Marker[parents.length - 1];
-            for (int i = 0; i < parents.length; ++i) {
-                Marker marker = parents[i];
+            Marker[] markers = new Marker[localParents.length - 1];
+            for (int i = 0; i < localParents.length; ++i) {
+                Marker marker = localParents[i];
                 if (!marker.equals(parent)) {
-                    if (index == parents.length - 1) {
+                    if (index == localParents.length - 1) {
                         return false;
                     }
                     markers[index++] = marker;
@@ -249,6 +253,19 @@ public final class MarkerManager {
             return false;
         }
 
+        /*
+         * Called from add while synchronized.
+         */
+        private boolean contains(Marker parent, Marker[] localParents) {
+
+            for (Marker marker : localParents) {
+                if (marker == parent) {
+                    return true;
+                }
+            }
+            return false;
+        }
+
         @Override
         public boolean equals(final Object o) {
             if (this == o) {