On Sun, 29 Nov 2020 01:21:08 GMT, Claes Redestad <[email protected]> wrote:
>> Here are the results, looks like we have no regression:
>> Benchmark (intValue) Mode Cnt
>> original patched Units Units
>> StringConcat.concat4String 4711 avgt 15 27.267 ±
>> 1.231 29.938 ± 1.099 ns/op ns/op
>> StringConcat.concat6String 4711 avgt 15 39.185 ±
>> 1.604 42.759 ± 2.564 ns/op ns/op
>> StringConcat.concatConst2String 4711 avgt 15 22.345 ±
>> 1.685 21.913 ± 1.716 ns/op ns/op
>> StringConcat.concatConst4String 4711 avgt 15 32.318 ±
>> 4.073 32.847 ± 1.082 ns/op ns/op
>> StringConcat.concatConst6Object 4711 avgt 15 9.227 ±
>> 0.146 9.999 ± 0.939 ns/op ns/op
>> StringConcat.concatConst6String 4711 avgt 15 46.134 ±
>> 2.729 44.903 ± 1.960 ns/op ns/op
>> StringConcat.concatConstBoolByte 4711 avgt 15 13.117 ±
>> 0.725 12.575 ± 0.380 ns/op ns/op
>> StringConcat.concatConstInt 4711 avgt 15 12.230 ±
>> 0.653 11.890 ± 0.556 ns/op ns/op
>> StringConcat.concatConstIntConstInt 4711 avgt 15 18.152 ±
>> 1.317 17.722 ± 0.566 ns/op ns/op
>> StringConcat.concatConstString 4711 avgt 15 12.644 ±
>> 0.656 13.424 ± 1.400 ns/op ns/op
>> StringConcat.concatConstStringConstInt 4711 avgt 15 20.836 ±
>> 0.703 19.975 ± 0.821 ns/op ns/op
>> StringConcat.concatEmptyConstInt 4711 avgt 15 10.604 ±
>> 0.443 10.229 ± 0.219 ns/op ns/op
>> StringConcat.concatEmptyConstString 4711 avgt 15 4.844 ±
>> 0.174 4.721 ± 0.147 ns/op ns/op
>> StringConcat.concatEmptyLeft 4711 avgt 15 5.386 ±
>> 0.190 5.314 ± 0.104 ns/op ns/op
>> StringConcat.concatEmptyRight 4711 avgt 15 5.352 ±
>> 0.471 5.484 ± 0.354 ns/op ns/op
>> StringConcat.concatMethodConstString 4711 avgt 15 11.887 ±
>> 0.560 14.025 ± 1.522 ns/op ns/op
>> StringConcat.concatMix4String 4711 avgt 15 172.425 ±
>> 6.868 169.658 ± 8.440 ns/op ns/op
>
> Right.
>
> I won't insist but I think some of these could warrant some extra runs and
> analysis of the disassembly just to make sure. E.g. `concatMethodConstString`
> and `concat6String` has regression with just barely non-overlapping errors.
> If these regressions persist it could be due the small difference in the code
> path (one extra call depth for `getBytes(byte[], int, byte)`) causing some
> difference in compilation order, inlining decision or otherwise.
>
> If the regression is real, the paranoid workaround would be to restore
> `String.getBytes(byte[], int, byte)` and not use delegation. A tiny bit of
> code duplication might be preferable to a surprise regression in code that
> has seen a lot of tuning work..
Looks like you are right: delegation caused tiny regression (I've rerun the
benchmark in 10 forks) which is missing from the case with no delegation (third
column):
Benchmark (intValue) Mode Cnt original
patched no delegate Units
StringConcat.concat4String 4711 avgt 100 26.400 ± 0.092
28.278 ± 0.046 26.245 ± 0.045 ns/op
StringConcat.concat6String 4711 avgt 100 37.846 ± 0.219
40.054 ± 0.150 37.857 ± 0.175 ns/op
StringConcat.concatConst2String 4711 avgt 100 18.732 ± 0.125
19.600 ± 0.153 18.598 ± 0.178 ns/op
StringConcat.concatConst4String 4711 avgt 100 29.303 ± 0.165
31.085 ± 0.086 29.246 ± 0.194 ns/op
StringConcat.concatConst6Object 4711 avgt 100 9.107 ± 0.164
8.948 ± 0.010 8.964 ± 0.059 ns/op
StringConcat.concatConst6String 4711 avgt 100 40.441 ± 0.111
41.037 ± 0.052 40.503 ± 0.352 ns/op
StringConcat.concatConstBoolByte 4711 avgt 100 11.934 ± 0.054
12.173 ± 0.197 11.904 ± 0.026 ns/op
StringConcat.concatConstInt 4711 avgt 100 10.968 ± 0.024
11.041 ± 0.034 10.956 ± 0.006 ns/op
StringConcat.concatConstIntConstInt 4711 avgt 100 16.853 ± 0.070
16.958 ± 0.064 16.932 ± 0.070 ns/op
StringConcat.concatConstString 4711 avgt 100 11.440 ± 0.113
12.049 ± 0.116 11.485 ± 0.116 ns/op
StringConcat.concatConstStringConstInt 4711 avgt 100 18.205 ± 0.068
18.612 ± 0.043 18.146 ± 0.065 ns/op
StringConcat.concatEmptyConstInt 4711 avgt 100 9.733 ± 0.121
9.742 ± 0.250 9.592 ± 0.011 ns/op
StringConcat.concatEmptyConstString 4711 avgt 100 4.601 ± 0.054
4.589 ± 0.030 4.579 ± 0.023 ns/op
StringConcat.concatEmptyLeft 4711 avgt 100 5.088 ± 0.031
5.109 ± 0.040 5.119 ± 0.059 ns/op
StringConcat.concatEmptyRight 4711 avgt 100 4.896 ± 0.038
4.876 ± 0.059 4.844 ± 0.025 ns/op
StringConcat.concatMethodConstString 4711 avgt 100 11.561 ± 0.157
12.126 ± 0.148 11.580 ± 0.162 ns/op
StringConcat.concatMix4String 4711 avgt 100 156.119 ± 0.675
154.814 ± 0.169 155.655 ± 1.741 ns/op
So I've dropped delegation and restored `String.getBytes(byte[], int, byte)`
-------------
PR: https://git.openjdk.java.net/jdk/pull/402
