Well, yeah, sure, as long as it makes sense... On Sun, Feb 21, 2016 at 6:01 PM, Remko Popma <[email protected]> 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 <[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 >> > > -- 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
