RFR: 8217442: Optimize native accesses to String.value

[Claes Redestad]

Hi,

why yes, those checks do seem necessary, don't they? So I did to some
digging before I settled on an approach (Erik Österlund helped me
interpret the profiles and explain to me why the repeated accesses are
a cost in these cases).

It appears it's related to crashes when debugging Strings that have only
been partially initialized. See
https://bugs.openjdk.java.net/browse/JDK-8025922 and
https://bugs.openjdk.java.net/browse/JDK-8021897 for more details.

So I don't think it's advisable to remove the null checks. And even
without them there would be a few double-accesses left to optimize away.

Thanks!

/Claes

On 2019-01-21 23:41, David Holmes wrote:
> Hi Claes,
> 
> Sorry to do this to you but I'm questioning the underlying assumption 
> here regarding the existing patterns of checking value for null before 
> getting the length. According to String.java
> 
>      * Additionally, it is marked with {@link Stable} to trust the contents
>       * of the array. No other facility in JDK provides this 
> functionality (yet).
>       * {@link Stable} is safe here, because value is never null.
>       */
>      @Stable
>      private final byte[] value;
> 
> value can never be null so the null checks should be removeable, then 
> you wouldn't have the double-accesses to value.
> 
> David
> -----
> 
> On 22/01/2019 8:07 am, Claes Redestad wrote:
>> Hi David,
>>
>> On 2019-01-21 21:54, David Holmes wrote:
>>> What if we refactored so that
>>>
>>> java_lang_String::length(oop java_string, typeArrayOop value)
>>>
>>> became
>>>
>>> java_lang_String::length(typeArrayOop value, bool isLatin1)
>>>
>>> would that also achieve the goal while presenting (IMHO) a cleaner API?
>>
>> doing so would get rid of the contorted asserts at least, and seems to
>> be performance neutral on all operations:
>>
>> http://cr.openjdk.java.net/~redestad/8217442/open.02/
>>
>> Re-running tests.
>>
>> /Claes