You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4j-dev@logging.apache.org by Gary Gregory <ga...@gmail.com> on 2016/02/19 18:03:48 UTC
Fwd: logging-log4j2 git commit: LOG4J2-1281 LoggerConfig.getProperties()
should not allocate on each call.
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: <rp...@apache.org>
Date: Feb 19, 2016 7:41 AM
Subject: logging-log4j2 git commit: LOG4J2-1281
LoggerConfig.getProperties() should not allocate on each call.
To: <co...@logging.apache.org>
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 <rp...@apache.org>
Authored: Sat Feb 20 00:41:24 2016 +0900
Committer: rpopma <rp...@apache.org>
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>
Re: logging-log4j2 git commit: LOG4J2-1281 LoggerConfig.getProperties()
should not allocate on each call.
Posted by Matt Sicker <bo...@gmail.com>.
I've added a @PerformanceSensitive annotation in log4j-api along with a few
places I could think of that warrant it. Though I'm not sure about what
attributes would be useful here.
On 21 February 2016 at 20:17, Remko Popma <re...@gmail.com> wrote:
> Haha! :-)
> Is that good or bad? :-O
>
> On Mon, Feb 22, 2016 at 11:04 AM, Gary Gregory <ga...@gmail.com>
> wrote:
>
>> You can be the Hemingway of Java code! ;-)
>>
>> On Sun, Feb 21, 2016 at 6:04 PM, Gary Gregory <ga...@gmail.com>
>> wrote:
>>
>>> Well, yeah, sure, as long as it makes sense...
>>>
>>>
>>> On Sun, Feb 21, 2016 at 6:01 PM, Remko Popma <re...@gmail.com>
>>> wrote:
>>>
>>>> You actually end up with really clean code where each method does only
>>>> one thing. Single responsibility principle etc.
>>>> Clean code is fast!
>>>>
>>>> On Mon, Feb 22, 2016 at 10:58 AM, Gary Gregory <ga...@gmail.com>
>>>> wrote:
>>>>
>>>>> The 35 bytecodes or less limit seems arbitrary. What about IBM JVMs?
>>>>> Other vendors'?
>>>>>
>>>>> I OK with refactoring as long as it makes sense but just doing it for
>>>>> an arbitrary byte limit if it makes the code harder to maintain seems
>>>>> like it could be... I'm not sure what the word is I'm even looking for here!
>>>>>
>>>>> Gary
>>>>>
>>>>> On Sun, Feb 21, 2016 at 5:49 PM, Remko Popma <re...@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> I'd say log4j-api util, so it can be used in the log4j-api
>>>>>> implementation classes as well.
>>>>>>
>>>>>> I would not want to over-complicate this, but there are two ways in
>>>>>> which methods can be optimized for performance which are "fragile" in that
>>>>>> a marker of some kind may be useful:
>>>>>>
>>>>>> * refactor into small private methods 35 bytecodes or less (these are
>>>>>> inlined automatically by Hotspot). I've tried to be consistent in adding a
>>>>>> comment to such methods, but writing the same comment get tedious.
>>>>>> * reduce GC pressure by replacing methods or constructs that allocate
>>>>>> temporary objects with alternatives that don't. However, there are many,
>>>>>> many methods and most don't allocate. I would not want to annotate all
>>>>>> methods... Hmm, need to think about this one more.
>>>>>>
>>>>>>
>>>>>> On Mon, Feb 22, 2016 at 10:24 AM, Matt Sicker <bo...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> Do we need this in log4j-api anywhere, or can we stick it in
>>>>>>> log4j-core?
>>>>>>>
>>>>>>> On 19 February 2016 at 18:29, Remko Popma <re...@gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> I like that idea very much.
>>>>>>>>
>>>>>>>> Sent from my iPhone
>>>>>>>>
>>>>>>>> On 2016/02/20, at 7:37, Matt Sicker <bo...@gmail.com> wrote:
>>>>>>>>
>>>>>>>> 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 <re...@gmail.com>
>>>>>>>> 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 <bo...@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> That sounds like a far more robust solution :)
>>>>>>>>>>
>>>>>>>>>> On 19 February 2016 at 11:18, Remko Popma <re...@gmail.com>
>>>>>>>>>> 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 <
>>>>>>>>>>> garydgregory@gmail.com> 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: <rp...@apache.org>
>>>>>>>>>>>> Date: Feb 19, 2016 7:41 AM
>>>>>>>>>>>> Subject: logging-log4j2 git commit: LOG4J2-1281
>>>>>>>>>>>> LoggerConfig.getProperties() should not allocate on each call.
>>>>>>>>>>>> To: <co...@logging.apache.org>
>>>>>>>>>>>> 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 <rp...@apache.org>
>>>>>>>>>>>> Authored: Sat Feb 20 00:41:24 2016 +0900
>>>>>>>>>>>> Committer: rpopma <rp...@apache.org>
>>>>>>>>>>>> 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 <bo...@gmail.com>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Matt Sicker <bo...@gmail.com>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Matt Sicker <bo...@gmail.com>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>>>>> Java Persistence with Hibernate, Second Edition
>>>>> <http://www.manning.com/bauer3/>
>>>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>>>>> Spring Batch in Action <http://www.manning.com/templier/>
>>>>> Blog: http://garygregory.wordpress.com
>>>>> Home: http://garygregory.com/
>>>>> Tweet! http://twitter.com/GaryGregory
>>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>>> Java Persistence with Hibernate, Second Edition
>>> <http://www.manning.com/bauer3/>
>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>>> Spring Batch in Action <http://www.manning.com/templier/>
>>> Blog: http://garygregory.wordpress.com
>>> Home: http://garygregory.com/
>>> Tweet! http://twitter.com/GaryGregory
>>>
>>
>>
>>
>> --
>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>> Java Persistence with Hibernate, Second Edition
>> <http://www.manning.com/bauer3/>
>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>> Spring Batch in Action <http://www.manning.com/templier/>
>> Blog: http://garygregory.wordpress.com
>> Home: http://garygregory.com/
>> Tweet! http://twitter.com/GaryGregory
>>
>
>
--
Matt Sicker <bo...@gmail.com>
Re: logging-log4j2 git commit: LOG4J2-1281 LoggerConfig.getProperties()
should not allocate on each call.
Posted by Remko Popma <re...@gmail.com>.
Haha! :-)
Is that good or bad? :-O
On Mon, Feb 22, 2016 at 11:04 AM, Gary Gregory <ga...@gmail.com>
wrote:
> You can be the Hemingway of Java code! ;-)
>
> On Sun, Feb 21, 2016 at 6:04 PM, Gary Gregory <ga...@gmail.com>
> wrote:
>
>> Well, yeah, sure, as long as it makes sense...
>>
>>
>> On Sun, Feb 21, 2016 at 6:01 PM, Remko Popma <re...@gmail.com>
>> wrote:
>>
>>> You actually end up with really clean code where each method does only
>>> one thing. Single responsibility principle etc.
>>> Clean code is fast!
>>>
>>> On Mon, Feb 22, 2016 at 10:58 AM, Gary Gregory <ga...@gmail.com>
>>> wrote:
>>>
>>>> The 35 bytecodes or less limit seems arbitrary. What about IBM JVMs?
>>>> Other vendors'?
>>>>
>>>> I OK with refactoring as long as it makes sense but just doing it for
>>>> an arbitrary byte limit if it makes the code harder to maintain seems
>>>> like it could be... I'm not sure what the word is I'm even looking for here!
>>>>
>>>> Gary
>>>>
>>>> On Sun, Feb 21, 2016 at 5:49 PM, Remko Popma <re...@gmail.com>
>>>> wrote:
>>>>
>>>>> I'd say log4j-api util, so it can be used in the log4j-api
>>>>> implementation classes as well.
>>>>>
>>>>> I would not want to over-complicate this, but there are two ways in
>>>>> which methods can be optimized for performance which are "fragile" in that
>>>>> a marker of some kind may be useful:
>>>>>
>>>>> * refactor into small private methods 35 bytecodes or less (these are
>>>>> inlined automatically by Hotspot). I've tried to be consistent in adding a
>>>>> comment to such methods, but writing the same comment get tedious.
>>>>> * reduce GC pressure by replacing methods or constructs that allocate
>>>>> temporary objects with alternatives that don't. However, there are many,
>>>>> many methods and most don't allocate. I would not want to annotate all
>>>>> methods... Hmm, need to think about this one more.
>>>>>
>>>>>
>>>>> On Mon, Feb 22, 2016 at 10:24 AM, Matt Sicker <bo...@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> Do we need this in log4j-api anywhere, or can we stick it in
>>>>>> log4j-core?
>>>>>>
>>>>>> On 19 February 2016 at 18:29, Remko Popma <re...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> I like that idea very much.
>>>>>>>
>>>>>>> Sent from my iPhone
>>>>>>>
>>>>>>> On 2016/02/20, at 7:37, Matt Sicker <bo...@gmail.com> wrote:
>>>>>>>
>>>>>>> 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 <re...@gmail.com>
>>>>>>> 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 <bo...@gmail.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> That sounds like a far more robust solution :)
>>>>>>>>>
>>>>>>>>> On 19 February 2016 at 11:18, Remko Popma <re...@gmail.com>
>>>>>>>>> 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 <
>>>>>>>>>> garydgregory@gmail.com> 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: <rp...@apache.org>
>>>>>>>>>>> Date: Feb 19, 2016 7:41 AM
>>>>>>>>>>> Subject: logging-log4j2 git commit: LOG4J2-1281
>>>>>>>>>>> LoggerConfig.getProperties() should not allocate on each call.
>>>>>>>>>>> To: <co...@logging.apache.org>
>>>>>>>>>>> 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 <rp...@apache.org>
>>>>>>>>>>> Authored: Sat Feb 20 00:41:24 2016 +0900
>>>>>>>>>>> Committer: rpopma <rp...@apache.org>
>>>>>>>>>>> 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 <bo...@gmail.com>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Matt Sicker <bo...@gmail.com>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Matt Sicker <bo...@gmail.com>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>>>> Java Persistence with Hibernate, Second Edition
>>>> <http://www.manning.com/bauer3/>
>>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>>>> Spring Batch in Action <http://www.manning.com/templier/>
>>>> Blog: http://garygregory.wordpress.com
>>>> Home: http://garygregory.com/
>>>> Tweet! http://twitter.com/GaryGregory
>>>>
>>>
>>>
>>
>>
>> --
>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>> Java Persistence with Hibernate, Second Edition
>> <http://www.manning.com/bauer3/>
>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>> Spring Batch in Action <http://www.manning.com/templier/>
>> Blog: http://garygregory.wordpress.com
>> Home: http://garygregory.com/
>> Tweet! http://twitter.com/GaryGregory
>>
>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> Java Persistence with Hibernate, Second Edition
> <http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
>
Re: logging-log4j2 git commit: LOG4J2-1281 LoggerConfig.getProperties()
should not allocate on each call.
Posted by Gary Gregory <ga...@gmail.com>.
You can be the Hemingway of Java code! ;-)
On Sun, Feb 21, 2016 at 6:04 PM, Gary Gregory <ga...@gmail.com>
wrote:
> Well, yeah, sure, as long as it makes sense...
>
>
> On Sun, Feb 21, 2016 at 6:01 PM, Remko Popma <re...@gmail.com>
> wrote:
>
>> You actually end up with really clean code where each method does only
>> one thing. Single responsibility principle etc.
>> Clean code is fast!
>>
>> On Mon, Feb 22, 2016 at 10:58 AM, Gary Gregory <ga...@gmail.com>
>> wrote:
>>
>>> The 35 bytecodes or less limit seems arbitrary. What about IBM JVMs?
>>> Other vendors'?
>>>
>>> I OK with refactoring as long as it makes sense but just doing it for an arbitrary
>>> byte limit if it makes the code harder to maintain seems like it could
>>> be... I'm not sure what the word is I'm even looking for here!
>>>
>>> Gary
>>>
>>> On Sun, Feb 21, 2016 at 5:49 PM, Remko Popma <re...@gmail.com>
>>> wrote:
>>>
>>>> I'd say log4j-api util, so it can be used in the log4j-api
>>>> implementation classes as well.
>>>>
>>>> I would not want to over-complicate this, but there are two ways in
>>>> which methods can be optimized for performance which are "fragile" in that
>>>> a marker of some kind may be useful:
>>>>
>>>> * refactor into small private methods 35 bytecodes or less (these are
>>>> inlined automatically by Hotspot). I've tried to be consistent in adding a
>>>> comment to such methods, but writing the same comment get tedious.
>>>> * reduce GC pressure by replacing methods or constructs that allocate
>>>> temporary objects with alternatives that don't. However, there are many,
>>>> many methods and most don't allocate. I would not want to annotate all
>>>> methods... Hmm, need to think about this one more.
>>>>
>>>>
>>>> On Mon, Feb 22, 2016 at 10:24 AM, Matt Sicker <bo...@gmail.com> wrote:
>>>>
>>>>> Do we need this in log4j-api anywhere, or can we stick it in
>>>>> log4j-core?
>>>>>
>>>>> On 19 February 2016 at 18:29, Remko Popma <re...@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> I like that idea very much.
>>>>>>
>>>>>> Sent from my iPhone
>>>>>>
>>>>>> On 2016/02/20, at 7:37, Matt Sicker <bo...@gmail.com> wrote:
>>>>>>
>>>>>> 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 <re...@gmail.com>
>>>>>> 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 <bo...@gmail.com> wrote:
>>>>>>>
>>>>>>>> That sounds like a far more robust solution :)
>>>>>>>>
>>>>>>>> On 19 February 2016 at 11:18, Remko Popma <re...@gmail.com>
>>>>>>>> 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 <
>>>>>>>>> garydgregory@gmail.com> 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: <rp...@apache.org>
>>>>>>>>>> Date: Feb 19, 2016 7:41 AM
>>>>>>>>>> Subject: logging-log4j2 git commit: LOG4J2-1281
>>>>>>>>>> LoggerConfig.getProperties() should not allocate on each call.
>>>>>>>>>> To: <co...@logging.apache.org>
>>>>>>>>>> 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 <rp...@apache.org>
>>>>>>>>>> Authored: Sat Feb 20 00:41:24 2016 +0900
>>>>>>>>>> Committer: rpopma <rp...@apache.org>
>>>>>>>>>> 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 <bo...@gmail.com>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Matt Sicker <bo...@gmail.com>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Matt Sicker <bo...@gmail.com>
>>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>>> Java Persistence with Hibernate, Second Edition
>>> <http://www.manning.com/bauer3/>
>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>>> Spring Batch in Action <http://www.manning.com/templier/>
>>> Blog: http://garygregory.wordpress.com
>>> Home: http://garygregory.com/
>>> Tweet! http://twitter.com/GaryGregory
>>>
>>
>>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> Java Persistence with Hibernate, Second Edition
> <http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
>
--
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory
Re: logging-log4j2 git commit: LOG4J2-1281 LoggerConfig.getProperties()
should not allocate on each call.
Posted by Gary Gregory <ga...@gmail.com>.
Well, yeah, sure, as long as it makes sense...
On Sun, Feb 21, 2016 at 6:01 PM, Remko Popma <re...@gmail.com> wrote:
> You actually end up with really clean code where each method does only one
> thing. Single responsibility principle etc.
> Clean code is fast!
>
> On Mon, Feb 22, 2016 at 10:58 AM, Gary Gregory <ga...@gmail.com>
> wrote:
>
>> The 35 bytecodes or less limit seems arbitrary. What about IBM JVMs?
>> Other vendors'?
>>
>> I OK with refactoring as long as it makes sense but just doing it for an arbitrary
>> byte limit if it makes the code harder to maintain seems like it could
>> be... I'm not sure what the word is I'm even looking for here!
>>
>> Gary
>>
>> On Sun, Feb 21, 2016 at 5:49 PM, Remko Popma <re...@gmail.com>
>> wrote:
>>
>>> I'd say log4j-api util, so it can be used in the log4j-api
>>> implementation classes as well.
>>>
>>> I would not want to over-complicate this, but there are two ways in
>>> which methods can be optimized for performance which are "fragile" in that
>>> a marker of some kind may be useful:
>>>
>>> * refactor into small private methods 35 bytecodes or less (these are
>>> inlined automatically by Hotspot). I've tried to be consistent in adding a
>>> comment to such methods, but writing the same comment get tedious.
>>> * reduce GC pressure by replacing methods or constructs that allocate
>>> temporary objects with alternatives that don't. However, there are many,
>>> many methods and most don't allocate. I would not want to annotate all
>>> methods... Hmm, need to think about this one more.
>>>
>>>
>>> On Mon, Feb 22, 2016 at 10:24 AM, Matt Sicker <bo...@gmail.com> wrote:
>>>
>>>> Do we need this in log4j-api anywhere, or can we stick it in log4j-core?
>>>>
>>>> On 19 February 2016 at 18:29, Remko Popma <re...@gmail.com>
>>>> wrote:
>>>>
>>>>> I like that idea very much.
>>>>>
>>>>> Sent from my iPhone
>>>>>
>>>>> On 2016/02/20, at 7:37, Matt Sicker <bo...@gmail.com> wrote:
>>>>>
>>>>> 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 <re...@gmail.com>
>>>>> 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 <bo...@gmail.com> wrote:
>>>>>>
>>>>>>> That sounds like a far more robust solution :)
>>>>>>>
>>>>>>> On 19 February 2016 at 11:18, Remko Popma <re...@gmail.com>
>>>>>>> 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 <
>>>>>>>> garydgregory@gmail.com> 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: <rp...@apache.org>
>>>>>>>>> Date: Feb 19, 2016 7:41 AM
>>>>>>>>> Subject: logging-log4j2 git commit: LOG4J2-1281
>>>>>>>>> LoggerConfig.getProperties() should not allocate on each call.
>>>>>>>>> To: <co...@logging.apache.org>
>>>>>>>>> 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 <rp...@apache.org>
>>>>>>>>> Authored: Sat Feb 20 00:41:24 2016 +0900
>>>>>>>>> Committer: rpopma <rp...@apache.org>
>>>>>>>>> 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 <bo...@gmail.com>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Matt Sicker <bo...@gmail.com>
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Matt Sicker <bo...@gmail.com>
>>>>
>>>
>>>
>>
>>
>> --
>> E-Mail: garydgregory@gmail.com | ggregory@apache.org
>> Java Persistence with Hibernate, Second Edition
>> <http://www.manning.com/bauer3/>
>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
>> Spring Batch in Action <http://www.manning.com/templier/>
>> Blog: http://garygregory.wordpress.com
>> Home: http://garygregory.com/
>> Tweet! http://twitter.com/GaryGregory
>>
>
>
--
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory
Re: logging-log4j2 git commit: LOG4J2-1281 LoggerConfig.getProperties()
should not allocate on each call.
Posted by Remko Popma <re...@gmail.com>.
You actually end up with really clean code where each method does only one
thing. Single responsibility principle etc.
Clean code is fast!
On Mon, Feb 22, 2016 at 10:58 AM, Gary Gregory <ga...@gmail.com>
wrote:
> The 35 bytecodes or less limit seems arbitrary. What about IBM JVMs?
> Other vendors'?
>
> I OK with refactoring as long as it makes sense but just doing it for an arbitrary
> byte limit if it makes the code harder to maintain seems like it could
> be... I'm not sure what the word is I'm even looking for here!
>
> Gary
>
> On Sun, Feb 21, 2016 at 5:49 PM, Remko Popma <re...@gmail.com>
> wrote:
>
>> I'd say log4j-api util, so it can be used in the log4j-api implementation
>> classes as well.
>>
>> I would not want to over-complicate this, but there are two ways in which
>> methods can be optimized for performance which are "fragile" in that a
>> marker of some kind may be useful:
>>
>> * refactor into small private methods 35 bytecodes or less (these are
>> inlined automatically by Hotspot). I've tried to be consistent in adding a
>> comment to such methods, but writing the same comment get tedious.
>> * reduce GC pressure by replacing methods or constructs that allocate
>> temporary objects with alternatives that don't. However, there are many,
>> many methods and most don't allocate. I would not want to annotate all
>> methods... Hmm, need to think about this one more.
>>
>>
>> On Mon, Feb 22, 2016 at 10:24 AM, Matt Sicker <bo...@gmail.com> wrote:
>>
>>> Do we need this in log4j-api anywhere, or can we stick it in log4j-core?
>>>
>>> On 19 February 2016 at 18:29, Remko Popma <re...@gmail.com> wrote:
>>>
>>>> I like that idea very much.
>>>>
>>>> Sent from my iPhone
>>>>
>>>> On 2016/02/20, at 7:37, Matt Sicker <bo...@gmail.com> wrote:
>>>>
>>>> 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 <re...@gmail.com>
>>>> 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 <bo...@gmail.com> wrote:
>>>>>
>>>>>> That sounds like a far more robust solution :)
>>>>>>
>>>>>> On 19 February 2016 at 11:18, Remko Popma <re...@gmail.com>
>>>>>> 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 <
>>>>>>> garydgregory@gmail.com> 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: <rp...@apache.org>
>>>>>>>> Date: Feb 19, 2016 7:41 AM
>>>>>>>> Subject: logging-log4j2 git commit: LOG4J2-1281
>>>>>>>> LoggerConfig.getProperties() should not allocate on each call.
>>>>>>>> To: <co...@logging.apache.org>
>>>>>>>> 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 <rp...@apache.org>
>>>>>>>> Authored: Sat Feb 20 00:41:24 2016 +0900
>>>>>>>> Committer: rpopma <rp...@apache.org>
>>>>>>>> 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 <bo...@gmail.com>
>>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Matt Sicker <bo...@gmail.com>
>>>>
>>>>
>>>
>>>
>>> --
>>> Matt Sicker <bo...@gmail.com>
>>>
>>
>>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> Java Persistence with Hibernate, Second Edition
> <http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
>
Re: logging-log4j2 git commit: LOG4J2-1281 LoggerConfig.getProperties()
should not allocate on each call.
Posted by Gary Gregory <ga...@gmail.com>.
The 35 bytecodes or less limit seems arbitrary. What about IBM JVMs? Other
vendors'?
I OK with refactoring as long as it makes sense but just doing it for
an arbitrary
byte limit if it makes the code harder to maintain seems like it could
be... I'm not sure what the word is I'm even looking for here!
Gary
On Sun, Feb 21, 2016 at 5:49 PM, Remko Popma <re...@gmail.com> wrote:
> I'd say log4j-api util, so it can be used in the log4j-api implementation
> classes as well.
>
> I would not want to over-complicate this, but there are two ways in which
> methods can be optimized for performance which are "fragile" in that a
> marker of some kind may be useful:
>
> * refactor into small private methods 35 bytecodes or less (these are
> inlined automatically by Hotspot). I've tried to be consistent in adding a
> comment to such methods, but writing the same comment get tedious.
> * reduce GC pressure by replacing methods or constructs that allocate
> temporary objects with alternatives that don't. However, there are many,
> many methods and most don't allocate. I would not want to annotate all
> methods... Hmm, need to think about this one more.
>
>
> On Mon, Feb 22, 2016 at 10:24 AM, Matt Sicker <bo...@gmail.com> wrote:
>
>> Do we need this in log4j-api anywhere, or can we stick it in log4j-core?
>>
>> On 19 February 2016 at 18:29, Remko Popma <re...@gmail.com> wrote:
>>
>>> I like that idea very much.
>>>
>>> Sent from my iPhone
>>>
>>> On 2016/02/20, at 7:37, Matt Sicker <bo...@gmail.com> wrote:
>>>
>>> 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 <re...@gmail.com> 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 <bo...@gmail.com> wrote:
>>>>
>>>>> That sounds like a far more robust solution :)
>>>>>
>>>>> On 19 February 2016 at 11:18, Remko Popma <re...@gmail.com>
>>>>> 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 <garydgregory@gmail.com
>>>>>> > 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: <rp...@apache.org>
>>>>>>> Date: Feb 19, 2016 7:41 AM
>>>>>>> Subject: logging-log4j2 git commit: LOG4J2-1281
>>>>>>> LoggerConfig.getProperties() should not allocate on each call.
>>>>>>> To: <co...@logging.apache.org>
>>>>>>> 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 <rp...@apache.org>
>>>>>>> Authored: Sat Feb 20 00:41:24 2016 +0900
>>>>>>> Committer: rpopma <rp...@apache.org>
>>>>>>> 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 <bo...@gmail.com>
>>>>>
>>>>
>>>
>>>
>>> --
>>> Matt Sicker <bo...@gmail.com>
>>>
>>>
>>
>>
>> --
>> Matt Sicker <bo...@gmail.com>
>>
>
>
--
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory
Re: logging-log4j2 git commit: LOG4J2-1281 LoggerConfig.getProperties()
should not allocate on each call.
Posted by Remko Popma <re...@gmail.com>.
I'd say log4j-api util, so it can be used in the log4j-api implementation
classes as well.
I would not want to over-complicate this, but there are two ways in which
methods can be optimized for performance which are "fragile" in that a
marker of some kind may be useful:
* refactor into small private methods 35 bytecodes or less (these are
inlined automatically by Hotspot). I've tried to be consistent in adding a
comment to such methods, but writing the same comment get tedious.
* reduce GC pressure by replacing methods or constructs that allocate
temporary objects with alternatives that don't. However, there are many,
many methods and most don't allocate. I would not want to annotate all
methods... Hmm, need to think about this one more.
On Mon, Feb 22, 2016 at 10:24 AM, Matt Sicker <bo...@gmail.com> wrote:
> Do we need this in log4j-api anywhere, or can we stick it in log4j-core?
>
> On 19 February 2016 at 18:29, Remko Popma <re...@gmail.com> wrote:
>
>> I like that idea very much.
>>
>> Sent from my iPhone
>>
>> On 2016/02/20, at 7:37, Matt Sicker <bo...@gmail.com> wrote:
>>
>> 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 <re...@gmail.com> 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 <bo...@gmail.com> wrote:
>>>
>>>> That sounds like a far more robust solution :)
>>>>
>>>> On 19 February 2016 at 11:18, Remko Popma <re...@gmail.com>
>>>> 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 <ga...@gmail.com>
>>>>> 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: <rp...@apache.org>
>>>>>> Date: Feb 19, 2016 7:41 AM
>>>>>> Subject: logging-log4j2 git commit: LOG4J2-1281
>>>>>> LoggerConfig.getProperties() should not allocate on each call.
>>>>>> To: <co...@logging.apache.org>
>>>>>> 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 <rp...@apache.org>
>>>>>> Authored: Sat Feb 20 00:41:24 2016 +0900
>>>>>> Committer: rpopma <rp...@apache.org>
>>>>>> 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 <bo...@gmail.com>
>>>>
>>>
>>
>>
>> --
>> Matt Sicker <bo...@gmail.com>
>>
>>
>
>
> --
> Matt Sicker <bo...@gmail.com>
>
Re: logging-log4j2 git commit: LOG4J2-1281 LoggerConfig.getProperties()
should not allocate on each call.
Posted by Matt Sicker <bo...@gmail.com>.
Do we need this in log4j-api anywhere, or can we stick it in log4j-core?
On 19 February 2016 at 18:29, Remko Popma <re...@gmail.com> wrote:
> I like that idea very much.
>
> Sent from my iPhone
>
> On 2016/02/20, at 7:37, Matt Sicker <bo...@gmail.com> wrote:
>
> 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 <re...@gmail.com> 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 <bo...@gmail.com> wrote:
>>
>>> That sounds like a far more robust solution :)
>>>
>>> On 19 February 2016 at 11:18, Remko Popma <re...@gmail.com> 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 <ga...@gmail.com>
>>>> 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: <rp...@apache.org>
>>>>> Date: Feb 19, 2016 7:41 AM
>>>>> Subject: logging-log4j2 git commit: LOG4J2-1281
>>>>> LoggerConfig.getProperties() should not allocate on each call.
>>>>> To: <co...@logging.apache.org>
>>>>> 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 <rp...@apache.org>
>>>>> Authored: Sat Feb 20 00:41:24 2016 +0900
>>>>> Committer: rpopma <rp...@apache.org>
>>>>> 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 <bo...@gmail.com>
>>>
>>
>
>
> --
> Matt Sicker <bo...@gmail.com>
>
>
--
Matt Sicker <bo...@gmail.com>
Re: logging-log4j2 git commit: LOG4J2-1281 LoggerConfig.getProperties() should not allocate on each call.
Posted by Remko Popma <re...@gmail.com>.
I like that idea very much.
Sent from my iPhone
> On 2016/02/20, at 7:37, Matt Sicker <bo...@gmail.com> wrote:
>
> 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 <re...@gmail.com> 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 <bo...@gmail.com> wrote:
>>> That sounds like a far more robust solution :)
>>>
>>>> On 19 February 2016 at 11:18, Remko Popma <re...@gmail.com> 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 <ga...@gmail.com> 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: <rp...@apache.org>
>>>>> Date: Feb 19, 2016 7:41 AM
>>>>> Subject: logging-log4j2 git commit: LOG4J2-1281 LoggerConfig.getProperties() should not allocate on each call.
>>>>> To: <co...@logging.apache.org>
>>>>> 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 <rp...@apache.org>
>>>>> Authored: Sat Feb 20 00:41:24 2016 +0900
>>>>> Committer: rpopma <rp...@apache.org>
>>>>> 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 <bo...@gmail.com>
>
>
>
> --
> Matt Sicker <bo...@gmail.com>
Re: logging-log4j2 git commit: LOG4J2-1281 LoggerConfig.getProperties()
should not allocate on each call.
Posted by Matt Sicker <bo...@gmail.com>.
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 <re...@gmail.com> 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 <bo...@gmail.com> wrote:
>
>> That sounds like a far more robust solution :)
>>
>> On 19 February 2016 at 11:18, Remko Popma <re...@gmail.com> 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 <ga...@gmail.com>
>>> 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: <rp...@apache.org>
>>>> Date: Feb 19, 2016 7:41 AM
>>>> Subject: logging-log4j2 git commit: LOG4J2-1281
>>>> LoggerConfig.getProperties() should not allocate on each call.
>>>> To: <co...@logging.apache.org>
>>>> 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 <rp...@apache.org>
>>>> Authored: Sat Feb 20 00:41:24 2016 +0900
>>>> Committer: rpopma <rp...@apache.org>
>>>> 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 <bo...@gmail.com>
>>
>
--
Matt Sicker <bo...@gmail.com>
Re: logging-log4j2 git commit: LOG4J2-1281 LoggerConfig.getProperties()
should not allocate on each call.
Posted by Remko Popma <re...@gmail.com>.
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 <bo...@gmail.com> wrote:
> That sounds like a far more robust solution :)
>
> On 19 February 2016 at 11:18, Remko Popma <remko.popma@gmail.com
> <javascript:_e(%7B%7D,'cvml','remko.popma@gmail.com');>> 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 <garydgregory@gmail.com
>> <javascript:_e(%7B%7D,'cvml','garydgregory@gmail.com');>> 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: <rpopma@apache.org
>>> <javascript:_e(%7B%7D,'cvml','rpopma@apache.org');>>
>>> Date: Feb 19, 2016 7:41 AM
>>> Subject: logging-log4j2 git commit: LOG4J2-1281
>>> LoggerConfig.getProperties() should not allocate on each call.
>>> To: <commits@logging.apache.org
>>> <javascript:_e(%7B%7D,'cvml','commits@logging.apache.org');>>
>>> 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 <rpopma@apache.org
>>> <javascript:_e(%7B%7D,'cvml','rpopma@apache.org');>>
>>> Authored: Sat Feb 20 00:41:24 2016 +0900
>>> Committer: rpopma <rpopma@apache.org
>>> <javascript:_e(%7B%7D,'cvml','rpopma@apache.org');>>
>>> 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 <boards@gmail.com
> <javascript:_e(%7B%7D,'cvml','boards@gmail.com');>>
>
Re: logging-log4j2 git commit: LOG4J2-1281 LoggerConfig.getProperties()
should not allocate on each call.
Posted by Matt Sicker <bo...@gmail.com>.
That sounds like a far more robust solution :)
On 19 February 2016 at 11:18, Remko Popma <re...@gmail.com> 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 <ga...@gmail.com>
> 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: <rp...@apache.org>
>> Date: Feb 19, 2016 7:41 AM
>> Subject: logging-log4j2 git commit: LOG4J2-1281
>> LoggerConfig.getProperties() should not allocate on each call.
>> To: <co...@logging.apache.org>
>> 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 <rp...@apache.org>
>> Authored: Sat Feb 20 00:41:24 2016 +0900
>> Committer: rpopma <rp...@apache.org>
>> 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 <bo...@gmail.com>
Re: logging-log4j2 git commit: LOG4J2-1281 LoggerConfig.getProperties()
should not allocate on each call.
Posted by Remko Popma <re...@gmail.com>.
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 <ga...@gmail.com>
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: <rp...@apache.org>
> Date: Feb 19, 2016 7:41 AM
> Subject: logging-log4j2 git commit: LOG4J2-1281
> LoggerConfig.getProperties() should not allocate on each call.
> To: <co...@logging.apache.org>
> 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 <rp...@apache.org>
> Authored: Sat Feb 20 00:41:24 2016 +0900
> Committer: rpopma <rp...@apache.org>
> 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>
>
>
Re: logging-log4j2 git commit: LOG4J2-1281 LoggerConfig.getProperties()
should not allocate on each call.
Posted by Matt Sicker <bo...@gmail.com>.
Agreed. Don't want to accidentally refactor performance-sensitive code! We
have similar comments in Marker.
On 19 February 2016 at 11:03, Gary Gregory <ga...@gmail.com> 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: <rp...@apache.org>
> Date: Feb 19, 2016 7:41 AM
> Subject: logging-log4j2 git commit: LOG4J2-1281
> LoggerConfig.getProperties() should not allocate on each call.
> To: <co...@logging.apache.org>
> 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 <rp...@apache.org>
> Authored: Sat Feb 20 00:41:24 2016 +0900
> Committer: rpopma <rp...@apache.org>
> 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 <bo...@gmail.com>