On Mon, 9 Dec 2024 19:46:43 GMT, Chen Liang <li...@openjdk.org> wrote:
>> The Class.getModifiers() method is implemented as a native method in >> java.lang.Class to access a field that we've calculated when creating the >> mirror. The field is final after that point. The VM doesn't need it >> anymore, so there's no real need for the jdk code to call into the VM to get >> it. This moves the field to Java and removes the intrinsic code. I >> promoted the compute_modifiers() functions to return int since that's how >> java.lang.Class uses the value. It should really be an unsigned short >> though. >> >> There's a couple of JMH benchmarks added with this change. One does show >> that for array classes for non-bootstrap class loader, this results in one >> extra load which in a long loop of just that, is observable. I don't think >> this is real life code. The other benchmarks added show no regression. >> >> Tested with tier1-8. > > src/hotspot/share/classfile/javaClasses.cpp line 1504: > >> 1502: macro(_reflectionData_offset, k, "reflectionData", >> java_lang_ref_SoftReference_signature, false); \ >> 1503: macro(_signers_offset, k, "signers", >> object_array_signature, false); \ >> 1504: macro(_modifiers_offset, k, vmSymbols::modifiers_name(), >> int_signature, false) > > Do we need a trailing semicolon here? yes. it is needed. > src/java.base/share/classes/java/lang/Class.java line 1315: > >> 1313: >> 1314: // Set by the JVM when creating the instance of this >> java.lang.Class >> 1315: private transient int modifiers; > > If this is set by the JVM, can this be marked `final` so JIT compiler can > trust this field? Also preferable if we can move this together with > components/signers/classData fields. The JVM rearranges these fields so that's why I put it near the caller. Let me check if final compiles. Edit: it looks better with the other fields though. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22652#discussion_r1876712191 PR Review Comment: https://git.openjdk.org/jdk/pull/22652#discussion_r1876713323