On Tue, Jan 23, 2018 at 3:34 PM, Ralph Goers <ralph.go...@dslextreme.com> wrote:
> Static vars are camelCase. Only class names are CamelCase. Isn’t that > documented in our conventions? > Fixed in git master. Thank you for the catch. Gary > > Ralph > > > On Jan 23, 2018, at 3:32 PM, Gary Gregory <garydgreg...@gmail.com> > wrote: > > > > On Tue, Jan 23, 2018 at 3:01 PM, Remko Popma <remko.po...@gmail.com> > wrote: > > > >> This solution should work. Thanks for taking care of this, Gary! > >> > >> One thing, why are the new fields capitalized? It makes the field names > >> look like class names in the code. I’ve never seen that convention. > Should > >> they not be simply lower case? > >> > > > > Hm... I use ALL_CAPS when the fields are final (constants). An instance > var > > is camelCase and a static var is CamelCase. Then you can tell what is > what. > > > > Should I change static vars to camelCase? > > > > Gary > > > > > >> (Shameless plug) Every java main() method deserves http://picocli.info > >> > >>> On Jan 24, 2018, at 6:20, ggreg...@apache.org wrote: > >>> > >>> Repository: logging-log4j2 > >>> Updated Branches: > >>> refs/heads/master 81d12ad14 -> 93e97932c > >>> > >>> > >>> [LOG4J2-2212]Unnecessary contention in > >>> CopyOnWriteSortedArrayThreadContextMap. > >>> [LOG4J2-2213] Unnecessary contention in > >>> GarbageFreeSortedArrayThreadContextMap. > >>> > >>> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo > >>> Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/ > >> commit/93e97932 > >>> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/ > >> 93e97932 > >>> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/ > >> 93e97932 > >>> > >>> Branch: refs/heads/master > >>> Commit: 93e97932c871a94486b8dc1c16f6ae3a77184871 > >>> Parents: 81d12ad > >>> Author: Gary Gregory <garydgreg...@gmail.com> > >>> Authored: Tue Jan 23 14:20:14 2018 -0700 > >>> Committer: Gary Gregory <garydgreg...@gmail.com> > >>> Committed: Tue Jan 23 14:20:14 2018 -0700 > >>> > >>> ---------------------------------------------------------------------- > >>> .../org/apache/logging/log4j/ThreadContext.java | 1 + > >>> .../CopyOnWriteSortedArrayThreadContextMap.java | 21 ++++++++--- > >>> .../GarbageFreeSortedArrayThreadContextMap.java | 23 ++++++++++-- > >>> .../log4j/spi/ThreadContextMapFactory.java | 39 > >> ++++++++++++++++---- > >>> .../log4j/ThreadContextInheritanceTest.java | 8 +++- > >>> .../logging/log4j/core/LoggerContext.java | 7 ++++ > >>> src/changes/changes.xml | 11 ++++++ > >>> 7 files changed, 92 insertions(+), 18 deletions(-) > >>> ---------------------------------------------------------------------- > >>> > >>> > >>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ > >> 93e97932/log4j-api/src/main/java/org/apache/logging/log4j/ > >> ThreadContext.java > >>> ---------------------------------------------------------------------- > >>> diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/ > ThreadContext.java > >> b/log4j-api/src/main/java/org/apache/logging/log4j/ThreadContext.java > >>> index fc018b9..c4ae445 100644 > >>> --- a/log4j-api/src/main/java/org/apache/logging/log4j/ > >> ThreadContext.java > >>> +++ b/log4j-api/src/main/java/org/apache/logging/log4j/ > >> ThreadContext.java > >>> @@ -211,6 +211,7 @@ public final class ThreadContext { > >>> * <em>Consider private, used for testing.</em> > >>> */ > >>> static void init() { > >>> + ThreadContextMapFactory.init(); > >>> contextMap = null; > >>> final PropertiesUtil managerProps = > >> PropertiesUtil.getProperties(); > >>> disableAll = managerProps.getBooleanProperty(DISABLE_ALL); > >>> > >>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ > >> 93e97932/log4j-api/src/main/java/org/apache/logging/log4j/spi/ > >> CopyOnWriteSortedArrayThreadContextMap.java > >>> ---------------------------------------------------------------------- > >>> diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/ > >> CopyOnWriteSortedArrayThreadContextMap.java > b/log4j-api/src/main/java/org/ > >> apache/logging/log4j/spi/CopyOnWriteSortedArrayThreadContextMap.java > >>> index d5e466f..fd94566 100644 > >>> --- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/ > >> CopyOnWriteSortedArrayThreadContextMap.java > >>> +++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/ > >> CopyOnWriteSortedArrayThreadContextMap.java > >>> @@ -53,9 +53,23 @@ class CopyOnWriteSortedArrayThreadContextMap > >> implements ReadOnlyThreadContextMap > >>> protected static final String PROPERTY_NAME_INITIAL_CAPACITY = > >> "log4j2.ThreadContext.initial.capacity"; > >>> > >>> private static final StringMap EMPTY_CONTEXT_DATA = new > >> SortedArrayStringMap(1); > >>> + > >>> + private static volatile int InitialCapacity; > >>> + private static volatile boolean InheritableMap; > >>> > >>> + /** > >>> + * Initializes static variables based on system properties. > >> Normally called when this class is initialized by the VM > >>> + * and when Log4j is reconfigured. > >>> + */ > >>> + static void init() { > >>> + final PropertiesUtil properties = > PropertiesUtil.getProperties() > >> ; > >>> + InitialCapacity = properties.getIntegerProperty( > >> PROPERTY_NAME_INITIAL_CAPACITY, DEFAULT_INITIAL_CAPACITY); > >>> + InheritableMap = properties.getBooleanProperty( > >> INHERITABLE_MAP); > >>> + } > >>> + > >>> static { > >>> EMPTY_CONTEXT_DATA.freeze(); > >>> + init(); > >>> } > >>> > >>> private final ThreadLocal<StringMap> localMap; > >>> @@ -67,9 +81,7 @@ class CopyOnWriteSortedArrayThreadContextMap > >> implements ReadOnlyThreadContextMap > >>> // LOG4J2-479: by default, use a plain ThreadLocal, only use > >> InheritableThreadLocal if configured. > >>> // (This method is package protected for JUnit tests.) > >>> private ThreadLocal<StringMap> createThreadLocalMap() { > >>> - final PropertiesUtil managerProps = > >> PropertiesUtil.getProperties(); > >>> - final boolean inheritable = managerProps.getBooleanProperty( > >> INHERITABLE_MAP); > >>> - if (inheritable) { > >>> + if (InheritableMap) { > >>> return new InheritableThreadLocal<StringMap>() { > >>> @Override > >>> protected StringMap childValue(final StringMap > >> parentValue) { > >>> @@ -94,8 +106,7 @@ class CopyOnWriteSortedArrayThreadContextMap > >> implements ReadOnlyThreadContextMap > >>> * @return an implementation of the {@code StringMap} used to back > >> this thread context map > >>> */ > >>> protected StringMap createStringMap() { > >>> - return new SortedArrayStringMap( > PropertiesUtil.getProperties() > >> .getIntegerProperty( > >>> - PROPERTY_NAME_INITIAL_CAPACITY, > >> DEFAULT_INITIAL_CAPACITY)); > >>> + return new SortedArrayStringMap(InitialCapacity); > >>> } > >>> > >>> /** > >>> > >>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ > >> 93e97932/log4j-api/src/main/java/org/apache/logging/log4j/spi/ > >> GarbageFreeSortedArrayThreadContextMap.java > >>> ---------------------------------------------------------------------- > >>> diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/ > >> GarbageFreeSortedArrayThreadContextMap.java > b/log4j-api/src/main/java/org/ > >> apache/logging/log4j/spi/GarbageFreeSortedArrayThreadContextMap.java > >>> index f11fd66..d695b9c 100644 > >>> --- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/ > >> GarbageFreeSortedArrayThreadContextMap.java > >>> +++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/ > >> GarbageFreeSortedArrayThreadContextMap.java > >>> @@ -53,6 +53,23 @@ class GarbageFreeSortedArrayThreadContextMap > >> implements ReadOnlyThreadContextMap > >>> protected static final String PROPERTY_NAME_INITIAL_CAPACITY = > >> "log4j2.ThreadContext.initial.capacity"; > >>> > >>> protected final ThreadLocal<StringMap> localMap; > >>> + > >>> + private static volatile int InitialCapacity; > >>> + private static volatile boolean InheritableMap; > >>> + > >>> + /** > >>> + * Initializes static variables based on system properties. > >> Normally called when this class is initialized by the VM > >>> + * and when Log4j is reconfigured. > >>> + */ > >>> + static void init() { > >>> + final PropertiesUtil properties = > PropertiesUtil.getProperties() > >> ; > >>> + InitialCapacity = properties.getIntegerProperty( > >> PROPERTY_NAME_INITIAL_CAPACITY, DEFAULT_INITIAL_CAPACITY); > >>> + InheritableMap = properties.getBooleanProperty( > >> INHERITABLE_MAP); > >>> + } > >>> + > >>> + static { > >>> + init(); > >>> + } > >>> > >>> public GarbageFreeSortedArrayThreadContextMap() { > >>> this.localMap = createThreadLocalMap(); > >>> @@ -62,8 +79,7 @@ class GarbageFreeSortedArrayThreadContextMap > >> implements ReadOnlyThreadContextMap > >>> // (This method is package protected for JUnit tests.) > >>> private ThreadLocal<StringMap> createThreadLocalMap() { > >>> final PropertiesUtil managerProps = > >> PropertiesUtil.getProperties(); > >>> - final boolean inheritable = managerProps.getBooleanProperty( > >> INHERITABLE_MAP); > >>> - if (inheritable) { > >>> + if (InheritableMap) { > >>> return new InheritableThreadLocal<StringMap>() { > >>> @Override > >>> protected StringMap childValue(final StringMap > >> parentValue) { > >>> @@ -83,8 +99,7 @@ class GarbageFreeSortedArrayThreadContextMap > >> implements ReadOnlyThreadContextMap > >>> * @return an implementation of the {@code StringMap} used to back > >> this thread context map > >>> */ > >>> protected StringMap createStringMap() { > >>> - return new SortedArrayStringMap( > PropertiesUtil.getProperties() > >> .getIntegerProperty( > >>> - PROPERTY_NAME_INITIAL_CAPACITY, > >> DEFAULT_INITIAL_CAPACITY)); > >>> + return new SortedArrayStringMap(InitialCapacity); > >>> } > >>> > >>> /** > >>> > >>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ > >> 93e97932/log4j-api/src/main/java/org/apache/logging/log4j/ > >> spi/ThreadContextMapFactory.java > >>> ---------------------------------------------------------------------- > >>> diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/ > ThreadContextMapFactory.java > >> b/log4j-api/src/main/java/org/apache/logging/log4j/spi/ > >> ThreadContextMapFactory.java > >>> index 47187dc..05c0e2d 100644 > >>> --- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/ > >> ThreadContextMapFactory.java > >>> +++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/ > >> ThreadContextMapFactory.java > >>> @@ -49,25 +49,50 @@ public final class ThreadContextMapFactory { > >>> private static final Logger LOGGER = StatusLogger.getLogger(); > >>> private static final String THREAD_CONTEXT_KEY = > >> "log4j2.threadContextMap"; > >>> private static final String GC_FREE_THREAD_CONTEXT_KEY = > >> "log4j2.garbagefree.threadContextMap"; > >>> + > >>> + private static boolean GcFreeThreadContextKey; > >>> + private static String ThreadContextMapName; > >>> > >>> + static { > >>> + initPrivate(); > >>> + } > >>> + > >>> + /** > >>> + * Initializes static variables based on system properties. > >> Normally called when this class is initialized by the VM > >>> + * and when Log4j is reconfigured. > >>> + */ > >>> + public static void init() { > >>> + CopyOnWriteSortedArrayThreadContextMap.init(); > >>> + GarbageFreeSortedArrayThreadContextMap.init(); > >>> + initPrivate(); > >>> + } > >>> + > >>> + /** > >>> + * Initializes static variables based on system properties. > >> Normally called when this class is initialized by the VM > >>> + * and when Log4j is reconfigured. > >>> + */ > >>> + private static void initPrivate() { > >>> + final PropertiesUtil properties = > PropertiesUtil.getProperties() > >> ; > >>> + ThreadContextMapName = properties.getStringProperty( > >> THREAD_CONTEXT_KEY); > >>> + GcFreeThreadContextKey = properties.getBooleanProperty( > >> GC_FREE_THREAD_CONTEXT_KEY); > >>> + } > >>> + > >>> private ThreadContextMapFactory() { > >>> } > >>> > >>> public static ThreadContextMap createThreadContextMap() { > >>> - final PropertiesUtil managerProps = > >> PropertiesUtil.getProperties(); > >>> - final String threadContextMapName = managerProps. > >> getStringProperty(THREAD_CONTEXT_KEY); > >>> final ClassLoader cl = ProviderUtil.findClassLoader(); > >>> ThreadContextMap result = null; > >>> - if (threadContextMapName != null) { > >>> + if (ThreadContextMapName != null) { > >>> try { > >>> - final Class<?> clazz = cl.loadClass( > >> threadContextMapName); > >>> + final Class<?> clazz = cl.loadClass( > >> ThreadContextMapName); > >>> if (ThreadContextMap.class.isAssignableFrom(clazz)) { > >>> result = (ThreadContextMap) clazz.newInstance(); > >>> } > >>> } catch (final ClassNotFoundException cnfe) { > >>> - LOGGER.error("Unable to locate configured > >> ThreadContextMap {}", threadContextMapName); > >>> + LOGGER.error("Unable to locate configured > >> ThreadContextMap {}", ThreadContextMapName); > >>> } catch (final Exception ex) { > >>> - LOGGER.error("Unable to create configured > >> ThreadContextMap {}", threadContextMapName, ex); > >>> + LOGGER.error("Unable to create configured > >> ThreadContextMap {}", ThreadContextMapName, ex); > >>> } > >>> } > >>> if (result == null && ProviderUtil.hasProviders() && > >> LogManager.getFactory() != null) { //LOG4J2-1658 > >>> @@ -96,7 +121,7 @@ public final class ThreadContextMapFactory { > >>> > >>> private static ThreadContextMap createDefaultThreadContextMap() { > >>> if (Constants.ENABLE_THREADLOCALS) { > >>> - if (PropertiesUtil.getProperties( > >> ).getBooleanProperty(GC_FREE_THREAD_CONTEXT_KEY)) { > >>> + if (GcFreeThreadContextKey) { > >>> return new GarbageFreeSortedArrayThreadContextMap(); > >>> } > >>> return new CopyOnWriteSortedArrayThreadContextMap(); > >>> > >>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ > >> 93e97932/log4j-api/src/test/java/org/apache/logging/log4j/ > >> ThreadContextInheritanceTest.java > >>> ---------------------------------------------------------------------- > >>> diff --git a/log4j-api/src/test/java/org/apache/logging/log4j/ > >> ThreadContextInheritanceTest.java b/log4j-api/src/test/java/org/ > >> apache/logging/log4j/ThreadContextInheritanceTest.java > >>> index 9c46e3f..a2e5fce 100644 > >>> --- a/log4j-api/src/test/java/org/apache/logging/log4j/ > >> ThreadContextInheritanceTest.java > >>> +++ b/log4j-api/src/test/java/org/apache/logging/log4j/ > >> ThreadContextInheritanceTest.java > >>> @@ -16,13 +16,16 @@ > >>> */ > >>> package org.apache.logging.log4j; > >>> > >>> +import static org.junit.Assert.assertEquals; > >>> +import static org.junit.Assert.assertFalse; > >>> +import static org.junit.Assert.assertNull; > >>> +import static org.junit.Assert.assertTrue; > >>> + > >>> import org.apache.logging.log4j.spi.DefaultThreadContextMap; > >>> import org.junit.AfterClass; > >>> import org.junit.BeforeClass; > >>> import org.junit.Test; > >>> > >>> -import static org.junit.Assert.*; > >>> - > >>> /** > >>> * Tests {@link ThreadContext}. > >>> */ > >>> @@ -54,6 +57,7 @@ public class ThreadContextInheritanceTest { > >>> @Test > >>> public void testInheritanceSwitchedOn() throws Exception { > >>> System.setProperty(DefaultThreadContextMap.INHERITABLE_MAP, > >> "true"); > >>> + ThreadContext.init(); > >>> try { > >>> ThreadContext.clearMap(); > >>> ThreadContext.put("Greeting", "Hello"); > >>> > >>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ > >> 93e97932/log4j-core/src/main/java/org/apache/logging/log4j/ > >> core/LoggerContext.java > >>> ---------------------------------------------------------------------- > >>> diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/ > LoggerContext.java > >> b/log4j-core/src/main/java/org/apache/logging/log4j/core/ > >> LoggerContext.java > >>> index cd15dce..8234024 100644 > >>> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/ > >> LoggerContext.java > >>> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/ > >> LoggerContext.java > >>> @@ -49,9 +49,11 @@ import org.apache.logging.log4j.spi.AbstractLogger; > >>> import org.apache.logging.log4j.spi.LoggerContextFactory; > >>> import org.apache.logging.log4j.spi.LoggerRegistry; > >>> import org.apache.logging.log4j.spi.Terminable; > >>> +import org.apache.logging.log4j.spi.ThreadContextMapFactory; > >>> import org.apache.logging.log4j.util.LoaderUtil; > >>> import org.apache.logging.log4j.util.PropertiesUtil; > >>> > >>> + > >>> /** > >>> * The LoggerContext is the anchor for the logging system. It maintains > >> a list of all the loggers requested by > >>> * applications and a reference to the Configuration. The Configuration > >> will contain the configured loggers, appenders, > >>> @@ -662,6 +664,7 @@ public class LoggerContext extends > AbstractLifeCycle > >>> @Override > >>> public synchronized void onChange(final Reconfigurable > >> reconfigurable) { > >>> LOGGER.debug("Reconfiguration started for context {} ({})", > >> contextName, this); > >>> + initApiModule(); > >>> final Configuration newConfig = reconfigurable.reconfigure(); > >>> if (newConfig != null) { > >>> setConfiguration(newConfig); > >>> @@ -671,6 +674,10 @@ public class LoggerContext extends > AbstractLifeCycle > >>> } > >>> } > >>> > >>> + private void initApiModule() { > >>> + ThreadContextMapFactory.init(); // Or make public and call > >> ThreadContext.init() which calls ThreadContextMapFactory.init(). > >>> + } > >>> + > >>> // LOG4J2-151: changed visibility from private to protected > >>> protected Logger newInstance(final LoggerContext ctx, final String > >> name, final MessageFactory messageFactory) { > >>> return new Logger(ctx, name, messageFactory); > >>> > >>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ > >> 93e97932/src/changes/changes.xml > >>> ---------------------------------------------------------------------- > >>> diff --git a/src/changes/changes.xml b/src/changes/changes.xml > >>> index b2f05d5..c918d0d 100644 > >>> --- a/src/changes/changes.xml > >>> +++ b/src/changes/changes.xml > >>> @@ -131,6 +131,17 @@ > >>> <action issue="LOG4J2-2210" dev="ggregory" type="update" > >> due-to="Björn Kautler"> > >>> Fix error log message for Script which says ScriptFile instead. > >>> </action> > >>> + <action issue="LOG4J2-2212" dev="ggregory" type="update" > >> due-to="Daniel Feist, Gary Gregory"> > >>> + Unnecessary contention in CopyOnWriteSortedArrayThreadCo > >> ntextMap. > >>> + </action> > >>> + <action issue="LOG4J2-2213" dev="ggregory" type="update" > >> due-to="Daniel Feist, Gary Gregory"> > >>> + Unnecessary contention in GarbageFreeSortedArrayThreadCo > >> ntextMap. > >>> + </action> > >>> + <!-- > >>> + <action issue="LOG4J2-2205" dev="ggregory" type="update" > >> due-to="Björn Kautler"> > >>> + New module log4j-mongodb3: Remove use of deprecated MongoDB > >> APIs and code to the Java driver version 3 API. > >>> + </action> > >>> + --> > >>> </release> > >>> <release version="2.10.0" date="2017-11-18" description="GA Release > >> 2.10.0"> > >>> <action issue="LOG4J2-2120" dev="mikes" type="add" due-to="Carter > >> Douglas Kozak "> > >>> > >> > > >
