RFR: 8202750: Reduce the use of get_canonical_path() in CDS [v2]

[Calvin Cheung]

On Tue, 16 Feb 2021 16:51:11 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> Calvin Cheung has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   per review comments from @iklam
>
> src/hotspot/share/classfile/classLoader.cpp line 1357:
> 
>> 1355:       assert(native_path_table_entry != NULL, "sanity");
>> 1356:       // If the path (from the class stream source) is the same as the shared
>> 1357:       // class or module path, then we have a match.
> 
> How about adding the comment:
> 
> // src may come from the App/Platform class loaders, which would canonicalize
> // the file name. We cannot use strcmp to check for equality against ent->name().
> // We must use os::same_files (which is faster than canonicalizing ent->name()).

Fixed.

> src/hotspot/share/classfile/classLoader.cpp line 1348:
> 
>> 1346:     char* native_path = get_native_path(path);
>> 1347:     // The path is from the ClassFileStream. Since a ClassFileStream has been created successfully in functions
>> 1348:     // such as ClassLoader::load_class(), its source path must be valid.
> 
> This comment was written when the class could only be parsed by the boot loader using ClassFileStream. Now the class can also be parsed by the app/platform class loaders. How about deleting this comment, and change the previous comment block to
> 
> // Save the path from the file: protocol or the module name from the jrt: protocol
> // if no protocol prefix is found, path is the same as stream->source(). This path
> // must be valid since the class has been successfully parsed.

Fixed.

> src/hotspot/share/classfile/classLoader.cpp line 1294:
> 
>> 1292:   return os::native_path(native_path);
>> 1293: }
>> 1294: 
> 
> If I understand correctly, the reason to call `os::native_path()` is the Windows version of `os::same_files()` cannot handle the difference between "/" (from `src`) and "\" (from `ent->name()`).
> 
> I think it's better to move the call to native_path inside the Windows implementation of `os::same_files()`. That way, we don't need to call `os::native_path()` in all the callers of `os::same_files()`.
> 
> This would also make it possible to use resource allocation inside `os::same_files()` to allocate the native path (instead of os::strdup).

I've added `os::native_path()` calls in the Windows version of `os::same_files()`.
`os::strip()` and `os::free()` are still being used because ResourceMark may not be available during early VM startup.

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

PR: https://git.openjdk.java.net/jdk/pull/2581