On Wed, 21 Aug 2024 16:03:15 GMT, Chen Liang <li...@openjdk.org> wrote:
>> A straightforward optimization, to share the signature parsing of method, >> constructor, and field between the root and the copied objects, like how >> method handle accessors are shared. > > Chen Liang has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains two commits: > > - Merge branch 'master' of https://github.com/openjdk/jdk into > feature/member-sig-share > - 8336267: Method and Constructor signature parsing can be shared on the > root object It's okay to extend the root object to share `genericInfo` besides the accessor. But we need to update the comment to make that clear. @Stable private ConstructorAccessor constructorAccessor; // For sharing of ConstructorAccessors. This branching structure // is currently only two levels deep (i.e., one root Constructor // and potentially many Constructor objects pointing to it.) // // If this branching structure would ever contain cycles, deadlocks can // occur in annotation code. private Constructor<T> root; The comment for `root` should be extended to cover the generics repository. In addition, I suggest to move the declaration of instance variable `genericInfo` closer to the accessor and `root`. src/java.base/share/classes/java/lang/reflect/Constructor.java line 100: > 98: genericInfo = > 99: ConstructorRepository.make(getSignature(), > 100: getFactory()); Suggestion: genericInfo = ConstructorRepository.make(getSignature(), getFactory()); Nit: This fits well in one line. src/java.base/share/classes/java/lang/reflect/Field.java line 118: > 116: // create and cache generic info repository > 117: genericInfo = FieldRepository.make(getGenericSignature(), > 118: getFactory()); Suggestion: genericInfo = FieldRepository.make(getGenericSignature(), getFactory()); src/java.base/share/classes/java/lang/reflect/Method.java line 122: > 120: // create and cache generic info repository > 121: genericInfo = > MethodRepository.make(getGenericSignature(), > 122: getFactory()); Suggestion: genericInfo = MethodRepository.make(getGenericSignature(), getFactory()); ------------- PR Review: https://git.openjdk.org/jdk/pull/20179#pullrequestreview-2403193407 PR Review Comment: https://git.openjdk.org/jdk/pull/20179#discussion_r1821607268 PR Review Comment: https://git.openjdk.org/jdk/pull/20179#discussion_r1821608006 PR Review Comment: https://git.openjdk.org/jdk/pull/20179#discussion_r1821608108
