RFR: 8357220: Introduce a BSMAttributeEntry struct
Lois Foltan
lfoltan at openjdk.org
Wed May 21 17:49:53 UTC 2025
On Mon, 19 May 2025 07:35:16 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:
> Hi,
>
> The constant pool currently has a lot of methods specific to extracting parts of the operands array. What this array actually is, is a sequence of bootstrap method attribute entries, where each entry has the following components:
>
> ```c++
> struct BSMAE {
> u2 bootstrap_method_index;
> u2 argument_count;
> u2 arguments[argument_count];
> }
>
>
> We can removes some of these operands array specific methods, and instead allows you to extract BSMAttributeEntrys which you can then use to extract its piece wise components. This makes for a nicer interface, and a bit easier to come into as a reader of the code, as it more closely mirrors the JVMS.
>
> Please consider!
>
> Testing: Currently GHA, running tier1-tier3
Overall looks good, a couple of comments below.
src/hotspot/share/oops/constantPool.hpp line 80:
> 78: };
> 79:
> 80: class BSMAttributeEntry {
Please consider moving the description comment from lines #569-#572 ahead of this structure
src/hotspot/share/oops/constantPool.hpp line 92:
> 90: return reinterpret_cast<u2*>(this + 1);
> 91: }
> 92:
Remove blank lines at #92, #95, #103
src/hotspot/share/oops/constantPool.hpp line 105:
> 103:
> 104: int argument_index(int n) const {
> 105: assert(checked_cast<u2>(n) < _argument_count, "oob");
Should the assert contain a check that _argument_count is >= 0 as well?
src/hotspot/share/oops/constantPool.hpp line 638:
> 636: u2 bootstrap_argument_count_at(int cp_index) {
> 637: assert(tag_at(cp_index).has_bootstrap(), "Corrupted constant pool");
> 638: int bsmai = bootstrap_methods_attribute_index(cp_index);
The "matched ending" assert was the only code that used bootstrap_operand_limit(), so that method could be removed as well. This comment applies to bootstrap_operand_base() as well.
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/ConstantPool.java line 124:
> 122: private static long elementSize;
> 123:
> 124: private static int INDY_BSM_OFFSET = 0;
Please update the copyright of this file.
-------------
PR Review: https://git.openjdk.org/jdk/pull/25298#pullrequestreview-2858597040
PR Review Comment: https://git.openjdk.org/jdk/pull/25298#discussion_r2100820121
PR Review Comment: https://git.openjdk.org/jdk/pull/25298#discussion_r2100820521
PR Review Comment: https://git.openjdk.org/jdk/pull/25298#discussion_r2100831579
PR Review Comment: https://git.openjdk.org/jdk/pull/25298#discussion_r2100833066
PR Review Comment: https://git.openjdk.org/jdk/pull/25298#discussion_r2100849208
More information about the serviceability-dev
mailing list