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 <[email protected]> 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 <[email protected]> > 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 <[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]> >>> >> >> > > > -- > E-Mail: [email protected] | [email protected] > 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 >
