logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@gmail.com>
Subject Re: Perf code
Date Wed, 11 Nov 2015 16:11:42 GMT
I suggest you add a comment there too or preferably refactor the
duplication.

Gary
On Nov 11, 2015 8:05 AM, "Remko Popma" <remko.popma@gmail.com> 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 <garydgregory@gmail.com>
> 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, <rpopma@apache.org> 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 <rpopma@apache.org>
>> > Authored: Wed Nov 11 22:48:42 2015 +0900
>> > Committer: rpopma <rpopma@apache.org>
>> > 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,  &lt;<a
>> href=3D"mailto:=
>> rpopma@apache.org">rpopma@apache.org</a>&gt; 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 -&gt; 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 &lt;<a href=3D"mailto:rpopma@apache.org">rpopma@apache.org
>> </=
>> a>&gt;<br>
>> Authored: Wed Nov 11 22:48:42 2015 +0900<br>
>> Committer: rpopma &lt;<a href=3D"mailto:rpopma@apache.org
>> ">rpopma@apache.or=
>> g</a>&gt;<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&lt;action issue=3D&quot;LOG4J2-1187&quot;
dev=
>> =3D&quot;ggregory&quot; type=3D&quot;add&quot;&gt;<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&lt;/action&gt;<br>
>> +=C2=A0 =C2=A0 =C2=A0 &lt;action issue=3D&quot;LOG4J2-1029&quot;
>> dev=3D&quo=
>> t;rpopma&quot; type=3D&quot;fix&quot; due-to=3D&quot;Stefan
>> Leonhartsberger=
>> &quot;&gt;<br>
>> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Performance improvement when gathering
>> locatio=
>> n information.<br>
>> +=C2=A0 =C2=A0 =C2=A0 &lt;/action&gt;<br>
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0&lt;action issue=3D&quot;LOG4J2-1172&quot;
dev=
>> =3D&quot;rpopma&quot; type=3D&quot;fix&quot;&gt;<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&lt;/action&gt;<br>
>> <br>
>> </blockquote></div>
>>
>> --001a11419fd83af63a052445b36d--
>>
>
>

Mime
View raw message