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 <[email protected]> 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 <[email protected]> > 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 <[email protected]> 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 <[email protected]> >>> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> >>>>>> 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 <[email protected]> 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. >>>>>>>> >>>>>>>> ---- >>>>>>>> >>>>>>>> >>>>>>>> --- >>>>>>>> >>>>>>> >>>>>>> >>>>> >>>> >>>> >>> >> >> >
