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

Reply via email to