Request for Review : CR#6924259: Remove String.count/String.offset (original) (raw)

David Schlosnagle schlosna at gmail.com
Fri May 25 17:24:46 UTC 2012

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

More information about the core-libs-dev mailing list