RFR 8150778: Reduce Throwable.getStackTrace() calls to the JVM

[David Holmes]

On 3/03/2016 10:03 AM, Coleen Phillimore wrote:
> Freshly tested changes with jck tests, with missing checks and other
> changes to use the depth field, as pointed out by Aleksey.  I've kept
> the StackTraceElement[] allocation in Java to match the new API that was
> approved.
>
> open webrev at http://cr.openjdk.java.net/~coleenp/8150778.02_hotspot/
> open webrev at http://cr.openjdk.java.net/~coleenp/8150778.02_jdk/

I get a 404 on that - the actual name seems to be jck. :)

David

> Thanks,
> Coleen
>
> On 3/2/16 2:57 PM, Coleen Phillimore wrote:
>>
>>
>> On 3/2/16 1:58 PM, Aleksey Shipilev wrote:
>>> Hi Coleen,
>>>
>>> On 03/02/2016 09:44 PM, Coleen Phillimore wrote:
>>>> Summary: replace JVM_GetStackTraceDepth and JVM_GetStackTraceElement,
>>>> with JVM_GetStackTraceElements that gets all the elements in the
>>>> StackTraceElement[]
>>>>
>>>> These improvements were found during the investigation for replacing
>>>> Throwable with the StackWalkAPI.   This change also adds iterator for
>>>> BacktraceBuilder to make changing format of backtrace easier.
>>>>
>>>> Tested with -testset core, RBT nightly hotspot nightly tests on all
>>>> platforms, and jck tests on linux x64.  Compatibility request is
>>>> approved.
>>>>
>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8150778_jdk/
>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8150778_hotspot
>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8150778
>>> Looks interesting!
>>>
>>> Is there an underlying reason why we can't return the pre-filled
>>> StackTraceElements[] array from the JVM_GetStackTraceElements to begin
>>> with? This will avoid leaking StackTraceElement constructor into
>>> standard library, *and* allows to make StackTraceElement fields final.
>>> Taking stuff back from the standard library is hard, if not impossible,
>>> so we better expose as little as possible.
>>
>> We measured that it's faster to allocate the StackTraceElement array
>> in Java and it seems cleaner to the Java guys.
>> It came from similar code we've been prototyping for StackFrameInfo.
>>>
>>> Other minor nits:
>>>
>>>   * Initializing fields to their default values is a code smell in Java:
>>>       private transient int depth = 0;
>>
>> ok, not sure what "code smell" means but it doesn't have to be
>> initialized like this.  It's set in the native code.
>>>
>>>   * Passing a null array to getStackTraceElement probably SEGVs? I don't
>>> see the null checks in native parts.
>>
>> Yes, it would SEGV.  I'll add some checks for null and make sure it's
>> an array of StackTraceElement.
>>
>> Thanks,
>> Coleen
>>>
>>> Thanks,
>>> -Aleksey
>>>
>>
>