RFR: 8309751: Duplicate constant pool entries added during default method processing [v2]

[Coleen Phillimore]

On Tue, 2 Apr 2024 18:00:58 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Added a comment and did some renaming to make it clearer what this is doing.
>
> src/hotspot/share/classfile/bytecodeAssembler.cpp line 60:
> 
>> 58:     if (entry._tag != BytecodeCPEntry::tag::ERROR_TAG) {
>> 59:       bool created = false;
>> 60:       u2* probe =_indices.put_if_absent(entry, i, &created);
> 
> Is `probe` meant to be used here?

Nope. I removed it.  I can't use 'put' instead even though it returns 'created' because 'put' would update the index to this one, not the one it matched, which is sort of a subtle difference.

> src/hotspot/share/classfile/bytecodeAssembler.hpp line 133:
> 
>> 131:   GrowableArray<BytecodeCPEntry> _entries;
>> 132:   IndexHash _indices;
>> 133:   int _orig_cp_added;
> 
> I think the names `_entries` and `indices` could be improved to be more clear. Specifically, `_entries` should refer to the fact that it is a temporary copy of the constant pool that is used to generate the new constant pool.

I renamed these fields to _added_entries and _index_map which makes the code a bit clearer, and added a couple of comments here.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18548#discussion_r1554362353
PR Review Comment: https://git.openjdk.org/jdk/pull/18548#discussion_r1554361362