On Fri, 8 Jul 2022 07:37:36 GMT, Сергей Цыпанов <d...@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
