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

[Matias Saavedra Silva]

On Tue, 2 Apr 2024 13:06:24 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> This uses Ashutosh's patch to avoid adding duplicate constant pool entries during default method processing, with one change to make it conditional if not needed. I tested this locally and started writing a test, but writing a test is more difficult because duplicate constant pool entries don't actually violate the spec.
>> 
>> Tested with tier1-4 and ran some startup performance benchmarks on the patch.
>
> 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.

I have two comments below, but otherwise looks good!

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?

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.

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

PR Review: https://git.openjdk.org/jdk/pull/18548#pullrequestreview-1974554503
PR Review Comment: https://git.openjdk.org/jdk/pull/18548#discussion_r1548332016
PR Review Comment: https://git.openjdk.org/jdk/pull/18548#discussion_r1554047836