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

[Coleen Phillimore]

Hi Harold,
Thank you for the code review!

I added a test for the stack trace logging.  I also made some other 
minor changes that you pointed out.  I also added a function to find the 
field offset from a char* which is used by "depth", but that can be used 
by other things in javaClasses.cpp that need to know field offsets 
without adding to vmSymbols.hpp.

Also, I made the assertion changes discussed with David in this 
version.   Reran some jck tests.

open webrev at http://cr.openjdk.java.net/~coleenp/8150778.03_hotspot/
open webrev at http://cr.openjdk.java.net/~coleenp/8150778.03_jdk/

I didn't change jdk since 02.

Thanks,
Coleen


On 3/9/16 2:35 PM, harold seigel wrote:
> Hi Coleen,
>
> The JVM changes look good.  Could you add a test or test case for the 
> new 'stacktrace' logging tag?
>
> Thanks, Harold
>
> On 3/2/2016 7:03 PM, 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/
>>
>> 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
>>>>
>>>
>>
>