RFR: 8289908: Skip bounds check for cases when String is constructed from entirely used byte[]

Roger Riggs rriggs at openjdk.org
Fri Jul 8 14:07:32 UTC 2022


On Fri, 8 Jul 2022 07:37:36 GMT, Сергей Цыпанов <duke at openjdk.org> wrote:

>> src/java.base/share/classes/java/lang/String.java line 1429:
>> 
>>> 1427:      */
>>> 1428:     public String(byte[] bytes, int offset, int length) {
>>> 1429:         this(bytes, offset, length, Charset.defaultCharset(), checkBoundsOffCount(offset, length, bytes.length));
>> 
>> Can you avoid the extra constructor by applying `checkBoundOffCount` to the offset argument; it returns the offset.
>> 
>> this(bytes, checkBoundsOffCount(offset, length, bytes.length), length, Charset.defaultCharset());
>> 
>> or call `Preconditions.checkFromIndexSize` directly.
>
> But if I roll back the added constructor I'll go through existing public one `public String(byte[] bytes, int offset, int length, Charset charset)` doing bounds check twice, won't I?

The new constructor looks very odd, especially when it does not have an explanation and doesn't describe the required preconditions for calling it.  Is there a better way than adding a non-functional argument?
The "unused" name is going to draw a warning from IntelliJ and some enterprising developer is going to try to remove it, not understanding why its there. And there is a bit of overhead pushing the extra arg.

The constructor should be private, it has a very narrow scope of use given the lack of checking its args.

It would be nice if the Hotspot compiler would recognize the llmits on the range and optimize away the checks;
it would have broader applicability then this point fix.
I would be interesting to ask the compiler folks if that's a future optimization.
These source changes make it harder to understand what's happening where; though that is sometimes work it for a noticeable performance improvement.

-------------

PR: https://git.openjdk.org/jdk/pull/9407



More information about the security-dev mailing list