Haha! :-) Is that good or bad? :-O On Mon, Feb 22, 2016 at 11:04 AM, Gary Gregory <[email protected]> wrote:
> You can be the Hemingway of Java code! ;-) > > On Sun, Feb 21, 2016 at 6:04 PM, Gary Gregory <[email protected]> > wrote: > >> 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 >> > > > > -- > 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 >
