You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@logging.apache.org by Ralph Goers <ra...@dslextreme.com> on 2018/08/07 23:20:40 UTC
Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve
ThrowableProxy perfo...
I have to wonder why using the security manager is faster than using StackWalker. StackWalker was created just for this purpose. Is the way it is implemented the problem?
Ralph
> On Aug 7, 2018, at 1:34 PM, cakofony <gi...@git.apache.org> wrote:
>
> GitHub user cakofony opened a pull request:
>
> https://github.com/apache/logging-log4j2/pull/202
>
> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>
> Share the SecurityManager implementation of getCurrentStackTrace
> with the java 9 implementation.
>
> You can merge this pull request into a Git repository by running:
>
> $ git pull https://github.com/cakofony/logging-log4j2 java9_getcurrentstacktrace
>
> Alternatively you can review and apply these changes as the patch at:
>
> https://github.com/apache/logging-log4j2/pull/202.patch
>
> To close this pull request, make a commit to your master/trunk branch
> with (at least) the following in the commit message:
>
> This closes #202
>
> ----
> commit 52bf8569e8e9881d0999156c31d99b99f28e6e73
> Author: Carter Kozak <ck...@...>
> Date: 2018-08-07T20:27:10Z
>
> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>
> Share the SecurityManager implementation of getCurrentStackTrace
> with the java 9 implementation.
>
> ----
>
>
> ---
>
Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve
ThrowableProxy perfo...
Posted by Ralph Goers <ra...@dslextreme.com>.
Yup. Part of the problem is that StackWalker is optimized for only walking a few frames. I think it caches 8 stack frames. After that it apparently slows down a lot. I don’t recall there being any way to tune that.
Ralph
> On Aug 8, 2018, at 5:07 PM, Remko Popma <re...@gmail.com> wrote:
>
> What I remember from email interactions that Ralph had with the people working on the Stackwalker implementation:
>
> Ostensibly Stackwalker was supposed to give performance benefits for use cases like ours. They even mentioned discovering the location of a logging call as one of the main use cases.
>
> They suggested a certain way of using the stackwalker API that gave very good performance. (I believe walking from the top down.)
> Then, when Ralph pointed out that this usage didn’t meet our requirements because of potential recursion (the actual first call into the logging framework may be a few frames deeper), they kind of ignored that and just continued with their previous plan.
> So we ended up with a stackwalker API that is designed for a different use case than ours and doesn’t help us much.
>
> Ralph, does that match your memories of what happened?
>
> Remko
>
> (Shameless plug) Every java main() method deserves http://picocli.info
>
>> On Aug 9, 2018, at 0:35, Matt Sicker <bo...@gmail.com> wrote:
>>
>> StackWalker seems to be implemented purely as it says: walking the call
>> stack. If all you need is the stack of classes (e.g., for checking runtime
>> permissions based on the calling class), then the SecurityManager makes
>> most sense since it seems built for that exact purpose. I'd imagine there
>> are optimizations in place considering how often that class stack is
>> checked at runtime by just the JDK.
>>
>>> On Tue, 7 Aug 2018 at 22:10, Carter Kozak <c4...@gmail.com> wrote:
>>>
>>> My guess is that StackWalker provides additional information beyond
>>> the array of classes
>>> supplied by the SecurityManager, though I've not done a thorough analysis
>>> yet.
>>>
>>> A quick targeted benchmark of our current implementations:
>>>
>>> Benchmark Mode Cnt
>>> Score Error Units
>>> StackTraceBenchmark.defaultJava8 thrpt 3 113965.921 ±
>>> 119706.986 ops/s
>>> StackTraceBenchmark.securityManager thrpt 3 788004.237 ± 82578.567
>>> ops/s
>>> StackTraceBenchmark.stackWalker thrpt 3 182902.031 ±
>>> 39018.395 ops/s
>>>
>>>
>>> -ck
>>>
>>> On Tue, Aug 7, 2018 at 7:20 PM, Ralph Goers <ra...@dslextreme.com>
>>> wrote:
>>>> I have to wonder why using the security manager is faster than using
>>> StackWalker. StackWalker was created just for this purpose. Is the way it
>>> is implemented the problem?
>>>>
>>>> Ralph
>>>>
>>>>
>>>>> On Aug 7, 2018, at 1:34 PM, cakofony <gi...@git.apache.org> wrote:
>>>>>
>>>>> GitHub user cakofony opened a pull request:
>>>>>
>>>>> https://github.com/apache/logging-log4j2/pull/202
>>>>>
>>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>>>
>>>>> Share the SecurityManager implementation of getCurrentStackTrace
>>>>> with the java 9 implementation.
>>>>>
>>>>> You can merge this pull request into a Git repository by running:
>>>>>
>>>>> $ git pull https://github.com/cakofony/logging-log4j2
>>> java9_getcurrentstacktrace
>>>>>
>>>>> Alternatively you can review and apply these changes as the patch at:
>>>>>
>>>>> https://github.com/apache/logging-log4j2/pull/202.patch
>>>>>
>>>>> To close this pull request, make a commit to your master/trunk branch
>>>>> with (at least) the following in the commit message:
>>>>>
>>>>> This closes #202
>>>>>
>>>>> ----
>>>>> commit 52bf8569e8e9881d0999156c31d99b99f28e6e73
>>>>> Author: Carter Kozak <ck...@...>
>>>>> Date: 2018-08-07T20:27:10Z
>>>>>
>>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>>>
>>>>> Share the SecurityManager implementation of getCurrentStackTrace
>>>>> with the java 9 implementation.
>>>>>
>>>>> ----
>>>>>
>>>>>
>>>>> ---
>>>>>
>>>>
>>>>
>>>
>>
>>
>> --
>> Matt Sicker <bo...@gmail.com>
>
Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve ThrowableProxy perfo...
Posted by Remko Popma <re...@gmail.com>.
What I remember from email interactions that Ralph had with the people working on the Stackwalker implementation:
Ostensibly Stackwalker was supposed to give performance benefits for use cases like ours. They even mentioned discovering the location of a logging call as one of the main use cases.
They suggested a certain way of using the stackwalker API that gave very good performance. (I believe walking from the top down.)
Then, when Ralph pointed out that this usage didn’t meet our requirements because of potential recursion (the actual first call into the logging framework may be a few frames deeper), they kind of ignored that and just continued with their previous plan.
So we ended up with a stackwalker API that is designed for a different use case than ours and doesn’t help us much.
Ralph, does that match your memories of what happened?
Remko
(Shameless plug) Every java main() method deserves http://picocli.info
> On Aug 9, 2018, at 0:35, Matt Sicker <bo...@gmail.com> wrote:
>
> StackWalker seems to be implemented purely as it says: walking the call
> stack. If all you need is the stack of classes (e.g., for checking runtime
> permissions based on the calling class), then the SecurityManager makes
> most sense since it seems built for that exact purpose. I'd imagine there
> are optimizations in place considering how often that class stack is
> checked at runtime by just the JDK.
>
>> On Tue, 7 Aug 2018 at 22:10, Carter Kozak <c4...@gmail.com> wrote:
>>
>> My guess is that StackWalker provides additional information beyond
>> the array of classes
>> supplied by the SecurityManager, though I've not done a thorough analysis
>> yet.
>>
>> A quick targeted benchmark of our current implementations:
>>
>> Benchmark Mode Cnt
>> Score Error Units
>> StackTraceBenchmark.defaultJava8 thrpt 3 113965.921 ±
>> 119706.986 ops/s
>> StackTraceBenchmark.securityManager thrpt 3 788004.237 ± 82578.567
>> ops/s
>> StackTraceBenchmark.stackWalker thrpt 3 182902.031 ±
>> 39018.395 ops/s
>>
>>
>> -ck
>>
>> On Tue, Aug 7, 2018 at 7:20 PM, Ralph Goers <ra...@dslextreme.com>
>> wrote:
>>> I have to wonder why using the security manager is faster than using
>> StackWalker. StackWalker was created just for this purpose. Is the way it
>> is implemented the problem?
>>>
>>> Ralph
>>>
>>>
>>>> On Aug 7, 2018, at 1:34 PM, cakofony <gi...@git.apache.org> wrote:
>>>>
>>>> GitHub user cakofony opened a pull request:
>>>>
>>>> https://github.com/apache/logging-log4j2/pull/202
>>>>
>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>>
>>>> Share the SecurityManager implementation of getCurrentStackTrace
>>>> with the java 9 implementation.
>>>>
>>>> You can merge this pull request into a Git repository by running:
>>>>
>>>> $ git pull https://github.com/cakofony/logging-log4j2
>> java9_getcurrentstacktrace
>>>>
>>>> Alternatively you can review and apply these changes as the patch at:
>>>>
>>>> https://github.com/apache/logging-log4j2/pull/202.patch
>>>>
>>>> To close this pull request, make a commit to your master/trunk branch
>>>> with (at least) the following in the commit message:
>>>>
>>>> This closes #202
>>>>
>>>> ----
>>>> commit 52bf8569e8e9881d0999156c31d99b99f28e6e73
>>>> Author: Carter Kozak <ck...@...>
>>>> Date: 2018-08-07T20:27:10Z
>>>>
>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>>
>>>> Share the SecurityManager implementation of getCurrentStackTrace
>>>> with the java 9 implementation.
>>>>
>>>> ----
>>>>
>>>>
>>>> ---
>>>>
>>>
>>>
>>
>
>
> --
> Matt Sicker <bo...@gmail.com>
Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve
ThrowableProxy perfo...
Posted by Matt Sicker <bo...@gmail.com>.
StackWalker seems to be implemented purely as it says: walking the call
stack. If all you need is the stack of classes (e.g., for checking runtime
permissions based on the calling class), then the SecurityManager makes
most sense since it seems built for that exact purpose. I'd imagine there
are optimizations in place considering how often that class stack is
checked at runtime by just the JDK.
On Tue, 7 Aug 2018 at 22:10, Carter Kozak <c4...@gmail.com> wrote:
> My guess is that StackWalker provides additional information beyond
> the array of classes
> supplied by the SecurityManager, though I've not done a thorough analysis
> yet.
>
> A quick targeted benchmark of our current implementations:
>
> Benchmark Mode Cnt
> Score Error Units
> StackTraceBenchmark.defaultJava8 thrpt 3 113965.921 ±
> 119706.986 ops/s
> StackTraceBenchmark.securityManager thrpt 3 788004.237 ± 82578.567
> ops/s
> StackTraceBenchmark.stackWalker thrpt 3 182902.031 ±
> 39018.395 ops/s
>
>
> -ck
>
> On Tue, Aug 7, 2018 at 7:20 PM, Ralph Goers <ra...@dslextreme.com>
> wrote:
> > I have to wonder why using the security manager is faster than using
> StackWalker. StackWalker was created just for this purpose. Is the way it
> is implemented the problem?
> >
> > Ralph
> >
> >
> >> On Aug 7, 2018, at 1:34 PM, cakofony <gi...@git.apache.org> wrote:
> >>
> >> GitHub user cakofony opened a pull request:
> >>
> >> https://github.com/apache/logging-log4j2/pull/202
> >>
> >> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
> >>
> >> Share the SecurityManager implementation of getCurrentStackTrace
> >> with the java 9 implementation.
> >>
> >> You can merge this pull request into a Git repository by running:
> >>
> >> $ git pull https://github.com/cakofony/logging-log4j2
> java9_getcurrentstacktrace
> >>
> >> Alternatively you can review and apply these changes as the patch at:
> >>
> >> https://github.com/apache/logging-log4j2/pull/202.patch
> >>
> >> To close this pull request, make a commit to your master/trunk branch
> >> with (at least) the following in the commit message:
> >>
> >> This closes #202
> >>
> >> ----
> >> commit 52bf8569e8e9881d0999156c31d99b99f28e6e73
> >> Author: Carter Kozak <ck...@...>
> >> Date: 2018-08-07T20:27:10Z
> >>
> >> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
> >>
> >> Share the SecurityManager implementation of getCurrentStackTrace
> >> with the java 9 implementation.
> >>
> >> ----
> >>
> >>
> >> ---
> >>
> >
> >
>
--
Matt Sicker <bo...@gmail.com>
Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve
ThrowableProxy perfo...
Posted by Carter Kozak <c4...@gmail.com>.
Hi Ralph,
I've pushed my benchmark to a branch, it's not in a state that I'd be
comfortable merging to master.
I had to copy implementations into the benchmark because the java 9
version overshadows the default.
Commit is here:
http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/64bfd314
Best,
-ck
On Fri, Oct 5, 2018 at 4:54 PM Ralph Goers <ra...@dslextreme.com> wrote:
>
> Carter,
>
> Did you check in this benchmark? I posted these numbers on Jigsaw-dev and they would like to see the benchmark.
>
> Ralph
>
> > On Aug 7, 2018, at 8:10 PM, Carter Kozak <c4...@gmail.com> wrote:
> >
> > My guess is that StackWalker provides additional information beyond
> > the array of classes
> > supplied by the SecurityManager, though I've not done a thorough analysis yet.
> >
> > A quick targeted benchmark of our current implementations:
> >
> > Benchmark Mode Cnt
> > Score Error Units
> > StackTraceBenchmark.defaultJava8 thrpt 3 113965.921 ±
> > 119706.986 ops/s
> > StackTraceBenchmark.securityManager thrpt 3 788004.237 ± 82578.567 ops/s
> > StackTraceBenchmark.stackWalker thrpt 3 182902.031 ±
> > 39018.395 ops/s
> >
> >
> > -ck
> >
> > On Tue, Aug 7, 2018 at 7:20 PM, Ralph Goers <ra...@dslextreme.com> wrote:
> >> I have to wonder why using the security manager is faster than using StackWalker. StackWalker was created just for this purpose. Is the way it is implemented the problem?
> >>
> >> Ralph
> >>
> >>
> >>> On Aug 7, 2018, at 1:34 PM, cakofony <gi...@git.apache.org> wrote:
> >>>
> >>> GitHub user cakofony opened a pull request:
> >>>
> >>> https://github.com/apache/logging-log4j2/pull/202
> >>>
> >>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
> >>>
> >>> Share the SecurityManager implementation of getCurrentStackTrace
> >>> with the java 9 implementation.
> >>>
> >>> You can merge this pull request into a Git repository by running:
> >>>
> >>> $ git pull https://github.com/cakofony/logging-log4j2 java9_getcurrentstacktrace
> >>>
> >>> Alternatively you can review and apply these changes as the patch at:
> >>>
> >>> https://github.com/apache/logging-log4j2/pull/202.patch
> >>>
> >>> To close this pull request, make a commit to your master/trunk branch
> >>> with (at least) the following in the commit message:
> >>>
> >>> This closes #202
> >>>
> >>> ----
> >>> commit 52bf8569e8e9881d0999156c31d99b99f28e6e73
> >>> Author: Carter Kozak <ck...@...>
> >>> Date: 2018-08-07T20:27:10Z
> >>>
> >>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
> >>>
> >>> Share the SecurityManager implementation of getCurrentStackTrace
> >>> with the java 9 implementation.
> >>>
> >>> ----
> >>>
> >>>
> >>> ---
> >>>
> >>
> >>
> >
>
>
Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve
ThrowableProxy perfo...
Posted by Ralph Goers <ra...@dslextreme.com>.
Carter,
Did you check in this benchmark? I posted these numbers on Jigsaw-dev and they would like to see the benchmark.
Ralph
> On Aug 7, 2018, at 8:10 PM, Carter Kozak <c4...@gmail.com> wrote:
>
> My guess is that StackWalker provides additional information beyond
> the array of classes
> supplied by the SecurityManager, though I've not done a thorough analysis yet.
>
> A quick targeted benchmark of our current implementations:
>
> Benchmark Mode Cnt
> Score Error Units
> StackTraceBenchmark.defaultJava8 thrpt 3 113965.921 ±
> 119706.986 ops/s
> StackTraceBenchmark.securityManager thrpt 3 788004.237 ± 82578.567 ops/s
> StackTraceBenchmark.stackWalker thrpt 3 182902.031 ±
> 39018.395 ops/s
>
>
> -ck
>
> On Tue, Aug 7, 2018 at 7:20 PM, Ralph Goers <ra...@dslextreme.com> wrote:
>> I have to wonder why using the security manager is faster than using StackWalker. StackWalker was created just for this purpose. Is the way it is implemented the problem?
>>
>> Ralph
>>
>>
>>> On Aug 7, 2018, at 1:34 PM, cakofony <gi...@git.apache.org> wrote:
>>>
>>> GitHub user cakofony opened a pull request:
>>>
>>> https://github.com/apache/logging-log4j2/pull/202
>>>
>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>
>>> Share the SecurityManager implementation of getCurrentStackTrace
>>> with the java 9 implementation.
>>>
>>> You can merge this pull request into a Git repository by running:
>>>
>>> $ git pull https://github.com/cakofony/logging-log4j2 java9_getcurrentstacktrace
>>>
>>> Alternatively you can review and apply these changes as the patch at:
>>>
>>> https://github.com/apache/logging-log4j2/pull/202.patch
>>>
>>> To close this pull request, make a commit to your master/trunk branch
>>> with (at least) the following in the commit message:
>>>
>>> This closes #202
>>>
>>> ----
>>> commit 52bf8569e8e9881d0999156c31d99b99f28e6e73
>>> Author: Carter Kozak <ck...@...>
>>> Date: 2018-08-07T20:27:10Z
>>>
>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>
>>> Share the SecurityManager implementation of getCurrentStackTrace
>>> with the java 9 implementation.
>>>
>>> ----
>>>
>>>
>>> ---
>>>
>>
>>
>
Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve
ThrowableProxy perfo...
Posted by Carter Kozak <c4...@gmail.com>.
> I started looking into it but got sidetracked
No worries, I've had more time than usual this week to work on side
projects. Please don't feel obligated to respond quickly.
On Thu, Aug 9, 2018 at 2:15 PM Ralph Goers <ra...@dslextreme.com> wrote:
>
> Looking at your latest commit, your number looks similar to mine.
>
> Ralph
>
> > On Aug 9, 2018, at 11:14 AM, Ralph Goers <ra...@dslextreme.com> wrote:
> >
> > No, I haven’t committed it. I started looking into it but got sidetracked with having to earn a living.
> >
> > Ralph
> >
> >> On Aug 9, 2018, at 11:13 AM, Carter Kozak <ck...@apache.org> wrote:
> >>
> >> I've pushed the benchmark here:
> >> https://github.com/apache/logging-log4j2/pull/209
> >>
> >> Is the extended stack trace benchmark committed somewhere? I don't see
> >> it in log4j-perf.
> >> My benchmark isn't very pretty either, I'm open to ideas for cleaning
> >> up the implementation.
> >>
> >> -ck
> >>
> >> On Thu, Aug 9, 2018 at 1:56 PM, Ralph Goers <ra...@dslextreme.com> wrote:
> >>> I created a micro benchmark for the extended stack trace. It isn’t pretty.
> >>>
> >>> Ralph
> >>>
> >>>> On Aug 9, 2018, at 10:53 AM, Carter Kozak <ck...@apache.org> wrote:
> >>>>
> >>>> That's true, as long as we collect the stack trace when the
> >>>> ThrowableProxy is created, the rest can be deferred. I can take a look
> >>>> at this.
> >>>>
> >>>> I'm working on a benchmark with a larger stack to simulate conditions
> >>>> in a real application as well.
> >>>>
> >>>> -ck
> >>>>
> >>>> On Thu, Aug 9, 2018 at 12:47 PM, Ralph Goers <ra...@dslextreme.com> wrote:
> >>>>> What I don’t understand is that ThrowableProxy seems to be doing a whole bunch of stuff in the constructor. I would think a lot of that could be deferred until the Throwable is being rendered.
> >>>>>
> >>>>> Ralph
> >>>>>
> >>>>>> On Aug 9, 2018, at 8:10 AM, Carter Kozak <c4...@gmail.com> wrote:
> >>>>>>
> >>>>>> The piece I'd like to avoid is ThrowableProxy.loadClass invocations,
> >>>>>> which can be expensive depending on the ClassLoader implementation.
> >>>>>> The ThrowablePatternConverter implementations all fall back to the
> >>>>>> default implementation which operates directly on a Throwable rather
> >>>>>> than the proxy object.
> >>>>>>
> >>>>>> I would prefer to make ThrowableProxy work better rather than
> >>>>>> disabling it entirely, which would open the flood gates for issues
> >>>>>> with the Jackson and Serializable layouts, though the counterargument
> >>>>>> is that without loading classes, there is no additional information
> >>>>>> provided by ThrowableProxy over a Throwable.
> >>>>>>
> >>>>>> I will take a look at flagging ThrowableProxy class loading instead of
> >>>>>> disabling ThrowableProxy entirely when I have a moment.
> >>>>>>
> >>>>>> Thanks,
> >>>>>> -ck
> >>>>>>
> >>>>>> On Thu, Aug 9, 2018 at 1:36 AM, Ralph Goers <ra...@dslextreme.com> wrote:
> >>>>>>> What is the purpose of disabling ThrowableProxy? All the throwable pattern converters rely on it.
> >>>>>>>
> >>>>>>> Ralph
> >>>>>>>
> >>>>>>>> On Aug 8, 2018, at 8:34 AM, Carter Kozak <ck...@apache.org> wrote:
> >>>>>>>>
> >>>>>>>> I'm curious what you would think of a system property to disable
> >>>>>>>> ThrowableProxy creation?
> >>>>>>>> My initial preference was to avoid this type of flag and make the
> >>>>>>>> common case cleaner, however
> >>>>>>>> without providing a mechanism to disable the functionality that
> >>>>>>>> differentiates it from Throwable
> >>>>>>>> I'm not sure that's feasible.
> >>>>>>>>
> >>>>>>>> An alternative approach may be to allow custom implementations of the
> >>>>>>>> logic surrounding
> >>>>>>>> ThrowableProxy.toCacheEntry which could potentially disable classloader lookups,
> >>>>>>>> or implement something like the request in github pull/195. This extension point
> >>>>>>>> would make ThrowableProxy refactors significantly more difficult in the future.
> >>>>>>>>
> >>>>>>>> -ck
> >>>>>>>>
> >>>>>>>> On Tue, Aug 7, 2018 at 11:10 PM, Carter Kozak <c4...@gmail.com> wrote:
> >>>>>>>>> My guess is that StackWalker provides additional information beyond
> >>>>>>>>> the array of classes
> >>>>>>>>> supplied by the SecurityManager, though I've not done a thorough analysis yet.
> >>>>>>>>>
> >>>>>>>>> A quick targeted benchmark of our current implementations:
> >>>>>>>>>
> >>>>>>>>> Benchmark Mode Cnt
> >>>>>>>>> Score Error Units
> >>>>>>>>> StackTraceBenchmark.defaultJava8 thrpt 3 113965.921 ±
> >>>>>>>>> 119706.986 ops/s
> >>>>>>>>> StackTraceBenchmark.securityManager thrpt 3 788004.237 ± 82578.567 ops/s
> >>>>>>>>> StackTraceBenchmark.stackWalker thrpt 3 182902.031 ±
> >>>>>>>>> 39018.395 ops/s
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> -ck
> >>>>>>>>>
> >>>>>>>>> On Tue, Aug 7, 2018 at 7:20 PM, Ralph Goers <ra...@dslextreme.com> wrote:
> >>>>>>>>>> I have to wonder why using the security manager is faster than using StackWalker. StackWalker was created just for this purpose. Is the way it is implemented the problem?
> >>>>>>>>>>
> >>>>>>>>>> Ralph
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>> On Aug 7, 2018, at 1:34 PM, cakofony <gi...@git.apache.org> wrote:
> >>>>>>>>>>>
> >>>>>>>>>>> GitHub user cakofony opened a pull request:
> >>>>>>>>>>>
> >>>>>>>>>>> https://github.com/apache/logging-log4j2/pull/202
> >>>>>>>>>>>
> >>>>>>>>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
> >>>>>>>>>>>
> >>>>>>>>>>> Share the SecurityManager implementation of getCurrentStackTrace
> >>>>>>>>>>> with the java 9 implementation.
> >>>>>>>>>>>
> >>>>>>>>>>> You can merge this pull request into a Git repository by running:
> >>>>>>>>>>>
> >>>>>>>>>>> $ git pull https://github.com/cakofony/logging-log4j2 java9_getcurrentstacktrace
> >>>>>>>>>>>
> >>>>>>>>>>> Alternatively you can review and apply these changes as the patch at:
> >>>>>>>>>>>
> >>>>>>>>>>> https://github.com/apache/logging-log4j2/pull/202.patch
> >>>>>>>>>>>
> >>>>>>>>>>> To close this pull request, make a commit to your master/trunk branch
> >>>>>>>>>>> with (at least) the following in the commit message:
> >>>>>>>>>>>
> >>>>>>>>>>> This closes #202
> >>>>>>>>>>>
> >>>>>>>>>>> ----
> >>>>>>>>>>> commit 52bf8569e8e9881d0999156c31d99b99f28e6e73
> >>>>>>>>>>> Author: Carter Kozak <ck...@...>
> >>>>>>>>>>> Date: 2018-08-07T20:27:10Z
> >>>>>>>>>>>
> >>>>>>>>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
> >>>>>>>>>>>
> >>>>>>>>>>> Share the SecurityManager implementation of getCurrentStackTrace
> >>>>>>>>>>> with the java 9 implementation.
> >>>>>>>>>>>
> >>>>>>>>>>> ----
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> ---
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>
> >>>
> >>>
> >>
> >
> >
> >
>
>
Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve
ThrowableProxy perfo...
Posted by Ralph Goers <ra...@dslextreme.com>.
Looking at your latest commit, your number looks similar to mine.
Ralph
> On Aug 9, 2018, at 11:14 AM, Ralph Goers <ra...@dslextreme.com> wrote:
>
> No, I haven’t committed it. I started looking into it but got sidetracked with having to earn a living.
>
> Ralph
>
>> On Aug 9, 2018, at 11:13 AM, Carter Kozak <ck...@apache.org> wrote:
>>
>> I've pushed the benchmark here:
>> https://github.com/apache/logging-log4j2/pull/209
>>
>> Is the extended stack trace benchmark committed somewhere? I don't see
>> it in log4j-perf.
>> My benchmark isn't very pretty either, I'm open to ideas for cleaning
>> up the implementation.
>>
>> -ck
>>
>> On Thu, Aug 9, 2018 at 1:56 PM, Ralph Goers <ra...@dslextreme.com> wrote:
>>> I created a micro benchmark for the extended stack trace. It isn’t pretty.
>>>
>>> Ralph
>>>
>>>> On Aug 9, 2018, at 10:53 AM, Carter Kozak <ck...@apache.org> wrote:
>>>>
>>>> That's true, as long as we collect the stack trace when the
>>>> ThrowableProxy is created, the rest can be deferred. I can take a look
>>>> at this.
>>>>
>>>> I'm working on a benchmark with a larger stack to simulate conditions
>>>> in a real application as well.
>>>>
>>>> -ck
>>>>
>>>> On Thu, Aug 9, 2018 at 12:47 PM, Ralph Goers <ra...@dslextreme.com> wrote:
>>>>> What I don’t understand is that ThrowableProxy seems to be doing a whole bunch of stuff in the constructor. I would think a lot of that could be deferred until the Throwable is being rendered.
>>>>>
>>>>> Ralph
>>>>>
>>>>>> On Aug 9, 2018, at 8:10 AM, Carter Kozak <c4...@gmail.com> wrote:
>>>>>>
>>>>>> The piece I'd like to avoid is ThrowableProxy.loadClass invocations,
>>>>>> which can be expensive depending on the ClassLoader implementation.
>>>>>> The ThrowablePatternConverter implementations all fall back to the
>>>>>> default implementation which operates directly on a Throwable rather
>>>>>> than the proxy object.
>>>>>>
>>>>>> I would prefer to make ThrowableProxy work better rather than
>>>>>> disabling it entirely, which would open the flood gates for issues
>>>>>> with the Jackson and Serializable layouts, though the counterargument
>>>>>> is that without loading classes, there is no additional information
>>>>>> provided by ThrowableProxy over a Throwable.
>>>>>>
>>>>>> I will take a look at flagging ThrowableProxy class loading instead of
>>>>>> disabling ThrowableProxy entirely when I have a moment.
>>>>>>
>>>>>> Thanks,
>>>>>> -ck
>>>>>>
>>>>>> On Thu, Aug 9, 2018 at 1:36 AM, Ralph Goers <ra...@dslextreme.com> wrote:
>>>>>>> What is the purpose of disabling ThrowableProxy? All the throwable pattern converters rely on it.
>>>>>>>
>>>>>>> Ralph
>>>>>>>
>>>>>>>> On Aug 8, 2018, at 8:34 AM, Carter Kozak <ck...@apache.org> wrote:
>>>>>>>>
>>>>>>>> I'm curious what you would think of a system property to disable
>>>>>>>> ThrowableProxy creation?
>>>>>>>> My initial preference was to avoid this type of flag and make the
>>>>>>>> common case cleaner, however
>>>>>>>> without providing a mechanism to disable the functionality that
>>>>>>>> differentiates it from Throwable
>>>>>>>> I'm not sure that's feasible.
>>>>>>>>
>>>>>>>> An alternative approach may be to allow custom implementations of the
>>>>>>>> logic surrounding
>>>>>>>> ThrowableProxy.toCacheEntry which could potentially disable classloader lookups,
>>>>>>>> or implement something like the request in github pull/195. This extension point
>>>>>>>> would make ThrowableProxy refactors significantly more difficult in the future.
>>>>>>>>
>>>>>>>> -ck
>>>>>>>>
>>>>>>>> On Tue, Aug 7, 2018 at 11:10 PM, Carter Kozak <c4...@gmail.com> wrote:
>>>>>>>>> My guess is that StackWalker provides additional information beyond
>>>>>>>>> the array of classes
>>>>>>>>> supplied by the SecurityManager, though I've not done a thorough analysis yet.
>>>>>>>>>
>>>>>>>>> A quick targeted benchmark of our current implementations:
>>>>>>>>>
>>>>>>>>> Benchmark Mode Cnt
>>>>>>>>> Score Error Units
>>>>>>>>> StackTraceBenchmark.defaultJava8 thrpt 3 113965.921 ±
>>>>>>>>> 119706.986 ops/s
>>>>>>>>> StackTraceBenchmark.securityManager thrpt 3 788004.237 ± 82578.567 ops/s
>>>>>>>>> StackTraceBenchmark.stackWalker thrpt 3 182902.031 ±
>>>>>>>>> 39018.395 ops/s
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> -ck
>>>>>>>>>
>>>>>>>>> On Tue, Aug 7, 2018 at 7:20 PM, Ralph Goers <ra...@dslextreme.com> wrote:
>>>>>>>>>> I have to wonder why using the security manager is faster than using StackWalker. StackWalker was created just for this purpose. Is the way it is implemented the problem?
>>>>>>>>>>
>>>>>>>>>> Ralph
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> On Aug 7, 2018, at 1:34 PM, cakofony <gi...@git.apache.org> wrote:
>>>>>>>>>>>
>>>>>>>>>>> GitHub user cakofony opened a pull request:
>>>>>>>>>>>
>>>>>>>>>>> https://github.com/apache/logging-log4j2/pull/202
>>>>>>>>>>>
>>>>>>>>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>>>>>>>>>
>>>>>>>>>>> Share the SecurityManager implementation of getCurrentStackTrace
>>>>>>>>>>> with the java 9 implementation.
>>>>>>>>>>>
>>>>>>>>>>> You can merge this pull request into a Git repository by running:
>>>>>>>>>>>
>>>>>>>>>>> $ git pull https://github.com/cakofony/logging-log4j2 java9_getcurrentstacktrace
>>>>>>>>>>>
>>>>>>>>>>> Alternatively you can review and apply these changes as the patch at:
>>>>>>>>>>>
>>>>>>>>>>> https://github.com/apache/logging-log4j2/pull/202.patch
>>>>>>>>>>>
>>>>>>>>>>> To close this pull request, make a commit to your master/trunk branch
>>>>>>>>>>> with (at least) the following in the commit message:
>>>>>>>>>>>
>>>>>>>>>>> This closes #202
>>>>>>>>>>>
>>>>>>>>>>> ----
>>>>>>>>>>> commit 52bf8569e8e9881d0999156c31d99b99f28e6e73
>>>>>>>>>>> Author: Carter Kozak <ck...@...>
>>>>>>>>>>> Date: 2018-08-07T20:27:10Z
>>>>>>>>>>>
>>>>>>>>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>>>>>>>>>
>>>>>>>>>>> Share the SecurityManager implementation of getCurrentStackTrace
>>>>>>>>>>> with the java 9 implementation.
>>>>>>>>>>>
>>>>>>>>>>> ----
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> ---
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>
>
Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve
ThrowableProxy perfo...
Posted by Ralph Goers <ra...@dslextreme.com>.
No, I haven’t committed it. I started looking into it but got sidetracked with having to earn a living.
Ralph
> On Aug 9, 2018, at 11:13 AM, Carter Kozak <ck...@apache.org> wrote:
>
> I've pushed the benchmark here:
> https://github.com/apache/logging-log4j2/pull/209
>
> Is the extended stack trace benchmark committed somewhere? I don't see
> it in log4j-perf.
> My benchmark isn't very pretty either, I'm open to ideas for cleaning
> up the implementation.
>
> -ck
>
> On Thu, Aug 9, 2018 at 1:56 PM, Ralph Goers <ra...@dslextreme.com> wrote:
>> I created a micro benchmark for the extended stack trace. It isn’t pretty.
>>
>> Ralph
>>
>>> On Aug 9, 2018, at 10:53 AM, Carter Kozak <ck...@apache.org> wrote:
>>>
>>> That's true, as long as we collect the stack trace when the
>>> ThrowableProxy is created, the rest can be deferred. I can take a look
>>> at this.
>>>
>>> I'm working on a benchmark with a larger stack to simulate conditions
>>> in a real application as well.
>>>
>>> -ck
>>>
>>> On Thu, Aug 9, 2018 at 12:47 PM, Ralph Goers <ra...@dslextreme.com> wrote:
>>>> What I don’t understand is that ThrowableProxy seems to be doing a whole bunch of stuff in the constructor. I would think a lot of that could be deferred until the Throwable is being rendered.
>>>>
>>>> Ralph
>>>>
>>>>> On Aug 9, 2018, at 8:10 AM, Carter Kozak <c4...@gmail.com> wrote:
>>>>>
>>>>> The piece I'd like to avoid is ThrowableProxy.loadClass invocations,
>>>>> which can be expensive depending on the ClassLoader implementation.
>>>>> The ThrowablePatternConverter implementations all fall back to the
>>>>> default implementation which operates directly on a Throwable rather
>>>>> than the proxy object.
>>>>>
>>>>> I would prefer to make ThrowableProxy work better rather than
>>>>> disabling it entirely, which would open the flood gates for issues
>>>>> with the Jackson and Serializable layouts, though the counterargument
>>>>> is that without loading classes, there is no additional information
>>>>> provided by ThrowableProxy over a Throwable.
>>>>>
>>>>> I will take a look at flagging ThrowableProxy class loading instead of
>>>>> disabling ThrowableProxy entirely when I have a moment.
>>>>>
>>>>> Thanks,
>>>>> -ck
>>>>>
>>>>> On Thu, Aug 9, 2018 at 1:36 AM, Ralph Goers <ra...@dslextreme.com> wrote:
>>>>>> What is the purpose of disabling ThrowableProxy? All the throwable pattern converters rely on it.
>>>>>>
>>>>>> Ralph
>>>>>>
>>>>>>> On Aug 8, 2018, at 8:34 AM, Carter Kozak <ck...@apache.org> wrote:
>>>>>>>
>>>>>>> I'm curious what you would think of a system property to disable
>>>>>>> ThrowableProxy creation?
>>>>>>> My initial preference was to avoid this type of flag and make the
>>>>>>> common case cleaner, however
>>>>>>> without providing a mechanism to disable the functionality that
>>>>>>> differentiates it from Throwable
>>>>>>> I'm not sure that's feasible.
>>>>>>>
>>>>>>> An alternative approach may be to allow custom implementations of the
>>>>>>> logic surrounding
>>>>>>> ThrowableProxy.toCacheEntry which could potentially disable classloader lookups,
>>>>>>> or implement something like the request in github pull/195. This extension point
>>>>>>> would make ThrowableProxy refactors significantly more difficult in the future.
>>>>>>>
>>>>>>> -ck
>>>>>>>
>>>>>>> On Tue, Aug 7, 2018 at 11:10 PM, Carter Kozak <c4...@gmail.com> wrote:
>>>>>>>> My guess is that StackWalker provides additional information beyond
>>>>>>>> the array of classes
>>>>>>>> supplied by the SecurityManager, though I've not done a thorough analysis yet.
>>>>>>>>
>>>>>>>> A quick targeted benchmark of our current implementations:
>>>>>>>>
>>>>>>>> Benchmark Mode Cnt
>>>>>>>> Score Error Units
>>>>>>>> StackTraceBenchmark.defaultJava8 thrpt 3 113965.921 ±
>>>>>>>> 119706.986 ops/s
>>>>>>>> StackTraceBenchmark.securityManager thrpt 3 788004.237 ± 82578.567 ops/s
>>>>>>>> StackTraceBenchmark.stackWalker thrpt 3 182902.031 ±
>>>>>>>> 39018.395 ops/s
>>>>>>>>
>>>>>>>>
>>>>>>>> -ck
>>>>>>>>
>>>>>>>> On Tue, Aug 7, 2018 at 7:20 PM, Ralph Goers <ra...@dslextreme.com> wrote:
>>>>>>>>> I have to wonder why using the security manager is faster than using StackWalker. StackWalker was created just for this purpose. Is the way it is implemented the problem?
>>>>>>>>>
>>>>>>>>> Ralph
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> On Aug 7, 2018, at 1:34 PM, cakofony <gi...@git.apache.org> wrote:
>>>>>>>>>>
>>>>>>>>>> GitHub user cakofony opened a pull request:
>>>>>>>>>>
>>>>>>>>>> https://github.com/apache/logging-log4j2/pull/202
>>>>>>>>>>
>>>>>>>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>>>>>>>>
>>>>>>>>>> Share the SecurityManager implementation of getCurrentStackTrace
>>>>>>>>>> with the java 9 implementation.
>>>>>>>>>>
>>>>>>>>>> You can merge this pull request into a Git repository by running:
>>>>>>>>>>
>>>>>>>>>> $ git pull https://github.com/cakofony/logging-log4j2 java9_getcurrentstacktrace
>>>>>>>>>>
>>>>>>>>>> Alternatively you can review and apply these changes as the patch at:
>>>>>>>>>>
>>>>>>>>>> https://github.com/apache/logging-log4j2/pull/202.patch
>>>>>>>>>>
>>>>>>>>>> To close this pull request, make a commit to your master/trunk branch
>>>>>>>>>> with (at least) the following in the commit message:
>>>>>>>>>>
>>>>>>>>>> This closes #202
>>>>>>>>>>
>>>>>>>>>> ----
>>>>>>>>>> commit 52bf8569e8e9881d0999156c31d99b99f28e6e73
>>>>>>>>>> Author: Carter Kozak <ck...@...>
>>>>>>>>>> Date: 2018-08-07T20:27:10Z
>>>>>>>>>>
>>>>>>>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>>>>>>>>
>>>>>>>>>> Share the SecurityManager implementation of getCurrentStackTrace
>>>>>>>>>> with the java 9 implementation.
>>>>>>>>>>
>>>>>>>>>> ----
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> ---
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>
Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve
ThrowableProxy perfo...
Posted by Carter Kozak <ck...@apache.org>.
I've pushed the benchmark here:
https://github.com/apache/logging-log4j2/pull/209
Is the extended stack trace benchmark committed somewhere? I don't see
it in log4j-perf.
My benchmark isn't very pretty either, I'm open to ideas for cleaning
up the implementation.
-ck
On Thu, Aug 9, 2018 at 1:56 PM, Ralph Goers <ra...@dslextreme.com> wrote:
> I created a micro benchmark for the extended stack trace. It isn’t pretty.
>
> Ralph
>
>> On Aug 9, 2018, at 10:53 AM, Carter Kozak <ck...@apache.org> wrote:
>>
>> That's true, as long as we collect the stack trace when the
>> ThrowableProxy is created, the rest can be deferred. I can take a look
>> at this.
>>
>> I'm working on a benchmark with a larger stack to simulate conditions
>> in a real application as well.
>>
>> -ck
>>
>> On Thu, Aug 9, 2018 at 12:47 PM, Ralph Goers <ra...@dslextreme.com> wrote:
>>> What I don’t understand is that ThrowableProxy seems to be doing a whole bunch of stuff in the constructor. I would think a lot of that could be deferred until the Throwable is being rendered.
>>>
>>> Ralph
>>>
>>>> On Aug 9, 2018, at 8:10 AM, Carter Kozak <c4...@gmail.com> wrote:
>>>>
>>>> The piece I'd like to avoid is ThrowableProxy.loadClass invocations,
>>>> which can be expensive depending on the ClassLoader implementation.
>>>> The ThrowablePatternConverter implementations all fall back to the
>>>> default implementation which operates directly on a Throwable rather
>>>> than the proxy object.
>>>>
>>>> I would prefer to make ThrowableProxy work better rather than
>>>> disabling it entirely, which would open the flood gates for issues
>>>> with the Jackson and Serializable layouts, though the counterargument
>>>> is that without loading classes, there is no additional information
>>>> provided by ThrowableProxy over a Throwable.
>>>>
>>>> I will take a look at flagging ThrowableProxy class loading instead of
>>>> disabling ThrowableProxy entirely when I have a moment.
>>>>
>>>> Thanks,
>>>> -ck
>>>>
>>>> On Thu, Aug 9, 2018 at 1:36 AM, Ralph Goers <ra...@dslextreme.com> wrote:
>>>>> What is the purpose of disabling ThrowableProxy? All the throwable pattern converters rely on it.
>>>>>
>>>>> Ralph
>>>>>
>>>>>> On Aug 8, 2018, at 8:34 AM, Carter Kozak <ck...@apache.org> wrote:
>>>>>>
>>>>>> I'm curious what you would think of a system property to disable
>>>>>> ThrowableProxy creation?
>>>>>> My initial preference was to avoid this type of flag and make the
>>>>>> common case cleaner, however
>>>>>> without providing a mechanism to disable the functionality that
>>>>>> differentiates it from Throwable
>>>>>> I'm not sure that's feasible.
>>>>>>
>>>>>> An alternative approach may be to allow custom implementations of the
>>>>>> logic surrounding
>>>>>> ThrowableProxy.toCacheEntry which could potentially disable classloader lookups,
>>>>>> or implement something like the request in github pull/195. This extension point
>>>>>> would make ThrowableProxy refactors significantly more difficult in the future.
>>>>>>
>>>>>> -ck
>>>>>>
>>>>>> On Tue, Aug 7, 2018 at 11:10 PM, Carter Kozak <c4...@gmail.com> wrote:
>>>>>>> My guess is that StackWalker provides additional information beyond
>>>>>>> the array of classes
>>>>>>> supplied by the SecurityManager, though I've not done a thorough analysis yet.
>>>>>>>
>>>>>>> A quick targeted benchmark of our current implementations:
>>>>>>>
>>>>>>> Benchmark Mode Cnt
>>>>>>> Score Error Units
>>>>>>> StackTraceBenchmark.defaultJava8 thrpt 3 113965.921 ±
>>>>>>> 119706.986 ops/s
>>>>>>> StackTraceBenchmark.securityManager thrpt 3 788004.237 ± 82578.567 ops/s
>>>>>>> StackTraceBenchmark.stackWalker thrpt 3 182902.031 ±
>>>>>>> 39018.395 ops/s
>>>>>>>
>>>>>>>
>>>>>>> -ck
>>>>>>>
>>>>>>> On Tue, Aug 7, 2018 at 7:20 PM, Ralph Goers <ra...@dslextreme.com> wrote:
>>>>>>>> I have to wonder why using the security manager is faster than using StackWalker. StackWalker was created just for this purpose. Is the way it is implemented the problem?
>>>>>>>>
>>>>>>>> Ralph
>>>>>>>>
>>>>>>>>
>>>>>>>>> On Aug 7, 2018, at 1:34 PM, cakofony <gi...@git.apache.org> wrote:
>>>>>>>>>
>>>>>>>>> GitHub user cakofony opened a pull request:
>>>>>>>>>
>>>>>>>>> https://github.com/apache/logging-log4j2/pull/202
>>>>>>>>>
>>>>>>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>>>>>>>
>>>>>>>>> Share the SecurityManager implementation of getCurrentStackTrace
>>>>>>>>> with the java 9 implementation.
>>>>>>>>>
>>>>>>>>> You can merge this pull request into a Git repository by running:
>>>>>>>>>
>>>>>>>>> $ git pull https://github.com/cakofony/logging-log4j2 java9_getcurrentstacktrace
>>>>>>>>>
>>>>>>>>> Alternatively you can review and apply these changes as the patch at:
>>>>>>>>>
>>>>>>>>> https://github.com/apache/logging-log4j2/pull/202.patch
>>>>>>>>>
>>>>>>>>> To close this pull request, make a commit to your master/trunk branch
>>>>>>>>> with (at least) the following in the commit message:
>>>>>>>>>
>>>>>>>>> This closes #202
>>>>>>>>>
>>>>>>>>> ----
>>>>>>>>> commit 52bf8569e8e9881d0999156c31d99b99f28e6e73
>>>>>>>>> Author: Carter Kozak <ck...@...>
>>>>>>>>> Date: 2018-08-07T20:27:10Z
>>>>>>>>>
>>>>>>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>>>>>>>
>>>>>>>>> Share the SecurityManager implementation of getCurrentStackTrace
>>>>>>>>> with the java 9 implementation.
>>>>>>>>>
>>>>>>>>> ----
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>
Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve
ThrowableProxy perfo...
Posted by Ralph Goers <ra...@dslextreme.com>.
I created a micro benchmark for the extended stack trace. It isn’t pretty.
Ralph
> On Aug 9, 2018, at 10:53 AM, Carter Kozak <ck...@apache.org> wrote:
>
> That's true, as long as we collect the stack trace when the
> ThrowableProxy is created, the rest can be deferred. I can take a look
> at this.
>
> I'm working on a benchmark with a larger stack to simulate conditions
> in a real application as well.
>
> -ck
>
> On Thu, Aug 9, 2018 at 12:47 PM, Ralph Goers <ra...@dslextreme.com> wrote:
>> What I don’t understand is that ThrowableProxy seems to be doing a whole bunch of stuff in the constructor. I would think a lot of that could be deferred until the Throwable is being rendered.
>>
>> Ralph
>>
>>> On Aug 9, 2018, at 8:10 AM, Carter Kozak <c4...@gmail.com> wrote:
>>>
>>> The piece I'd like to avoid is ThrowableProxy.loadClass invocations,
>>> which can be expensive depending on the ClassLoader implementation.
>>> The ThrowablePatternConverter implementations all fall back to the
>>> default implementation which operates directly on a Throwable rather
>>> than the proxy object.
>>>
>>> I would prefer to make ThrowableProxy work better rather than
>>> disabling it entirely, which would open the flood gates for issues
>>> with the Jackson and Serializable layouts, though the counterargument
>>> is that without loading classes, there is no additional information
>>> provided by ThrowableProxy over a Throwable.
>>>
>>> I will take a look at flagging ThrowableProxy class loading instead of
>>> disabling ThrowableProxy entirely when I have a moment.
>>>
>>> Thanks,
>>> -ck
>>>
>>> On Thu, Aug 9, 2018 at 1:36 AM, Ralph Goers <ra...@dslextreme.com> wrote:
>>>> What is the purpose of disabling ThrowableProxy? All the throwable pattern converters rely on it.
>>>>
>>>> Ralph
>>>>
>>>>> On Aug 8, 2018, at 8:34 AM, Carter Kozak <ck...@apache.org> wrote:
>>>>>
>>>>> I'm curious what you would think of a system property to disable
>>>>> ThrowableProxy creation?
>>>>> My initial preference was to avoid this type of flag and make the
>>>>> common case cleaner, however
>>>>> without providing a mechanism to disable the functionality that
>>>>> differentiates it from Throwable
>>>>> I'm not sure that's feasible.
>>>>>
>>>>> An alternative approach may be to allow custom implementations of the
>>>>> logic surrounding
>>>>> ThrowableProxy.toCacheEntry which could potentially disable classloader lookups,
>>>>> or implement something like the request in github pull/195. This extension point
>>>>> would make ThrowableProxy refactors significantly more difficult in the future.
>>>>>
>>>>> -ck
>>>>>
>>>>> On Tue, Aug 7, 2018 at 11:10 PM, Carter Kozak <c4...@gmail.com> wrote:
>>>>>> My guess is that StackWalker provides additional information beyond
>>>>>> the array of classes
>>>>>> supplied by the SecurityManager, though I've not done a thorough analysis yet.
>>>>>>
>>>>>> A quick targeted benchmark of our current implementations:
>>>>>>
>>>>>> Benchmark Mode Cnt
>>>>>> Score Error Units
>>>>>> StackTraceBenchmark.defaultJava8 thrpt 3 113965.921 ±
>>>>>> 119706.986 ops/s
>>>>>> StackTraceBenchmark.securityManager thrpt 3 788004.237 ± 82578.567 ops/s
>>>>>> StackTraceBenchmark.stackWalker thrpt 3 182902.031 ±
>>>>>> 39018.395 ops/s
>>>>>>
>>>>>>
>>>>>> -ck
>>>>>>
>>>>>> On Tue, Aug 7, 2018 at 7:20 PM, Ralph Goers <ra...@dslextreme.com> wrote:
>>>>>>> I have to wonder why using the security manager is faster than using StackWalker. StackWalker was created just for this purpose. Is the way it is implemented the problem?
>>>>>>>
>>>>>>> Ralph
>>>>>>>
>>>>>>>
>>>>>>>> On Aug 7, 2018, at 1:34 PM, cakofony <gi...@git.apache.org> wrote:
>>>>>>>>
>>>>>>>> GitHub user cakofony opened a pull request:
>>>>>>>>
>>>>>>>> https://github.com/apache/logging-log4j2/pull/202
>>>>>>>>
>>>>>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>>>>>>
>>>>>>>> Share the SecurityManager implementation of getCurrentStackTrace
>>>>>>>> with the java 9 implementation.
>>>>>>>>
>>>>>>>> You can merge this pull request into a Git repository by running:
>>>>>>>>
>>>>>>>> $ git pull https://github.com/cakofony/logging-log4j2 java9_getcurrentstacktrace
>>>>>>>>
>>>>>>>> Alternatively you can review and apply these changes as the patch at:
>>>>>>>>
>>>>>>>> https://github.com/apache/logging-log4j2/pull/202.patch
>>>>>>>>
>>>>>>>> To close this pull request, make a commit to your master/trunk branch
>>>>>>>> with (at least) the following in the commit message:
>>>>>>>>
>>>>>>>> This closes #202
>>>>>>>>
>>>>>>>> ----
>>>>>>>> commit 52bf8569e8e9881d0999156c31d99b99f28e6e73
>>>>>>>> Author: Carter Kozak <ck...@...>
>>>>>>>> Date: 2018-08-07T20:27:10Z
>>>>>>>>
>>>>>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>>>>>>
>>>>>>>> Share the SecurityManager implementation of getCurrentStackTrace
>>>>>>>> with the java 9 implementation.
>>>>>>>>
>>>>>>>> ----
>>>>>>>>
>>>>>>>>
>>>>>>>> ---
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>
Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve
ThrowableProxy perfo...
Posted by Carter Kozak <ck...@apache.org>.
That's true, as long as we collect the stack trace when the
ThrowableProxy is created, the rest can be deferred. I can take a look
at this.
I'm working on a benchmark with a larger stack to simulate conditions
in a real application as well.
-ck
On Thu, Aug 9, 2018 at 12:47 PM, Ralph Goers <ra...@dslextreme.com> wrote:
> What I don’t understand is that ThrowableProxy seems to be doing a whole bunch of stuff in the constructor. I would think a lot of that could be deferred until the Throwable is being rendered.
>
> Ralph
>
>> On Aug 9, 2018, at 8:10 AM, Carter Kozak <c4...@gmail.com> wrote:
>>
>> The piece I'd like to avoid is ThrowableProxy.loadClass invocations,
>> which can be expensive depending on the ClassLoader implementation.
>> The ThrowablePatternConverter implementations all fall back to the
>> default implementation which operates directly on a Throwable rather
>> than the proxy object.
>>
>> I would prefer to make ThrowableProxy work better rather than
>> disabling it entirely, which would open the flood gates for issues
>> with the Jackson and Serializable layouts, though the counterargument
>> is that without loading classes, there is no additional information
>> provided by ThrowableProxy over a Throwable.
>>
>> I will take a look at flagging ThrowableProxy class loading instead of
>> disabling ThrowableProxy entirely when I have a moment.
>>
>> Thanks,
>> -ck
>>
>> On Thu, Aug 9, 2018 at 1:36 AM, Ralph Goers <ra...@dslextreme.com> wrote:
>>> What is the purpose of disabling ThrowableProxy? All the throwable pattern converters rely on it.
>>>
>>> Ralph
>>>
>>>> On Aug 8, 2018, at 8:34 AM, Carter Kozak <ck...@apache.org> wrote:
>>>>
>>>> I'm curious what you would think of a system property to disable
>>>> ThrowableProxy creation?
>>>> My initial preference was to avoid this type of flag and make the
>>>> common case cleaner, however
>>>> without providing a mechanism to disable the functionality that
>>>> differentiates it from Throwable
>>>> I'm not sure that's feasible.
>>>>
>>>> An alternative approach may be to allow custom implementations of the
>>>> logic surrounding
>>>> ThrowableProxy.toCacheEntry which could potentially disable classloader lookups,
>>>> or implement something like the request in github pull/195. This extension point
>>>> would make ThrowableProxy refactors significantly more difficult in the future.
>>>>
>>>> -ck
>>>>
>>>> On Tue, Aug 7, 2018 at 11:10 PM, Carter Kozak <c4...@gmail.com> wrote:
>>>>> My guess is that StackWalker provides additional information beyond
>>>>> the array of classes
>>>>> supplied by the SecurityManager, though I've not done a thorough analysis yet.
>>>>>
>>>>> A quick targeted benchmark of our current implementations:
>>>>>
>>>>> Benchmark Mode Cnt
>>>>> Score Error Units
>>>>> StackTraceBenchmark.defaultJava8 thrpt 3 113965.921 ±
>>>>> 119706.986 ops/s
>>>>> StackTraceBenchmark.securityManager thrpt 3 788004.237 ± 82578.567 ops/s
>>>>> StackTraceBenchmark.stackWalker thrpt 3 182902.031 ±
>>>>> 39018.395 ops/s
>>>>>
>>>>>
>>>>> -ck
>>>>>
>>>>> On Tue, Aug 7, 2018 at 7:20 PM, Ralph Goers <ra...@dslextreme.com> wrote:
>>>>>> I have to wonder why using the security manager is faster than using StackWalker. StackWalker was created just for this purpose. Is the way it is implemented the problem?
>>>>>>
>>>>>> Ralph
>>>>>>
>>>>>>
>>>>>>> On Aug 7, 2018, at 1:34 PM, cakofony <gi...@git.apache.org> wrote:
>>>>>>>
>>>>>>> GitHub user cakofony opened a pull request:
>>>>>>>
>>>>>>> https://github.com/apache/logging-log4j2/pull/202
>>>>>>>
>>>>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>>>>>
>>>>>>> Share the SecurityManager implementation of getCurrentStackTrace
>>>>>>> with the java 9 implementation.
>>>>>>>
>>>>>>> You can merge this pull request into a Git repository by running:
>>>>>>>
>>>>>>> $ git pull https://github.com/cakofony/logging-log4j2 java9_getcurrentstacktrace
>>>>>>>
>>>>>>> Alternatively you can review and apply these changes as the patch at:
>>>>>>>
>>>>>>> https://github.com/apache/logging-log4j2/pull/202.patch
>>>>>>>
>>>>>>> To close this pull request, make a commit to your master/trunk branch
>>>>>>> with (at least) the following in the commit message:
>>>>>>>
>>>>>>> This closes #202
>>>>>>>
>>>>>>> ----
>>>>>>> commit 52bf8569e8e9881d0999156c31d99b99f28e6e73
>>>>>>> Author: Carter Kozak <ck...@...>
>>>>>>> Date: 2018-08-07T20:27:10Z
>>>>>>>
>>>>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>>>>>
>>>>>>> Share the SecurityManager implementation of getCurrentStackTrace
>>>>>>> with the java 9 implementation.
>>>>>>>
>>>>>>> ----
>>>>>>>
>>>>>>>
>>>>>>> ---
>>>>>>>
>>>>>>
>>>>>>
>>>>
>>>
>>>
>>
>
>
Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve
ThrowableProxy perfo...
Posted by Ralph Goers <ra...@dslextreme.com>.
What I don’t understand is that ThrowableProxy seems to be doing a whole bunch of stuff in the constructor. I would think a lot of that could be deferred until the Throwable is being rendered.
Ralph
> On Aug 9, 2018, at 8:10 AM, Carter Kozak <c4...@gmail.com> wrote:
>
> The piece I'd like to avoid is ThrowableProxy.loadClass invocations,
> which can be expensive depending on the ClassLoader implementation.
> The ThrowablePatternConverter implementations all fall back to the
> default implementation which operates directly on a Throwable rather
> than the proxy object.
>
> I would prefer to make ThrowableProxy work better rather than
> disabling it entirely, which would open the flood gates for issues
> with the Jackson and Serializable layouts, though the counterargument
> is that without loading classes, there is no additional information
> provided by ThrowableProxy over a Throwable.
>
> I will take a look at flagging ThrowableProxy class loading instead of
> disabling ThrowableProxy entirely when I have a moment.
>
> Thanks,
> -ck
>
> On Thu, Aug 9, 2018 at 1:36 AM, Ralph Goers <ra...@dslextreme.com> wrote:
>> What is the purpose of disabling ThrowableProxy? All the throwable pattern converters rely on it.
>>
>> Ralph
>>
>>> On Aug 8, 2018, at 8:34 AM, Carter Kozak <ck...@apache.org> wrote:
>>>
>>> I'm curious what you would think of a system property to disable
>>> ThrowableProxy creation?
>>> My initial preference was to avoid this type of flag and make the
>>> common case cleaner, however
>>> without providing a mechanism to disable the functionality that
>>> differentiates it from Throwable
>>> I'm not sure that's feasible.
>>>
>>> An alternative approach may be to allow custom implementations of the
>>> logic surrounding
>>> ThrowableProxy.toCacheEntry which could potentially disable classloader lookups,
>>> or implement something like the request in github pull/195. This extension point
>>> would make ThrowableProxy refactors significantly more difficult in the future.
>>>
>>> -ck
>>>
>>> On Tue, Aug 7, 2018 at 11:10 PM, Carter Kozak <c4...@gmail.com> wrote:
>>>> My guess is that StackWalker provides additional information beyond
>>>> the array of classes
>>>> supplied by the SecurityManager, though I've not done a thorough analysis yet.
>>>>
>>>> A quick targeted benchmark of our current implementations:
>>>>
>>>> Benchmark Mode Cnt
>>>> Score Error Units
>>>> StackTraceBenchmark.defaultJava8 thrpt 3 113965.921 ±
>>>> 119706.986 ops/s
>>>> StackTraceBenchmark.securityManager thrpt 3 788004.237 ± 82578.567 ops/s
>>>> StackTraceBenchmark.stackWalker thrpt 3 182902.031 ±
>>>> 39018.395 ops/s
>>>>
>>>>
>>>> -ck
>>>>
>>>> On Tue, Aug 7, 2018 at 7:20 PM, Ralph Goers <ra...@dslextreme.com> wrote:
>>>>> I have to wonder why using the security manager is faster than using StackWalker. StackWalker was created just for this purpose. Is the way it is implemented the problem?
>>>>>
>>>>> Ralph
>>>>>
>>>>>
>>>>>> On Aug 7, 2018, at 1:34 PM, cakofony <gi...@git.apache.org> wrote:
>>>>>>
>>>>>> GitHub user cakofony opened a pull request:
>>>>>>
>>>>>> https://github.com/apache/logging-log4j2/pull/202
>>>>>>
>>>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>>>>
>>>>>> Share the SecurityManager implementation of getCurrentStackTrace
>>>>>> with the java 9 implementation.
>>>>>>
>>>>>> You can merge this pull request into a Git repository by running:
>>>>>>
>>>>>> $ git pull https://github.com/cakofony/logging-log4j2 java9_getcurrentstacktrace
>>>>>>
>>>>>> Alternatively you can review and apply these changes as the patch at:
>>>>>>
>>>>>> https://github.com/apache/logging-log4j2/pull/202.patch
>>>>>>
>>>>>> To close this pull request, make a commit to your master/trunk branch
>>>>>> with (at least) the following in the commit message:
>>>>>>
>>>>>> This closes #202
>>>>>>
>>>>>> ----
>>>>>> commit 52bf8569e8e9881d0999156c31d99b99f28e6e73
>>>>>> Author: Carter Kozak <ck...@...>
>>>>>> Date: 2018-08-07T20:27:10Z
>>>>>>
>>>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>>>>
>>>>>> Share the SecurityManager implementation of getCurrentStackTrace
>>>>>> with the java 9 implementation.
>>>>>>
>>>>>> ----
>>>>>>
>>>>>>
>>>>>> ---
>>>>>>
>>>>>
>>>>>
>>>
>>
>>
>
Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve
ThrowableProxy perfo...
Posted by Carter Kozak <c4...@gmail.com>.
The piece I'd like to avoid is ThrowableProxy.loadClass invocations,
which can be expensive depending on the ClassLoader implementation.
The ThrowablePatternConverter implementations all fall back to the
default implementation which operates directly on a Throwable rather
than the proxy object.
I would prefer to make ThrowableProxy work better rather than
disabling it entirely, which would open the flood gates for issues
with the Jackson and Serializable layouts, though the counterargument
is that without loading classes, there is no additional information
provided by ThrowableProxy over a Throwable.
I will take a look at flagging ThrowableProxy class loading instead of
disabling ThrowableProxy entirely when I have a moment.
Thanks,
-ck
On Thu, Aug 9, 2018 at 1:36 AM, Ralph Goers <ra...@dslextreme.com> wrote:
> What is the purpose of disabling ThrowableProxy? All the throwable pattern converters rely on it.
>
> Ralph
>
>> On Aug 8, 2018, at 8:34 AM, Carter Kozak <ck...@apache.org> wrote:
>>
>> I'm curious what you would think of a system property to disable
>> ThrowableProxy creation?
>> My initial preference was to avoid this type of flag and make the
>> common case cleaner, however
>> without providing a mechanism to disable the functionality that
>> differentiates it from Throwable
>> I'm not sure that's feasible.
>>
>> An alternative approach may be to allow custom implementations of the
>> logic surrounding
>> ThrowableProxy.toCacheEntry which could potentially disable classloader lookups,
>> or implement something like the request in github pull/195. This extension point
>> would make ThrowableProxy refactors significantly more difficult in the future.
>>
>> -ck
>>
>> On Tue, Aug 7, 2018 at 11:10 PM, Carter Kozak <c4...@gmail.com> wrote:
>>> My guess is that StackWalker provides additional information beyond
>>> the array of classes
>>> supplied by the SecurityManager, though I've not done a thorough analysis yet.
>>>
>>> A quick targeted benchmark of our current implementations:
>>>
>>> Benchmark Mode Cnt
>>> Score Error Units
>>> StackTraceBenchmark.defaultJava8 thrpt 3 113965.921 ±
>>> 119706.986 ops/s
>>> StackTraceBenchmark.securityManager thrpt 3 788004.237 ± 82578.567 ops/s
>>> StackTraceBenchmark.stackWalker thrpt 3 182902.031 ±
>>> 39018.395 ops/s
>>>
>>>
>>> -ck
>>>
>>> On Tue, Aug 7, 2018 at 7:20 PM, Ralph Goers <ra...@dslextreme.com> wrote:
>>>> I have to wonder why using the security manager is faster than using StackWalker. StackWalker was created just for this purpose. Is the way it is implemented the problem?
>>>>
>>>> Ralph
>>>>
>>>>
>>>>> On Aug 7, 2018, at 1:34 PM, cakofony <gi...@git.apache.org> wrote:
>>>>>
>>>>> GitHub user cakofony opened a pull request:
>>>>>
>>>>> https://github.com/apache/logging-log4j2/pull/202
>>>>>
>>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>>>
>>>>> Share the SecurityManager implementation of getCurrentStackTrace
>>>>> with the java 9 implementation.
>>>>>
>>>>> You can merge this pull request into a Git repository by running:
>>>>>
>>>>> $ git pull https://github.com/cakofony/logging-log4j2 java9_getcurrentstacktrace
>>>>>
>>>>> Alternatively you can review and apply these changes as the patch at:
>>>>>
>>>>> https://github.com/apache/logging-log4j2/pull/202.patch
>>>>>
>>>>> To close this pull request, make a commit to your master/trunk branch
>>>>> with (at least) the following in the commit message:
>>>>>
>>>>> This closes #202
>>>>>
>>>>> ----
>>>>> commit 52bf8569e8e9881d0999156c31d99b99f28e6e73
>>>>> Author: Carter Kozak <ck...@...>
>>>>> Date: 2018-08-07T20:27:10Z
>>>>>
>>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>>>
>>>>> Share the SecurityManager implementation of getCurrentStackTrace
>>>>> with the java 9 implementation.
>>>>>
>>>>> ----
>>>>>
>>>>>
>>>>> ---
>>>>>
>>>>
>>>>
>>
>
>
Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve
ThrowableProxy perfo...
Posted by Ralph Goers <ra...@dslextreme.com>.
What is the purpose of disabling ThrowableProxy? All the throwable pattern converters rely on it.
Ralph
> On Aug 8, 2018, at 8:34 AM, Carter Kozak <ck...@apache.org> wrote:
>
> I'm curious what you would think of a system property to disable
> ThrowableProxy creation?
> My initial preference was to avoid this type of flag and make the
> common case cleaner, however
> without providing a mechanism to disable the functionality that
> differentiates it from Throwable
> I'm not sure that's feasible.
>
> An alternative approach may be to allow custom implementations of the
> logic surrounding
> ThrowableProxy.toCacheEntry which could potentially disable classloader lookups,
> or implement something like the request in github pull/195. This extension point
> would make ThrowableProxy refactors significantly more difficult in the future.
>
> -ck
>
> On Tue, Aug 7, 2018 at 11:10 PM, Carter Kozak <c4...@gmail.com> wrote:
>> My guess is that StackWalker provides additional information beyond
>> the array of classes
>> supplied by the SecurityManager, though I've not done a thorough analysis yet.
>>
>> A quick targeted benchmark of our current implementations:
>>
>> Benchmark Mode Cnt
>> Score Error Units
>> StackTraceBenchmark.defaultJava8 thrpt 3 113965.921 ±
>> 119706.986 ops/s
>> StackTraceBenchmark.securityManager thrpt 3 788004.237 ± 82578.567 ops/s
>> StackTraceBenchmark.stackWalker thrpt 3 182902.031 ±
>> 39018.395 ops/s
>>
>>
>> -ck
>>
>> On Tue, Aug 7, 2018 at 7:20 PM, Ralph Goers <ra...@dslextreme.com> wrote:
>>> I have to wonder why using the security manager is faster than using StackWalker. StackWalker was created just for this purpose. Is the way it is implemented the problem?
>>>
>>> Ralph
>>>
>>>
>>>> On Aug 7, 2018, at 1:34 PM, cakofony <gi...@git.apache.org> wrote:
>>>>
>>>> GitHub user cakofony opened a pull request:
>>>>
>>>> https://github.com/apache/logging-log4j2/pull/202
>>>>
>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>>
>>>> Share the SecurityManager implementation of getCurrentStackTrace
>>>> with the java 9 implementation.
>>>>
>>>> You can merge this pull request into a Git repository by running:
>>>>
>>>> $ git pull https://github.com/cakofony/logging-log4j2 java9_getcurrentstacktrace
>>>>
>>>> Alternatively you can review and apply these changes as the patch at:
>>>>
>>>> https://github.com/apache/logging-log4j2/pull/202.patch
>>>>
>>>> To close this pull request, make a commit to your master/trunk branch
>>>> with (at least) the following in the commit message:
>>>>
>>>> This closes #202
>>>>
>>>> ----
>>>> commit 52bf8569e8e9881d0999156c31d99b99f28e6e73
>>>> Author: Carter Kozak <ck...@...>
>>>> Date: 2018-08-07T20:27:10Z
>>>>
>>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>>
>>>> Share the SecurityManager implementation of getCurrentStackTrace
>>>> with the java 9 implementation.
>>>>
>>>> ----
>>>>
>>>>
>>>> ---
>>>>
>>>
>>>
>
Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve
ThrowableProxy perfo...
Posted by Carter Kozak <ck...@apache.org>.
I'm curious what you would think of a system property to disable
ThrowableProxy creation?
My initial preference was to avoid this type of flag and make the
common case cleaner, however
without providing a mechanism to disable the functionality that
differentiates it from Throwable
I'm not sure that's feasible.
An alternative approach may be to allow custom implementations of the
logic surrounding
ThrowableProxy.toCacheEntry which could potentially disable classloader lookups,
or implement something like the request in github pull/195. This extension point
would make ThrowableProxy refactors significantly more difficult in the future.
-ck
On Tue, Aug 7, 2018 at 11:10 PM, Carter Kozak <c4...@gmail.com> wrote:
> My guess is that StackWalker provides additional information beyond
> the array of classes
> supplied by the SecurityManager, though I've not done a thorough analysis yet.
>
> A quick targeted benchmark of our current implementations:
>
> Benchmark Mode Cnt
> Score Error Units
> StackTraceBenchmark.defaultJava8 thrpt 3 113965.921 ±
> 119706.986 ops/s
> StackTraceBenchmark.securityManager thrpt 3 788004.237 ± 82578.567 ops/s
> StackTraceBenchmark.stackWalker thrpt 3 182902.031 ±
> 39018.395 ops/s
>
>
> -ck
>
> On Tue, Aug 7, 2018 at 7:20 PM, Ralph Goers <ra...@dslextreme.com> wrote:
>> I have to wonder why using the security manager is faster than using StackWalker. StackWalker was created just for this purpose. Is the way it is implemented the problem?
>>
>> Ralph
>>
>>
>>> On Aug 7, 2018, at 1:34 PM, cakofony <gi...@git.apache.org> wrote:
>>>
>>> GitHub user cakofony opened a pull request:
>>>
>>> https://github.com/apache/logging-log4j2/pull/202
>>>
>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>
>>> Share the SecurityManager implementation of getCurrentStackTrace
>>> with the java 9 implementation.
>>>
>>> You can merge this pull request into a Git repository by running:
>>>
>>> $ git pull https://github.com/cakofony/logging-log4j2 java9_getcurrentstacktrace
>>>
>>> Alternatively you can review and apply these changes as the patch at:
>>>
>>> https://github.com/apache/logging-log4j2/pull/202.patch
>>>
>>> To close this pull request, make a commit to your master/trunk branch
>>> with (at least) the following in the commit message:
>>>
>>> This closes #202
>>>
>>> ----
>>> commit 52bf8569e8e9881d0999156c31d99b99f28e6e73
>>> Author: Carter Kozak <ck...@...>
>>> Date: 2018-08-07T20:27:10Z
>>>
>>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>>
>>> Share the SecurityManager implementation of getCurrentStackTrace
>>> with the java 9 implementation.
>>>
>>> ----
>>>
>>>
>>> ---
>>>
>>
>>
Re: [GitHub] logging-log4j2 pull request #202: [LOG4J2-2391] Improve
ThrowableProxy perfo...
Posted by Carter Kozak <c4...@gmail.com>.
My guess is that StackWalker provides additional information beyond
the array of classes
supplied by the SecurityManager, though I've not done a thorough analysis yet.
A quick targeted benchmark of our current implementations:
Benchmark Mode Cnt
Score Error Units
StackTraceBenchmark.defaultJava8 thrpt 3 113965.921 ±
119706.986 ops/s
StackTraceBenchmark.securityManager thrpt 3 788004.237 ± 82578.567 ops/s
StackTraceBenchmark.stackWalker thrpt 3 182902.031 ±
39018.395 ops/s
-ck
On Tue, Aug 7, 2018 at 7:20 PM, Ralph Goers <ra...@dslextreme.com> wrote:
> I have to wonder why using the security manager is faster than using StackWalker. StackWalker was created just for this purpose. Is the way it is implemented the problem?
>
> Ralph
>
>
>> On Aug 7, 2018, at 1:34 PM, cakofony <gi...@git.apache.org> wrote:
>>
>> GitHub user cakofony opened a pull request:
>>
>> https://github.com/apache/logging-log4j2/pull/202
>>
>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>
>> Share the SecurityManager implementation of getCurrentStackTrace
>> with the java 9 implementation.
>>
>> You can merge this pull request into a Git repository by running:
>>
>> $ git pull https://github.com/cakofony/logging-log4j2 java9_getcurrentstacktrace
>>
>> Alternatively you can review and apply these changes as the patch at:
>>
>> https://github.com/apache/logging-log4j2/pull/202.patch
>>
>> To close this pull request, make a commit to your master/trunk branch
>> with (at least) the following in the commit message:
>>
>> This closes #202
>>
>> ----
>> commit 52bf8569e8e9881d0999156c31d99b99f28e6e73
>> Author: Carter Kozak <ck...@...>
>> Date: 2018-08-07T20:27:10Z
>>
>> [LOG4J2-2391] Improve ThrowableProxy performace on java 9+
>>
>> Share the SecurityManager implementation of getCurrentStackTrace
>> with the java 9 implementation.
>>
>> ----
>>
>>
>> ---
>>
>
>