RFR: 8304069: ClassFileParser has ad-hoc hashtables

[Coleen Phillimore]

On Wed, 22 Mar 2023 14:56:55 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

> The ClassFileParser has ad-hoc hash tables for checking for duplicate names for fields, methods and interfaces. ResourceHashtable will now be used instead. Verified with tier1-4 tests.

I have some suggestions for improving this.

src/hotspot/share/classfile/classFileParser.cpp line 782:

> 780:     raw_hash += ((unsigned int)(uintptr_t)namesig->_sig) >> LogHeapWordSize;
> 781: 
> 782:     return (raw_hash + (unsigned int)(uintptr_t)namesig->_name) % HASH_ROW_SIZE;

I don't know if you need to use % since that's what the hashtable will do to find the bucket.

src/hotspot/share/classfile/classFileParser.cpp line 862:

> 860:     // Set containing interface names
> 861:     NameSigHashtable* interface_names = new NameSigHashtable();
> 862:     bool dup = true;

Can you change the variable name 'dup' to 'created'?

src/hotspot/share/classfile/classFileParser.cpp line 868:

> 866:       for (index = 0; (index < itfs_len) && dup; index++) {
> 867:         const InstanceKlass* const k = _local_interfaces->at(index);
> 868:         interface_name = new NameSigHash();

You could use a local NameSigHash and copy for the values like:
  name_sig_hash = NameSigHash(k->name(), nullptr);
Then the hashtable key could be a non-pointer, saving a pointer per entry and may be more efficient.

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

Changes requested by coleenp (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13141#pullrequestreview-1353171894
PR Review Comment: https://git.openjdk.org/jdk/pull/13141#discussion_r1145201747
PR Review Comment: https://git.openjdk.org/jdk/pull/13141#discussion_r1145214822
PR Review Comment: https://git.openjdk.org/jdk/pull/13141#discussion_r1145217695