RFR: 8240094: Optimize empty substring

[Ivan Gerasimov]

Looks good to me!

The copyright years need to be updated.

With kind regards,

Ivan


On 2/26/20 6:12 AM, Claes Redestad wrote:
> Hi,
>
> I took the liberty to file 
> https://bugs.openjdk.java.net/browse/JDK-8240094 for this.
>
> Webrev: http://cr.openjdk.java.net/~redestad/8240094/open.00/
>
> I added a trivial micro and verified that there's a roughly 2.5x
> improvement in this targeted test on my machine (~10 ns/op -> ~4ns/op),
> while staying neutral on a selection of benchmarks that exercise
> newString.
>
> /Claes
>
> On 2020-02-26 14:19, Claes Redestad wrote:
>> On 2020-02-26 11:01, Сергей Цыпанов wrote:
>>> Currently we have
>>>
>>> public static String stripLeading(byte[] value) {
>>>    int left = indexOfNonWhitespace(value);
>>>    if (left == value.length) {
>>>      return "";
>>>    }
>>>    return (left != 0) ? newString(value, left, value.length - left) 
>>> : null;
>>> }
>>>
>>> With the patch we change behaviour of this method for the case when 
>>> value.length == 0:
>>>
>>> public static String stripLeading(byte[] value) {
>>>    int left = indexOfNonWhitespace(value);
>>>    return (left != 0) ? newString(value, left, value.length - left) 
>>> : null;
>>> }
>>>
>>> Unlike original method this code returns null instead of "" for 
>>> empty array.
>>> This does not affect caller (String.stripLeading()) i. e. visible 
>>> behaviour
>>> remains the same for String user, but is it OK in general?
>>
>> One observable difference on the public API I think will happen here is
>> that new String("").stripLeading() == "" will change from true to false,
>> since the null return from the inner method mean we return the argument.
>>
>>  From a compatibility point of view I think this should be fine, as
>> the identity of the returned empty string isn't specified. I don't think
>> a CSR is required, but bringing it up since others think otherwise.
>>
>> Overall I like how the patch now cleans things up a bit on top of
>> (likely) optimizing a few edge cases, and can volunteer to sponsor it if
>> no one else has spoken up already.
>>
>> Doing some quick performance testing and possibly adding a
>> microbenchmark before push would be good.
>>
>> Thanks!
>>
>> /Claes

-- 
With kind regards,
Ivan Gerasimov