> 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 <ralph.go...@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 <ralph.go...@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 <cko...@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 <ralph.go...@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 <cko...@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 > >>>> <ralph.go...@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 <c4kof...@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 > >>>>>> <ralph.go...@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 <cko...@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 <c4kof...@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 > >>>>>>>>> <ralph.go...@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 <g...@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 <ckozak@...> > >>>>>>>>>>> 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. > >>>>>>>>>>> > >>>>>>>>>>> ---- > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> --- > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>>> > >>>>>> > >>>>> > >>>>> > >>>> > >>> > >>> > >> > > > > > > > >
