Request for review: 7034585 Adjust fillinStackTrace filtering to assist 6998871

[David Holmes]

Hi Tom,

Tom Rodriguez said the following on 04/09/11 06:53:
> Performance of fillInStackTrace is somewhat important so I think you should avoid using as_C_string since that creates a copy of both fillInStackTrace and the method being checked.  Maybe something like this:
> 
> const char* fillInStackTrace = "fillInStackTrace";
> int len = 16;
> assert(strlen(fillInStackTrace) == len, "must agree");
> if (method->name()->starts_with(fillInStackTrace, 16) &&
>     throwable->is_a(method->method_holder())) {

I hear you, but we shouldn't be hard-coding names like that - which is 
why we use the vmSymbols entry in the first place. It's a pity symbols 
don't have a set of operators for comparing against other symbols etc. 
I'll poke a little deeper to see what impact as_C_string will have and 
whether there is a way to mitigate it - perhaps use the variant where I 
supply the buffer eg:

   char[] buf = ...
   if 
(method->name()->starts_with(vmSymbols::fillInstackTrace()->as_C_string(buf), 
16) { ...

Thanks,
David

> tom
> 
> On Apr 8, 2011, at 4:33 AM, David Holmes wrote:
> 
>> webrev:
>>
>> http://cr.openjdk.java.net/~dholmes/7034585/webrev/
>>
>> When an exception is created, fillInStacktrace is called to populate the backtrace information. This is done in java_lang_Throwable::fill_in_stack_trace in the VM. Because the interesting part of the stacktrace is from the location where the exception was created, and upwards, filtering is applied in fill_in_stack_trace to remove the entry for fillInStackTrace() itself, and the exception constructors.
>>
>> The current filtering code only expects to find a single frame for the fillInStackTrace method, so if an exception class overrides fillInStackTrace (and invokes super.fillInStackTrace) this actually disables the filtering of the constructors. Eg we see:
>>
>> Exception in thread "main" MyException
>>        at MyException.fillInStackTrace(MyException.java:3)
>>        at java.lang.Throwable.<init>(Throwable.java:260)
>>        at java.lang.Exception.<init>(Exception.java:54)
>>        at java.lang.RuntimeException.<init>(RuntimeException.java:51)
>>        at MyException.<init>(MyException.java:1)
>>        at MyException.main(MyException.java:7)
>>
>> instead of:
>>
>> Exception in thread "main" MyException
>>        at MyException.main(MyException.java:7)
>>
>> The changes to Throwable.java for 6998871 effectively introduce an additional fillInStackTrace() frame and so this too disables the desired filtering of the backtrace.
>>
>> The proposal is quite simple: to modify fill_in_stack_trace so that it expects one or more fillInStackTrace frames, followed by zero or more constructor frames. This addresses the needs of 6998871 as well as fixing any user-defined overriding of fillInStackTrace.
>>
>> Thanks,
>> David
>