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
