On Sun, 4 Jul 2021 21:35:31 GMT, Сергей Цыпанов
<[email protected]> wrote:
>> As of JDK 17 some of primitive wrappers, e.g. `Long`, `Integer`, `Double`
>> and `Float` in their implementations of `Object.toString()` delegate to own
>> utility `toString(primitive)` methods.
>>
>> Unlike those, `Boolean`, `Byte`, `Character` and `Short` just duplicate the
>> contents of utility methods in implementations of `Object.toString()`.
>>
>> Yet another issue is a tiny discrepancy in implementation related to `Byte`
>> and `Short` (see the first):
>>
>> public static String toString(byte b) {
>> return Integer.toString((int)b, 10);
>> }
>>
>> public String toString() {
>> return Integer.toString((int)value);
>> }
>>
>> Unlike in overriden method, In utility one they explicitly specify radix
>> which can be skipped, as implementation of `Integer.toString(int,int)` has a
>> fast-path for radix 10, ending in `Integer.toString(int)`. This
>> simplification gives tiny improvement, see benchmark:
>>
>> @BenchmarkMode(Mode.AverageTime)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
>> public class ByteToStringBenchmark {
>> @Benchmark
>> public String byteToString() {
>> return Byte.toString((byte) 1);
>> }
>> }
>>
>> Results:
>>
>> before
>>
>> Benchmark Mode
>> Cnt Score Error Units
>> ByteToStringBenchmark.byteToString avgt
>> 30 11,648 ± 1,906 ns/op
>> ByteToStringBenchmark.byteToString:·gc.alloc.rate avgt
>> 30 3288,576 ± 418,119 MB/sec
>> ByteToStringBenchmark.byteToString:·gc.alloc.rate.norm avgt
>> 30 48,001 ± 0,001 B/op
>> ByteToStringBenchmark.byteToString:·gc.churn.G1_Eden_Space avgt
>> 30 3301,804 ± 455,932 MB/sec
>> ByteToStringBenchmark.byteToString:·gc.churn.G1_Eden_Space.norm avgt
>> 30 48,158 ± 2,085 B/op
>> ByteToStringBenchmark.byteToString:·gc.churn.G1_Survivor_Space avgt
>> 30 0,004 ± 0,001 MB/sec
>> ByteToStringBenchmark.byteToString:·gc.churn.G1_Survivor_Space.norm avgt
>> 30 ≈ 10⁻⁴ B/op
>> ByteToStringBenchmark.byteToString:·gc.count avgt
>> 30 202,000 counts
>> ByteToStringBenchmark.byteToString:·gc.time avgt
>> 30 413,000 ms
>>
>> after
>>
>> Benchmark Mode
>> Cnt Score Error Units
>> ByteToStringBenchmark.byteToString avgt
>> 30 10,016 ± 0,530 ns/op
>> ByteToStringBenchmark.byteToString:·gc.alloc.rate avgt
>> 30 3673,700 ± 167,450 MB/sec
>> ByteToStringBenchmark.byteToString:·gc.alloc.rate.norm avgt
>> 30 48,001 ± 0,001 B/op
>> ByteToStringBenchmark.byteToString:·gc.churn.G1_Eden_Space avgt
>> 30 3662,406 ± 205,978 MB/sec
>> ByteToStringBenchmark.byteToString:·gc.churn.G1_Eden_Space.norm avgt
>> 30 47,870 ± 1,750 B/op
>> ByteToStringBenchmark.byteToString:·gc.churn.G1_Survivor_Space avgt
>> 30 0,004 ± 0,002 MB/sec
>> ByteToStringBenchmark.byteToString:·gc.churn.G1_Survivor_Space.norm avgt
>> 30 ≈ 10⁻⁴ B/op
>> ByteToStringBenchmark.byteToString:·gc.count avgt
>> 30 224,000 counts
>> ByteToStringBenchmark.byteToString:·gc.time avgt
>> 30 358,000 ms
>
> Сергей Цыпанов has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - 8269665: Update copy-right year
> - 8269665: Reuse String.valueOf(boolean)
Changes requested by redestad (Reviewer).
src/java.base/share/classes/java/lang/Byte.java line 93:
> 91: */
> 92: public static String toString(byte b) {
> 93: return Integer.toString(b);
This change makes sense given the evidence that the radix tests in
`Integer.toString(int, int)` are not elided. Same for the equivalent change in
`Short.java`.
src/java.base/share/classes/java/lang/Byte.java line 441:
> 439: @Override
> 440: public String toString() {
> 441: return toString(value);
I'm a bit more skeptical about these changes that re-route from
`Integer.toString(int)` to the local `Byte.toString(byte)` which will then call
`Integer.toString(int)` anyhow. An extra indirection will likely not be seen
having an effect in micros, but can mess things up due inlining heuristics and
of course slightly hamper startup, so I would prefer leaving the code as is.
Adding missing `@Override`s is fine, of course.
-------------
PR: https://git.openjdk.java.net/jdk/pull/4633
