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

Reply via email to