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