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-- >> > >
