RFR 8170155: StringBuffer and StringBuilder stream methods are not late-binding

[Paul Sandoz]

> On 25 Nov 2016, at 02:47, Tobias Hartmann <tobias.hartmann at oracle.com> wrote:
> 
>> I'm not sure if it is still desired to do the same boundary check in case of LATIN1
>> for the benefit of consistency.  Assume there might be concurrent access/operation
>> between val = this.value and count = this.count; (for StringBuilder) for example,
>> the this.value got doubled and the this.count got increased. One will end up with
>> StringIndexOutOfBoundsException() from checkOffset, but the other will be ioobe
>> from vm?
> 
> You mean when hitting a check in an intrinsic compared to when hitting a check in the Java wrapper?
> 

Not quite. There is a spliterator implementation for each coder, in the case of the LATIN1 coder there are no associated intrinsics. I think it’s ok just to perform the explicit bounds check for the UTF16 coder, since for the LATIN1 bounds checks will be performed by baloads.

However, i think there is bug. The coder could change from LATIN1 to UTF16, while holding a reference to a byte[] value as LATIN1.

For StringBuilder i could fix that by atomically reading the value/count/coder, but there is still an issue with StringBuffer. Thus, in the UTF16 coder spliterator we need to perform the explicit bounds before *every* StringUTF16.getChar().

Webrev updated:

  http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8170155-string-buffer-builder-late-binding/webrev/

Paul.

> Actually, bound checks should always be done in the Java wrapper method that calls the (unchecked) intrinsic. In general, the unchecked intrinsic should not be called directly. StringUTF16.putChar/getChar() is an exception because there are places where we we need to omit the check for performance reasons.
> 
> I'm planning to revisit this with JDK-8156534 in JDK 10.
> 
> Best regards,
> Tobias
> 
>> 
>> Sherman
>> 
>> 
>>> If so i propose the following to make this clearer:
>>> 
>>> return StreamSupport.intStream(
>>>         () ->  {
>>>             byte[] val = this.value; int count = this.count;
>>>             if (coder == LATIN1) {
>>>                 return new StringLatin1.CharsSpliterator(val, 0, count, 0);
>>>             } else {
>>>                 // Perform an explicit bounds check since HotSpot
>>>                 // intrinsics to access UTF-16 characters in the byte[]
>>>                 // array will not perform bounds checks
>>>                 checkOffset(count, val.length>>  coder);
>>>                 return new StringUTF16.CharsSpliterator(val, 0, count, 0);
>>>             }
>>>         },
>>>         Spliterator.ORDERED | Spliterator.SIZED | Spliterator.SUBSIZED,
>>>         false);
>>> 
>>> Paul.
>>