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