RFR: 7903478: JMH: Use different padding names for B1 and B3 classes to avoid "hiding fields with same name" warnings

[Aleksey Shipilev]

On Tue, 30 May 2023 13:17:58 GMT, Vladimir Sitnikov <vsitnikov at openjdk.org> wrote:

>> jmh-core/src/main/java/org/openjdk/jmh/generators/core/Paddings.java line 42:
>> 
>>> 40:             throw new IllegalArgumentException("prefix must not be empty");
>>> 41:         }
>>> 42:         StringJoiner sj = new StringJoiner("");
>> 
>> What's the point of rewriting this to `StringJoiner`? `StringBuilder` optimizations would be nice to have, if available in the JIT.
>
> I doubt `StringBuilder` optimizations are available in JIT since the code uses for-loop and a non-trivial `String.format`. At the same time, `StringJoiner` can estimate the target buffer size before it joins.
> `StringBuilder` have to re-allocate the internal array.
> 
> Moving `new StringBuilder()` outside of a loop and using `setLength(0)` 
> 
> 
> @Fork(value = 3, jvmArgsPrepend = {"-Xmx128m"})
> @Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
> @Warmup(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
> @State(Scope.Thread)
> @BenchmarkMode(Mode.AverageTime)
> @OutputTimeUnit(TimeUnit.MICROSECONDS)
> 
> String prefix = "b1";
> 
> @Benchmark
> public List<String> paddings_list_old() {
>     List<String> list = new ArrayList<>();
>     for (int p = 0; p < 16; p++) {
>         StringBuilder sb = new StringBuilder();
>         sb.append("    ");
>         sb.append(String.format("byte p%03d", p * 16 + 0));
>         for (int q = 1; q < 16; q++) {
>             sb.append(String.format(", p%03d", p * 16 + q));
>         }
>         sb.append(";");
>         list.add(sb.toString());
>     }
>     return list;
> }
> 
> @Benchmark
> public List<String> paddings_list_reusedBuilder() {
>     List<String> list = new ArrayList<>();
>     StringBuilder sb = new StringBuilder();
>     for (int p = 0; p < 16; p++) {
>         sb.setLength(0);
>         sb.append("    ");
>         sb.append(String.format("byte p%03d", p * 16 + 0));
>         for (int q = 1; q < 16; q++) {
>             sb.append(String.format(", p%03d", p * 16 + q));
>         }
>         sb.append(";");
>         list.add(sb.toString());
>     }
>     return list;
> }
> 
> @Benchmark
> public String paddings_str_preallocatedBuilder() {
>     StringBuilder sb = new StringBuilder(1680);
>     for (int p = 0; p < 16; p++) {
>         sb.append("    ");
>         sb.append(String.format("byte p%03d", p * 16 + 0));
>         for (int q = 1; q < 16; q++) {
>             sb.append(String.format(", p%03d", p * 16 + q));
>         }
>         sb.append(";");
>         sb.append(System.lineSeparator());
>     }
> 
>     return sb.toString();
> }
> 
> @Benchmark
> public String paddings_str_joiner() {
>     String prefix = this.prefix;
>     StringJoiner sj = new StringJoiner("");
>     for (int p = 0; p < 16; p++) {
>         sj.add("    byte ");
>         for (int q = 0; q < 16; q++) {
>             if (q != 0) {
>                 sj.add(", ");
>             }
>             sj.add(prefix);
>             sj.add(String.format("%03d", p * 16 + q));
>         }
> ...

All right, fine. But you need to pass GHA testing with this patch.

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

PR Review Comment: https://git.openjdk.org/jmh/pull/104#discussion_r1211225152