Another thing:if you look at the new code, where should I put the comment? There is nothing tricky about the new logic...
Gary has a point that it may be good to have some sort of standardized reminder to distinguish performance sensitive methods (executed for each event) from non-sensitive logic. Need to think about that one. (Ideas welcome.) On Saturday, 20 February 2016, Matt Sicker <[email protected]> wrote: > That sounds like a far more robust solution :) > > On 19 February 2016 at 11:18, Remko Popma <[email protected] > <javascript:_e(%7B%7D,'cvml','[email protected]');>> wrote: > >> Good point. >> I'm looking at a way to use aspects during unit testing to automatically >> detect if objects are allocated. >> This will help prevent regressions once the no-GC goal has been achieved. >> >> On Sat, Feb 20, 2016 at 2:03 AM, Gary Gregory <[email protected] >> <javascript:_e(%7B%7D,'cvml','[email protected]');>> wrote: >> >>> I think that all these perf changes need a method comment that says >>> something like "note that this code carefully does this and not that for >>> performance". >>> >>> It should all be doc'd otherwise it is too easy for someone else to edit >>> the method and undo the intent of the carefully tweaked perf changes. >>> >>> Gary >>> ---------- Forwarded message ---------- >>> From: <[email protected] >>> <javascript:_e(%7B%7D,'cvml','[email protected]');>> >>> Date: Feb 19, 2016 7:41 AM >>> Subject: logging-log4j2 git commit: LOG4J2-1281 >>> LoggerConfig.getProperties() should not allocate on each call. >>> To: <[email protected] >>> <javascript:_e(%7B%7D,'cvml','[email protected]');>> >>> Cc: >>> >>> Repository: logging-log4j2 >>> Updated Branches: >>> refs/heads/master 25a780e95 -> 3458ea944 >>> >>> >>> LOG4J2-1281 LoggerConfig.getProperties() should not allocate on each >>> call. >>> >>> >>> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo >>> Commit: >>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/3458ea94 >>> Tree: >>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/3458ea94 >>> Diff: >>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/3458ea94 >>> >>> Branch: refs/heads/master >>> Commit: 3458ea94491c01a4ce91bcf5bb249c80306fa42c >>> Parents: 25a780e >>> Author: rpopma <[email protected] >>> <javascript:_e(%7B%7D,'cvml','[email protected]');>> >>> Authored: Sat Feb 20 00:41:24 2016 +0900 >>> Committer: rpopma <[email protected] >>> <javascript:_e(%7B%7D,'cvml','[email protected]');>> >>> Committed: Sat Feb 20 00:41:24 2016 +0900 >>> >>> ---------------------------------------------------------------------- >>> .../org/apache/logging/log4j/core/config/LoggerConfig.java | 7 >>> ++++--- >>> src/changes/changes.xml | 3 +++ >>> 2 files changed, 7 insertions(+), 3 deletions(-) >>> ---------------------------------------------------------------------- >>> >>> >>> >>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/3458ea94/log4j-core/src/main/java/org/apache/logging/log4j/core/config/LoggerConfig.java >>> ---------------------------------------------------------------------- >>> diff --git >>> a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/LoggerConfig.java >>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/LoggerConfig.java >>> index 2bd25be..1d0f530 100644 >>> --- >>> a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/LoggerConfig.java >>> +++ >>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/LoggerConfig.java >>> @@ -127,11 +127,12 @@ public class LoggerConfig extends >>> AbstractFilterable { >>> this.includeLocation = includeLocation; >>> this.config = config; >>> if (properties != null && properties.length > 0) { >>> - this.properties = new HashMap<>(properties.length); >>> + final Map<Property, Boolean> map = new >>> HashMap<>(properties.length); >>> for (final Property prop : properties) { >>> final boolean interpolate = >>> prop.getValue().contains("${"); >>> - this.properties.put(prop, interpolate); >>> + map.put(prop, interpolate); >>> } >>> + this.properties = Collections.unmodifiableMap(map); >>> } else { >>> this.properties = null; >>> } >>> @@ -308,7 +309,7 @@ public class LoggerConfig extends AbstractFilterable >>> { >>> */ >>> // LOG4J2-157 >>> public Map<Property, Boolean> getProperties() { >>> - return properties == null ? null : >>> Collections.unmodifiableMap(properties); >>> + return properties; >>> } >>> >>> /** >>> >>> >>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/3458ea94/src/changes/changes.xml >>> ---------------------------------------------------------------------- >>> diff --git a/src/changes/changes.xml b/src/changes/changes.xml >>> index e579ed4..a006907 100644 >>> --- a/src/changes/changes.xml >>> +++ b/src/changes/changes.xml >>> @@ -24,6 +24,9 @@ >>> </properties> >>> <body> >>> <release version="2.6" date="201Y-MM-DD" description="GA Release >>> 2.6"> >>> + <action issue="LOG4J2-1281" dev="rpopma" type="fix"> >>> + LoggerConfig.getProperties() should not allocate on each call. >>> + </action> >>> <action issue="LOG4J2-1280" dev="rpopma" type="fix"> >>> Logger methods taking Supplier parameters now correctly handle >>> cases where the supplied value is a Message. >>> </action> >>> >>> >> > > > -- > Matt Sicker <[email protected] > <javascript:_e(%7B%7D,'cvml','[email protected]');>> >
