RFR (S) 8134758: Final String field values should be trusted as stable

[Vladimir Kozlov]

On 8/31/15 10:15 AM, Aleksey Shipilev wrote:
> On 08/31/2015 07:25 PM, Vladimir Kozlov wrote:
>> On 8/31/15 7:59 AM, Aleksey Shipilev wrote:
>>> I would like to make a forward move and make VM to trust all final
>>> String fields. I cannot quickly find the scenario where it helps current
>>> JDK -- there is only String.value field, which components are not
>>> treated as constants anyway. But, it helps a lot the upcoming Compact
>>> Strings change, which introduces String.coder field.
>>
>> Reflection?
>
> Sorry? In Compact Strings, String.coder field defines how to interpret
> String.value byte[] array. See the example in the bug:
>
>      public int length() {
>          return value.length >> coder;
>      }
>
> Ah, and if you worry about changing the field via Reflection, then like
> String.value, which is already treated as "stable" by intrinsics,
> String.coder is not supposed to be changed via Reflection (that is,
> without potentially devastating consequences). Strings are also
> deserialized via StringBuilder, so we are safe there as well.

Yes, I meant changing fields. Thank you for explaining.

>
>>>
>>> String.value is actually handled as stable in the GraphKit with
>>> UseImplicitStableValues, but it does not affect "normal" Java code.
>>
>> What do you mean by "normal" code?
>
> The code that is not handled by intrinsics on any other special explicit
> magic in a compiler. Just plain Java code, like the length() method
> above. How do you call it?

Got it. LoadNode::Value() has special case for String fields when string is constant. We can add UseImplicitStableValues 
code there for non-constant String.

Thanks,
Vladimir

>
>
>>> Therefore, in a way, this change extends the same behavior to the normal
>>> code. See more here:
>>>     https://bugs.openjdk.java.net/browse/JDK-8134758
>>>
>>> Here is a patch:
>>>     http://cr.openjdk.java.net/~shade/8134758/webrev.00/
>>
>> Do we still need UseImplicitStableValues code in
>> GraphKit::load_String_value() and library_call.cpp?
>
> I think so. These seem to enforce that array components are also treated
> as constants? Vladimir I., can you take a look?
>
> Thanks,
> -Aleksey
>
>