Request for Review : CR#6924259: Remove String.count/String.offset

[David Schlosnagle]

>> On 24/05/2012 21:45, Mike Duigou wrote:
>>>
>>> Hello all;
>>>
>>> For a long time preparations and planing have been underway to remove the
>>> offset and count fields from java.lang.String. These two fields enable
>>> multiple String instances to share the same backing character buffer. Shared
>>> character buffers were an important optimization for old benchmarks but with
>>> current real world code and benchmarks it's actually better to not share
>>> backing buffers. Shared char array backing buffers only "win" with very
>>> heavy use of String.substring. The negatively impacted situations can
>>> include parsers and compilers however current testing shows that overall
>>> this change is beneficial.
>>>
>>> The current plan is to commit this change to jdk8 before build 40
>>> followed by jdk7u6. Removing offset/count is a complicated process because
>>> HotSpot has special case code for String that depends upon the two fields.
>>> Vladimir Kozlov has committed the required HotSpot changes already
>>> (http://hg.openjdk.java.net/jdk8/jdk8/hotspot/rev/8f972594effc).
>>>
>>> A patch of the changes to java.lang.String for JDK 8 are
>>> at<http://cr.openjdk.java.net/~mduigou/6924259/0/webrev/>. The changes for
>>> JDK 7 have only superficial differences (line offsets in the patch).
>>>
>>> Comments are welcome.
>>>
>>> Mike

Hi Mike,

I had a few quick comments that I didn't see exactly covered so far:

Should we extract original.value to a local to avoid multiple getfield
instructions in the non-empty case?

    public String(String original) {
        char[] originalValue = original.value;
        this.value = (originalValue.length > 0)
                ? originalValue
                : EMPTY_STRING_VALUE;
        this.hash = original.hash;
    }


Should the len be moved outside of the if condition in the following, and use
Arrays.copyOf (or value.clone()) to simplify the code and hopefully be
intrinsified?

    public String(char value[]) {
        final int len = value.length;
        this.value = (len > 0)
                ? Arrays.copyOf(value, len);
                : EMPTY_STRING_VALUE;
    }

[1] http://markmail.org/message/ubg6s5dyvsidjipj

Thanks,
Dave