On Thu, 27 Apr 2023 14:21:23 GMT, Coleen Phillimore <[email protected]> wrote:
>> This change moves the flags from AccessFlags to either ConstMethodFlags or
>> MethodFlags, depending on whether they are set at class file parse time,
>> which makes them essentially const, or at runtime, which makes them needing
>> atomic access.
>>
>> This leaves AccessFlags int size because Klass still has JVM flags that are
>> more work to move, but this change doesn't increase Method size. I didn't
>> remove JVM_RECOGNIZED_METHOD_MODIFIERS with this change since there are
>> several of these in other places, and with this change the code is benign.
>>
>> Tested with tier1-6, and some manual verification of printing.
>
> Coleen Phillimore has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Remove bool argument from ConstMethodFlags.set function.
Nice change! Just some small nits but it otherwise looks good.
src/hotspot/share/oops/method.hpp line 606:
> 604:
> 605: bool compute_has_loops_flag();
> 606: bool set_has_loops() { set_has_loops_flag();
> set_has_loops_flag_init(); return true; }
Since this has multiple statements it should probably be on different lines
src/hotspot/share/oops/method.hpp line 615:
> 613: // has not been computed yet.
> 614: bool guaranteed_monitor_matching() const { return
> monitor_matching(); }
> 615: void set_guaranteed_monitor_matching() {
> set_monitor_matching(); }
Is this method just obsolete now? If so it might be worth replacing the callers
with `set_monitor_matching()` unless `set_monitor_matching()` is still meant to
be private.
-------------
Changes requested by matsaave (Committer).
PR Review: https://git.openjdk.org/jdk/pull/13654#pullrequestreview-1406036462
PR Review Comment: https://git.openjdk.org/jdk/pull/13654#discussion_r1180462644
PR Review Comment: https://git.openjdk.org/jdk/pull/13654#discussion_r1180472698
