On Mon, 3 Apr 2023 18:07:52 GMT, Jorn Vernee <jver...@openjdk.org> wrote:
>> Mandy Chung has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> set -D<key> or -D<key>=true will enable dumping
>
> src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java
> line 85:
>
>> 83:
>> 84: // Used to ensure that dumped class files for failed definitions
>> have a unique class name
>> 85: private static final AtomicInteger counter = new AtomicInteger();
>
> This counter could be removed now, it looks like.
thanks for catching it. will fix.
> src/java.base/share/classes/jdk/internal/util/ClassFileDumper.java line 115:
>
>> 113: if (dumper.isEnabled() && !path.equals(dumper.dumpPath())) {
>> 114: throw new IllegalArgumentException("mismatched dump path
>> for " + key);
>> 115: }
>
> I don't see how this exception case could ever occur, given that
> `dumper.dumpPath` is directly coming from `dir`, and `validateDumpDir`
> doesn't return a modified path either.
>
> Can this check be removed?
This is to check if two callers get the `ClassFileDumper` of the same key but
different path.
ClassFileDumper.getInstance("jdk.foo.dump", "DUMP_DIR");
ClassFileDumper.getInstance("jdk.foo.dump", "DUMP_NEW_DIR"); <--- IAE
> src/java.base/share/classes/jdk/internal/util/ClassFileDumper.java line 145:
>
>> 143: public Path pathname(String internalName) {
>> 144: return dumpDir.resolve(encodeForFilename(internalName) +
>> ".class");
>> 145: }
>
> Could be `private` I think
It's public in case the caller who invokes `ClassDefiner::defineClass` wants to
print the path to which the classfile is written.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13182#discussion_r1156367767
PR Review Comment: https://git.openjdk.org/jdk/pull/13182#discussion_r1156366699
PR Review Comment: https://git.openjdk.org/jdk/pull/13182#discussion_r1156367573
