RFR: 8306851: Move Method access flags

Coleen Phillimore coleenp at openjdk.org
Thu Apr 27 12:13:56 UTC 2023


On Thu, 27 Apr 2023 04:28:31 GMT, David Holmes <dholmes at openjdk.org> 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.
>
> src/hotspot/share/classfile/classFileParser.cpp line 2741:
> 
>> 2739:     parsed_annotations.apply_to(methodHandle(THREAD, m));
>> 2740: 
>> 2741:   if (is_hidden() && !m->is_hidden()) { // Mark methods in hidden classes as 'hidden'.
> 
> This seems odd - how would m already be marked hidden? And why do we care? The check-and-branch is more expensive than just setting the field.

This hidden flag is set by both the Method attribute during class file parsing and set to true if the whole class is hidden.  My original version of the assert_is_safe() function only allowed a flag to be set once, but I've changed it to only set to true.  So I can undo this.

> src/hotspot/share/oops/method.cpp line 735:
> 
>> 733:       case Bytecodes::_jsr:
>> 734:         if (bcs.dest() < bcs.next_bci()) {
>> 735:           return set_has_loops();
> 
> I don't understand the new return logic here. The break gets us out of the switch but we are still in the while loop, but the return takes us all the way out. ???

Yes, I had a version of this change where we only let has_loops get set once, and this code set it over and over again, which is unnecessary. Once it's true, it stays true.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13654#discussion_r1179049959
PR Review Comment: https://git.openjdk.org/jdk/pull/13654#discussion_r1179044448


More information about the serviceability-dev mailing list