Static vars are camelCase. Only class names are CamelCase. Isn’t that documented in our conventions?
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 "> >>> >>
