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