On Thu, 16 Feb 2023 19:37:18 GMT, ExE Boss <d...@openjdk.org> wrote:
>> David M. Lloyd has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Use a unique index for the dumped lambda class instead of a time stamp
>
> src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java
> line 208:
>
>> 206: name = name.replace('/', '_');
>> 207: }
>> 208: return name.replace('.', '/') + "$$Lambda$";
>
> Using `String::concat` uses the same code path as indy‑based String
> concatenation[^1].
> Suggestion:
>
> return name.replace('.', '/').concat("$$Lambda");
>
>
> [^1]: The JDK uses `StringBuilder`‑based String concatenation:
> https://github.com/openjdk/jdk/blob/a39cf2e3b242298fbf5fafdb8aa9b5d4562061ef/make/modules/java.base/Java.gmk#L28
The original code used `+` so I assume it's not a problem here... but if I'm
wrong, wouldn't it be better to migrate from `+` to `concat` in a separate
change?
> src/java.base/share/classes/java/lang/invoke/ProxyClassesDumper.java line 103:
>
>> 101: final int len = className.length();
>> 102: // add some padding to `len` for the index
>> 103: StringBuilder sb = new StringBuilder(len + 5);
>
> Suggestion:
>
> StringBuilder sb = new StringBuilder(len + 6);
The index is unlikely to be more than 5 digits (pulled out of a hat really) for
most applications I would think; is there something I overlooked here?
> src/java.base/share/classes/java/lang/invoke/ProxyClassesDumper.java line 124:
>
>> 122: }
>> 123: }
>> 124: sb.append(counter.incrementAndGet());
>
> Suggestion:
>
> sb.append('$').append(counter.incrementAndGet());
The Lamdba class name already ends in `$` - is there a reason we'd need a
second one when dumping?
-------------
PR: https://git.openjdk.org/jdk/pull/12579
