Thank you for checking. Gary
On Wed, Nov 11, 2015 at 8:45 AM, Remko Popma <[email protected]> wrote: > Gary, looks like the code in ReflectionUtil already has a comment with a > reference to the log4j-perf benchmark. (Nice, Matt!) > The duplication in this case is just "new Throwable()", so I think we're > good. > > On Thu, Nov 12, 2015 at 1:16 AM, Matt Sicker <[email protected]> wrote: > >> I know I used that in ReflectionUtil because of the log4j-perf test that >> demonstrates the difference between Throwable and Thread. >> >> On 11 November 2015 at 10:11, Gary Gregory <[email protected]> >> wrote: >> >>> I suggest you add a comment there too or preferably refactor the >>> duplication. >>> >>> Gary >>> On Nov 11, 2015 8:05 AM, "Remko Popma" <[email protected]> wrote: >>> >>>> I agree I should have added a ref to the JIRA to document the reason >>>> for doing it this way. I added a comment just now. >>>> >>>> About moving this to a util class, I'd be fine with that. >>>> FYI, the other place that uses {{new Throwable().getStackTrace()}} is >>>> ReflectionUtil#getEquivalentStackTraceElement. >>>> >>>> >>>> On Thu, Nov 12, 2015 at 12:48 AM, Gary Gregory <[email protected]> >>>> wrote: >>>> >>>>> I think this needs a code comment to avoid the code being changed in >>>>> the >>>>> future to undo the improvement. >>>>> >>>>> IMO all perf changes like this need good docs with a ref to the Jira. >>>>> Who >>>>> knows how this kind of stuff will play out on top on Java 8 and 9. >>>>> >>>>> If we do this kind of call in different places, we should abstract it >>>>> out >>>>> as well in a util method which can be amply documented. >>>>> >>>>> Gary >>>>> >>>>> On Nov 11, 2015 5:48 AM, <[email protected]> wrote: >>>>> >>>>> > Repository: logging-log4j2 >>>>> > Updated Branches: >>>>> > refs/heads/master 381acc0e3 -> 65adfab1b >>>>> > >>>>> > >>>>> > LOG4J2-1029 Performance improvement when gathering location >>>>> information >>>>> > >>>>> > - Use new Throwable().getStackTrace() instead of >>>>> > Thread.currentThread().getStackTrace() >>>>> > >>>>> > Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo >>>>> > Commit: >>>>> > >>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/65adfab1 >>>>> > Tree: >>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/65adfab1 >>>>> > Diff: >>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/65adfab1 >>>>> > >>>>> > Branch: refs/heads/master >>>>> > Commit: 65adfab1b6afcbffde767b5d8a3213341c84c1eb >>>>> > Parents: 381acc0 >>>>> > Author: rpopma <[email protected]> >>>>> > Authored: Wed Nov 11 22:48:42 2015 +0900 >>>>> > Committer: rpopma <[email protected]> >>>>> > Committed: Wed Nov 11 22:48:42 2015 +0900 >>>>> > >>>>> > >>>>> ---------------------------------------------------------------------- >>>>> > .../java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java | >>>>> 2 +- >>>>> > src/changes/changes.xml | >>>>> 3 +++ >>>>> > 2 files changed, 4 insertions(+), 1 deletion(-) >>>>> > >>>>> ---------------------------------------------------------------------- >>>>> > >>>>> > >>>>> > >>>>> > >>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/65adfab1/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java >>>>> > >>>>> ---------------------------------------------------------------------- >>>>> > diff --git >>>>> > >>>>> a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java >>>>> > >>>>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java >>>>> > index d74d6e2..a6c2087 100644 >>>>> > --- >>>>> > >>>>> a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java >>>>> > +++ >>>>> > >>>>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java >>>>> > @@ -542,7 +542,7 @@ public Log4jLogEvent(final String loggerName, >>>>> final >>>>> > Marker marker, final String >>>>> > if (fqcnOfLogger == null) { >>>>> > return null; >>>>> > } >>>>> > - final StackTraceElement[] stackTrace = >>>>> > Thread.currentThread().getStackTrace(); >>>>> > + final StackTraceElement[] stackTrace = new >>>>> > Throwable().getStackTrace(); >>>>> > StackTraceElement last = null; >>>>> > for (int i = stackTrace.length - 1; i > 0; i--) { >>>>> > final String className = stackTrace[i].getClassName(); >>>>> > >>>>> > >>>>> > >>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/65adfab1/src/changes/changes.xml >>>>> > >>>>> ---------------------------------------------------------------------- >>>>> > diff --git a/src/changes/changes.xml b/src/changes/changes.xml >>>>> > index c82cc9d..e6b2f97 100644 >>>>> > --- a/src/changes/changes.xml >>>>> > +++ b/src/changes/changes.xml >>>>> > @@ -42,6 +42,9 @@ >>>>> > <action issue="LOG4J2-1187" dev="ggregory" type="add"> >>>>> > Support use case for >>>>> > java.sql.DriverManager.setLogStream(PrintStream). >>>>> > </action> >>>>> > + <action issue="LOG4J2-1029" dev="rpopma" type="fix" >>>>> due-to="Stefan >>>>> > Leonhartsberger"> >>>>> > + Performance improvement when gathering location information. >>>>> > + </action> >>>>> > <action issue="LOG4J2-1172" dev="rpopma" type="fix"> >>>>> > Fixed ThreadLocal leak [AsyncLogger$Info] on Tomcat when >>>>> using >>>>> > AsyncLoggerContextSelector. >>>>> > </action> >>>>> > >>>>> > >>>>> >>>>> --001a11419fd83af63a052445b36d >>>>> Content-Type: text/html; charset=UTF-8 >>>>> Content-Transfer-Encoding: quoted-printable >>>>> >>>>> <p dir=3D"ltr">I think this needs a code comment to avoid the code >>>>> being ch= >>>>> anged in the future to undo the improvement. </p> >>>>> <p dir=3D"ltr">IMO all perf changes like this need good docs with a >>>>> ref to = >>>>> the Jira. Who knows how this kind of stuff will play out on top on >>>>> Java 8 a= >>>>> nd 9. </p> >>>>> <p dir=3D"ltr">If we do this kind of call in different places, we >>>>> should ab= >>>>> stract it out as well in a util method which can be amply >>>>> documented.</p> >>>>> <p dir=3D"ltr">Gary</p> >>>>> <div class=3D"gmail_quote">On Nov 11, 2015 5:48 AM, <<a >>>>> href=3D"mailto:= >>>>> [email protected]">[email protected]</a>> wrote:<br >>>>> type=3D"attribution"= >>>>> ><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 >>>>> .8ex;border-left:1= >>>>> px #ccc solid;padding-left:1ex">Repository: logging-log4j2<br> >>>>> Updated Branches:<br> >>>>> =C2=A0 refs/heads/master 381acc0e3 -> 65adfab1b<br> >>>>> <br> >>>>> <br> >>>>> LOG4J2-1029 Performance improvement when gathering location >>>>> information<br> >>>>> <br> >>>>> - Use new Throwable().getStackTrace() instead of<br> >>>>> Thread.currentThread().getStackTrace()<br> >>>>> <br> >>>>> Project: <a href=3D" >>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/r= >>>>> epo" rel=3D"noreferrer" target=3D"_blank"> >>>>> http://git-wip-us.apache.org/repo= >>>>> s/asf/logging-log4j2/repo</a><br> >>>>> Commit: <a href=3D" >>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/co= >>>>> mmit/65adfab1" rel=3D"noreferrer" target=3D"_blank"> >>>>> http://git-wip-us.apach= >>>>> e.org/repos/asf/logging-log4j2/commit/65adfab1</a><br> >>>>> Tree: <a href=3D" >>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree= >>>>> /65adfab1" rel=3D"noreferrer" target=3D"_blank"> >>>>> http://git-wip-us.apache.or= >>>>> g/repos/asf/logging-log4j2/tree/65adfab1</a><br> >>>>> Diff: <a href=3D" >>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff= >>>>> /65adfab1" rel=3D"noreferrer" target=3D"_blank"> >>>>> http://git-wip-us.apache.or= >>>>> g/repos/asf/logging-log4j2/diff/65adfab1</a><br> >>>>> <br> >>>>> Branch: refs/heads/master<br> >>>>> Commit: 65adfab1b6afcbffde767b5d8a3213341c84c1eb<br> >>>>> Parents: 381acc0<br> >>>>> Author: rpopma <<a href=3D"mailto:[email protected]";> >>>>> [email protected]</= >>>>> a>><br> >>>>> Authored: Wed Nov 11 22:48:42 2015 +0900<br> >>>>> Committer: rpopma <<a href=3D"mailto:[email protected] >>>>> ">[email protected]= >>>>> g</a>><br> >>>>> Committed: Wed Nov 11 22:48:42 2015 +0900<br> >>>>> <br> >>>>> >>>>> ----------------------------------------------------------------------<br> >>>>> =C2=A0.../java/org/apache/logging/log4j/core/impl/Log4jLogEvent.java=C2=A0 >>>>> = >>>>> =C2=A0 | 2 +-<br> >>>>> =C2=A0src/changes/changes.xml=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 >>>>> =C2= >>>>> =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 >>>>> =C2=A0 = >>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| 3 +++<br> >>>>> =C2=A02 files changed, 4 insertions(+), 1 deletion(-)<br> >>>>> >>>>> ----------------------------------------------------------------------<br> >>>>> <br> >>>>> <br> >>>>> <a href=3D" >>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/65adf= >>>>> >>>>> ab1/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogEve= >>>>> nt.java" rel=3D"noreferrer" target=3D"_blank"> >>>>> http://git-wip-us.apache.org/= >>>>> >>>>> repos/asf/logging-log4j2/blob/65adfab1/log4j-core/src/main/java/org/apache/= >>>>> logging/log4j/core/impl/Log4jLogEvent.java</a><br> >>>>> >>>>> ----------------------------------------------------------------------<br> >>>>> diff --git >>>>> a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Lo= >>>>> g4jLogEvent.java >>>>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/i= >>>>> mpl/Log4jLogEvent.java<br> >>>>> index d74d6e2..a6c2087 100644<br> >>>>> --- >>>>> a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogE= >>>>> vent.java<br> >>>>> +++ >>>>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/Log4jLogE= >>>>> vent.java<br> >>>>> @@ -542,7 +542,7 @@ public Log4jLogEvent(final String loggerName, >>>>> final Mar= >>>>> ker marker, final String<br> >>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (fqcnOfLogger =3D=3D null) {<br> >>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return null;<br> >>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}<br> >>>>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 final StackTraceElement[] stackTrace =3D >>>>> Threa= >>>>> d.currentThread().getStackTrace();<br> >>>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 final StackTraceElement[] stackTrace =3D >>>>> new T= >>>>> hrowable().getStackTrace();<br> >>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0StackTraceElement last =3D null;<br> >>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0for (int i =3D stackTrace.length - >>>>> 1; i &= >>>>> gt; 0; i--) {<br> >>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0final String className >>>>> =3D = >>>>> stackTrace[i].getClassName();<br> >>>>> <br> >>>>> <a href=3D" >>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/65adf= >>>>> ab1/src/changes/changes.xml" rel=3D"noreferrer" target=3D"_blank"> >>>>> http://gi= >>>>> >>>>> t-wip-us.apache.org/repos/asf/logging-log4j2/blob/65adfab1/src/changes/chan= >>>>> ges.xml</a><br> >>>>> >>>>> ----------------------------------------------------------------------<br> >>>>> diff --git a/src/changes/changes.xml b/src/changes/changes.xml<br> >>>>> index c82cc9d..e6b2f97 100644<br> >>>>> --- a/src/changes/changes.xml<br> >>>>> +++ b/src/changes/changes.xml<br> >>>>> @@ -42,6 +42,9 @@<br> >>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0<action issue=3D"LOG4J2-1187" >>>>> dev= >>>>> =3D"ggregory" type=3D"add"><br> >>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Support use case for >>>>> java.sql.DriverManag= >>>>> er.setLogStream(PrintStream).<br> >>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0</action><br> >>>>> +=C2=A0 =C2=A0 =C2=A0 <action issue=3D"LOG4J2-1029" >>>>> dev=3D&quo= >>>>> t;rpopma" type=3D"fix" due-to=3D"Stefan >>>>> Leonhartsberger= >>>>> "><br> >>>>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Performance improvement when gathering >>>>> locatio= >>>>> n information.<br> >>>>> +=C2=A0 =C2=A0 =C2=A0 </action><br> >>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0<action issue=3D"LOG4J2-1172" >>>>> dev= >>>>> =3D"rpopma" type=3D"fix"><br> >>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Fixed ThreadLocal leak >>>>> [AsyncLogger$Info]= >>>>> on Tomcat when using AsyncLoggerContextSelector.<br> >>>>> =C2=A0 =C2=A0 =C2=A0 =C2=A0</action><br> >>>>> <br> >>>>> </blockquote></div> >>>>> >>>>> --001a11419fd83af63a052445b36d-- >>>>> >>>> >>>> >> >> >> -- >> 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
