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

[Coleen Phillimore]

On 3/9/16 4:12 PM, Coleen Phillimore wrote:
>
> 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.

I reverted this last change because it used CATCH to exit if there's an 
exception, ie didn't properly handle errors.  To pass a name to 
compute_offset() you need to add the string to vmSymbols.hpp for now.

Also hg added the test.

Thanks,
Coleen
>
> 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
>>>>>
>>>>
>>>
>>
>