RFR: 8240094: Optimize empty substring

Claes Redestad claes.redestad at oracle.com
Wed Feb 26 14:12:58 UTC 2020


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


More information about the core-libs-dev mailing list