On Thu, 14 Nov 2024 05:45:48 GMT, David Holmes <[email protected]> wrote:
>> Coleen Phillimore has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Purge last references to SecurityManager.
>
> src/hotspot/share/classfile/dictionary.cpp line 80:
>
>> 78:
>> 79: void Dictionary::Config::free_node(void* context, void* memory, Value
>> const& value) {
>> 80: delete value; // Call DictionaryEntry destructor
>
> `using Value = XXX` seems like an unwanted/unnecessary abstraction in this
> code, because depending on what `XX` is you either will or won't need to call
> `delete`. That is a more general cleanup though.
This is sort of the standard way we use the CHT.
> src/hotspot/share/classfile/javaClasses.cpp line 1617:
>
>> 1615: macro(_holder_offset, k, "holder",
>> thread_fieldholder_signature, false); \
>> 1616: macro(_name_offset, k, vmSymbols::name_name(),
>> string_signature, false); \
>> 1617: macro(_contextClassLoader_offset, k, "contextClassLoader",
>> classloader_signature, false); \
>
> I didn't think the context class loader was related to SM in any way. ??
It isn't. This symbol was near the ones I deleted, and I deleted it by
mistake, so I moved it here.
> src/hotspot/share/logging/logDiagnosticCommand.hpp line 62:
>
>> 60: }
>> 61:
>> 62: static const JavaPermission permission() {
>
> Is any of this permission stuff still relevant? I couldn't figure out what
> ultimately looks at them. ??
I don't know that. It is passed by the MBean code. It might be another
(different) opportunity for a cleanup if the MBean code doesn't use it anymore.
> src/hotspot/share/prims/jvm.cpp line 154:
>
>> 152: */
>> 153:
>> 154: extern void trace_class_resolution(Klass* to_class) {
>
> why `extern` ?
jni.cpp functions call this.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22064#discussion_r1843665871
PR Review Comment: https://git.openjdk.org/jdk/pull/22064#discussion_r1843668071
PR Review Comment: https://git.openjdk.org/jdk/pull/22064#discussion_r1843668939
PR Review Comment: https://git.openjdk.org/jdk/pull/22064#discussion_r1843667220
