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