RFR (S) 8134758: Final String field values should be trusted as stable
Vladimir Kozlov
vladimir.kozlov at oracle.com
Tue Sep 1 01:31:04 UTC 2015
Why you check constant.basic_type() == T_OBJECT?
In old compact strings version it was value field's type but now it is
always array.
This optimization only done for Sting which are constant. Is it enough
for you?
Thanks,
Vladimir
On 8/31/15 5:22 PM, Aleksey Shipilev wrote:
> On 08/31/2015 11:36 PM, Aleksey Shipilev wrote:
>> On 08/31/2015 08:25 PM, Vladimir Kozlov wrote:
>>> 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:
>>>>>> 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.
>>
>> Ohhhh, thanks! In fact, that where it should be fixed.
>>
>> Current JDK 9 code captures T_INT constants there, but we need T_BYTE as
>> well. Compact Strings version has even more constant types handled, but
>> not T_BYTE [1] (this should be the leftovers from CompressedStrings).
>> Tried to hack T_BYTE there for Compact Strings, and the same performance
>> effect was achieved.
>>
>> I'll see how and where to fix this properly. Tentatively, I'd say all
>> types should be handled in the mainline version, and Compact Strings
>> should purge its own version, to avoid further omissions.
>
> All right, how about this?
> http://cr.openjdk.java.net/~shade/8134758/webrev.01/
>
> Respinned JPRT, plus length() benchmarks against Compact Strings,
> eyeballed assembly.
>
> Tobias, please take a look as well, since you are the one who would
> probably do the merge to Compact Strings sandbox.
>
> Thanks,
> -Aleksey
>
>
>
More information about the hotspot-compiler-dev
mailing list