We could use an annotation. That would make it easier to implement an aspect-oriented unit test to verify gc-free code paths.
On 19 February 2016 at 16:14, Remko Popma <[email protected]> wrote: > 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]> 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]> >>> 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]> >>>> 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]> >>>> 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]> >>>> Authored: Sat Feb 20 00:41:24 2016 +0900 >>>> Committer: rpopma <[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]> >> > -- Matt Sicker <[email protected]>
