RFR JDK-8193325: StackFrameInfo::getByteCodeIndex returns wrong value if bci > 32767

[Mandy Chung]

Peter, Aleksey,

On 8/13/19 2:17 AM, Peter Levart wrote:
> What are you trying to achieve with @Stable annotation?
>

I chose the path of using it for at least documentation purpose that it 
is initialized once after <init> :)  This field is not final as it's 
modified by the VM but it's a stable variable that can benefit from JIT 
optimization while it's unlikely a hot path with the current usage.

> If you were hoping for eventual optimization by JIT (i.e. constant 
> folding) then this would only be effective if JIT could prove that the 
> reference to the StackFrameInfo instance can also be constant folded. 
> This is very unlikely considering the use cases of StackFrameInfo 
> objects (results returned by StackWalker API). There are currently no 
> other optimizations that I know of that would pertain to @Stable 
> annotation.
>
> If you were hoping to achieve some kind of "safer" publication of 
> StackFrameInfo instances (like for example when the fields are final 
> and initialized in the constructor) then @Stable is not a replacement 
> for final. Even if you declared bci as final, its effective value is 
> assigned in the VM after constructor is finished. So the JMM rules for 
> final fields can not apply here. So it is my belief that neither 
> placing @Stable nor final on the bci has any desirable effect. I would 
> just leave it as plain instance field. 

There is no existing way to annotate a stable variable besides 
comments.  It's fair to wait until we have data to show performance 
benefit to add @Stable.   None of the choices (int, final int, @Stable 
int) is ideal.   I'll drop @Stable in webrev.04 and move on if that's 
okay with you.

> I do like the zero default value of it and corresponding +/-1 offset 
> computations in the getter and initialization code. If there is a data 
> race when publishing StackFrameInfo objects unsafely to other threads, 
> let there be only two values that are observable instead of three. BTW 
> the unsafe publication applies not only to bci value but to the whole 
> instance referenced by the 'memberName' field. The field is final and 
> initialized in the constructor, but the MemberName instance is 
> modified afterwards...
>

True.

> So let's not pretend publishing StackFrameInfo via data race is safe 
> (like for example String objects). And I think it is not critical to 
> be safe - no security decisions are being made on the StackFrameInfo 
> objects' content, right?
>

Right.

Mandy
> Regards, Peter
>