On Thu, 1 Jun 2023 15:20:27 GMT, Adam Sotona <asot...@openjdk.org> wrote:
>> Classfile context object and multi-state options have been discussed at >> https://mail.openjdk.org/pipermail/classfile-api-dev/2023-May/000321.html >> This patch implements the proposed changes in Classfile API and fixes all >> affected code across JDK sources and tests. >> >> Please review. >> >> Thanks, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > fixed benchmarks In fact, I think we can probably mark Classfile as value-based. Then, whether it's created from a factory at every call or if it's cached (my first 2 review comments) no longer matters as much; the caching behavior will be handled by individual option instances (such as class hierarchy caching in the class hierarchy option's resolver) src/java.base/share/classes/java/lang/Module.java line 1593: > 1591: private Class<?> loadModuleInfoClass(InputStream in) throws > IOException { > 1592: final String MODULE_INFO = "module-info"; > 1593: var cc = > Classfile.of(Classfile.ConstantPoolSharingOption.DO_NOT_SHARE_CONSTANT_POOL); This `cc` can be stored in a static final field instead. src/java.base/share/classes/jdk/internal/classfile/Classfile.java line 55: > 53: > 54: static Classfile of() { > 55: return new ClassfileImpl(); We can create a static final field in `ClassfileImpl` holding a default instance equivalent to that created by `new ClassfileImpl()` and have the `of()` factory return that instance instead. src/java.base/share/classes/jdk/internal/classfile/impl/StackMapGenerator.java line 840: > 838: try { > 839: //clone SplitConstantPool with alternate context > 840: var cc = > Classfile.of(Classfile.StackMapsOption.DO_NOT_GENERATE_STACK_MAPS); This should set the Class hierarchy resolver, patch dead code, and filter dead labels options from the original context. ------------- PR Comment: https://git.openjdk.org/jdk/pull/14180#issuecomment-1572260124 PR Review Comment: https://git.openjdk.org/jdk/pull/14180#discussion_r1213300095 PR Review Comment: https://git.openjdk.org/jdk/pull/14180#discussion_r1213302665 PR Review Comment: https://git.openjdk.org/jdk/pull/14180#discussion_r1213314212
