On Fri, 17 Mar 2023 15:58:35 GMT, Frederic Parain <[email protected]> wrote:
>> Please review this change re-implementing the FieldInfo data structure. >> >> The FieldInfo array is an old data structure storing fields metadata. It has >> poor extension capabilities, a complex management code because of lack of >> strong typing and semantic overloading, and a poor memory efficiency. >> >> The new implementation uses a compressed stream to store those metadata, >> achieving better memory density and providing flexible extensibility, while >> exposing a strongly typed set of data when uncompressed. The stream is >> compressed using the unsigned5 encoding, which alreay present in the JDK >> (because of pack200) and the JVM (because JIT compulers use it to comrpess >> debugging information). >> >> More technical details are available in the CR: >> https://bugs.openjdk.org/browse/JDK-8292818 >> >> Those changes include a re-organisation of fields' flags, splitting the >> previous heterogeneous AccessFlags field into three distincts flag >> categories: immutable flags from the class file, immutable fields defined by >> the JVM, and finally mutable flags defined by the JVM. >> >> The SA, CI, and JVMCI, which all used to access the old FieldInfo array, >> have been updated too to deal with the new FieldInfo format. >> >> Tested with mach5, tier 1 to 7. >> >> Thank you. > > Frederic Parain has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains nine commits: > > - Style fixes > - Merge remote-tracking branch 'upstream/master' into fieldinfo_unsigned5 > - Style fixes > - SA and JVMCI fixes > - Fixes includes and style > - SA additional caching from Chris Plummer > - Addressing comments from first reviews > - Merge remote-tracking branch 'upstream/master' into fieldinfo_unsigned5 > - Reimplementation of FieldInfo as an unsigned5 stream One side-effect of this change is that verifying certain old classes (think JCK) can take a long time. For example: Test.java: import java.lang.classfile.ClassFile; import java.lang.constant.ClassDesc; import static java.lang.constant.ConstantDescs.CD_int; public class Test { public static void main(String[] args) throws Exception { int numberOfFields = 65_000; ClassDesc classDesc = ClassDesc.of("ManyFieldsClass"); byte[] classfile = ClassFile.of().build(classDesc, classBuilder -> { int flags = ClassFile.ACC_STATIC | ClassFile.ACC_PUBLIC; classBuilder.withVersion(ClassFile.JAVA_5_VERSION, 0); classBuilder.withField("myField0", CD_int, flags); for (int i = 1; i < numberOfFields; i++) { classBuilder.withField("myField" + i, CD_int, flags); } }); ClassLoader cl = new ClassLoader() { @Override protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException { if (name.equals("ManyFieldsClass")) { return defineClass(name, classfile, 0, classfile.length); } return super.loadClass(name, resolve); } }; long start = System.currentTimeMillis(); var c = Class.forName("ManyFieldsClass", true, cl); System.out.printf("took %,d ms to load %s%n", (System.currentTimeMillis() - start), c); } } > java Test.java took 19,854 ms to load class ManyFieldsClass Probably not worth worrying about - just wanted to point out something I hit while processing the JCK with Native Image. ------------- PR Comment: https://git.openjdk.org/jdk/pull/12855#issuecomment-3274222505
