RFR: 8258048: Placeholder hash code is the same as Dictionary hash code [v2]

Coleen Phillimore coleenp at openjdk.java.net
Wed Dec 16 17:00:00 UTC 2020


On Wed, 16 Dec 2020 15:13:42 GMT, Lois Foltan <lfoltan at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision:
>> 
>>  - Merge branch 'master' into class-loading
>>  - 8258048: Placeholder hash code is the same as Dictionary hash code
>
> Nice cleanup Coleen!
> Lois

I took out some extra header file includes and fixed or removed some null checks.  I'm rerunning sanity checks (tier1 on linux-x64, macosx-64, windows-x64 and linux-aarch64).  Also cross compiled for ppc, arm, s390 and zero.  Changing include files can be dangerous.
I also reran jck lang and vm tests.

> src/hotspot/share/classfile/placeholders.cpp line 133:
> 
>> 131:                                                  Symbol* supername,
>> 132:                                                  Thread* thread) {
>> 133:   PlaceholderEntry* probe = get_entry(hash, name, loader_data);
> 
> Minor nit of a comment since you are in this file and specifically in this method can you consider changing the if statement conditional at line #143 to explicitly check if (probe != NULL)?  I noticed line #167 has this type of conditional and it would be good to have consistency.  I think there are some other cases in this file, like line #119, where it is inconsistent.

The if (probe) conditional on line #143 isn't necessary because we've just created a placeholder and code above would have null pointer references if it failed (in add_entry -> new_entry), so I removed it.  I fixed another check against NULL just above this. I don't see any others here at least.
Thanks for reviewing!

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

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


More information about the hotspot-runtime-dev mailing list