On Wed, 30 Jul 2025 17:32:57 GMT, John R Rose <jr...@openjdk.org> wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Test to verify observed internal unsafe behaviors
>
> src/hotspot/share/prims/unsafe.cpp line 496:
>
>> 494: Symbol *name = fs.name();
>> 495: if (name->equals(utf_name)) {
>> 496: offset = fs.offset();
>
> While you are here, if you keep this check, rename this particular function
> `find_field_offset` to `find_nonstatic_field_offset` (not the other one that
> takes the "must be static" flag).
>
> The logic, as written, is difficult to understand because of the behavioral
> difference between the two overloading.
>
> (I note that "instance field" is also a term of art in our code base, but
> "nonstatic_field" is more common.)
>
> Alternatively, and probably better, take out this particular check (restoring
> equal access to static and non-static) and just rely on the checks in Java
> code. Such checks are almost always better (easier to reason about by humans
> and JITs) than checks in C++ code.
This particular method intentionally avoids accessing Java reflective objects
to reduce startup overhead. I think a better approach may be returning error
code and have Java code report a failure. Thoughts?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25945#discussion_r2245996580
