RFR: 8197594 - String and character repeat

[Jim Laskey]

> On Feb 15, 2018, at 3:36 PM, Ivan Gerasimov <ivan.gerasimov at oracle.com> wrote:
> 
> Hello!
> 
> The link with the webrev returned 404, but I could find it at this location: http://cr.openjdk.java.net/~jlaskey/8197594/webrev-00/
> 
> A few minor comments:
> 
> 1)
> 
> This check:
> 
> 2992             final long limit = (long)count * 2L;
> 2993             if ((long)Integer.MAX_VALUE < limit) {
> 
> can be possibly simplified as
> if (count > Integer.MAX_VALUE - count) {

Good.

> 
> 2)
> Should String repeat(final int codepoint, final int count) be optimized for codepoints that can be represented with a single char?
> 
> E.g. like this:
> 
> public static String repeat(final int codepoint, final int count) {
>    return Character.isBmpCodePoint(codepoint))
>        ? repeat((char) codepoint, count)
>        : (new String(Character.toChars(codepoint))).repeat(count);
> }

Yes, avoid array allocation.

> 
> 3)
> Using long arithmetic can possibly be avoided in the common path of repeat(final int count):
> 
> E.g. like this:
> 
>         if (count < 0) {
>             throw new IllegalArgumentException("count is negative, " + count);
>         } else if (count == 1) {
>             return this;
>         } else if (count == 0) {
>             return "";
> }
>         final int len = value.length;
>         if (Integer.MAX_VALUE / count < len) {
>             throw new IllegalArgumentException(
>                     "Resulting string exceeds maximum string length: " + ((long)len * (long)count));
>         }
>         final int limit = count * len;

Good.

Thank you.

> 
> With kind regards,
> Ivan
> 
> On 2/15/18 9:20 AM, Jim Laskey wrote:
>> This is a pre-CSR code review [1] for String repeat methods (Enhancement).
>> 
>> The proposal is to introduce four new methods;
>> 
>> 1. public String repeat(final int count)
>> 2. public static String repeat(final char ch, final int count)
>> 3. public static String repeat(final int codepoint, final int count)
>> 4. public static String repeat(final CharSequence seq, final int count)
>> 
>> For the sake of transparency, only 1 is necessary, 2-4 are convenience methods.
>> In the case of 2, “*”.repeat(10) performs as well as String.repeat(‘*’, 10).
>> 3 and 4 convert to String before calling 1.
>> 
>> Performance runs with jmh (results as comment in [2]) show that these
>> methods are significantly faster that StringBuilder equivalents.
>>  - fewer memory allocations
>>  - fewer char to byte array conversions
>>  - faster pyramid replication vs O(N) copying
>> 
>> I left StringBuilder out of scope. It falls under the category of
>> Appendables#append with repeat. A much bigger project.
>> 
>> All comments welcome. Especially around the need for convenience
>> methods, the JavaDoc content and expanding the tests.
>> 
>> — Jim
>> 
>> [1] webrev: http://cr.openjdk.java.net//oj/home/jlaskey/8197594/webrev-00
>> [2] jbs: https://bugs.openjdk.java.net/browse/JDK-8197594
>> 
>> 
> 
> -- 
> With kind regards,
> Ivan Gerasimov
>