Skip to content

Commit 8d6cfba

Browse files
liachliach
committed
8336267: Method and Constructor signature parsing can be shared on the root object
Reviewed-by: mchung
1 parent 1f7d524 commit 8d6cfba

File tree

3 files changed

+57
-72
lines changed

3 files changed

+57
-72
lines changed

‎src/java.base/share/classes/java/lang/reflect/Constructor.java

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,6 @@ public final class Constructor<T> extends Executable {
7272
private final int modifiers;
7373
// Generics and annotations support
7474
private final transient String signature;
75-
// generic info repository; lazily initialized
76-
private transient volatile ConstructorRepository genericInfo;
7775
private final byte[] annotations;
7876
private final byte[] parameterAnnotations;
7977

@@ -88,26 +86,27 @@ private GenericsFactory getFactory() {
8886
@Override
8987
ConstructorRepository getGenericInfo() {
9088
var genericInfo = this.genericInfo;
91-
// lazily initialize repository if necessary
9289
if (genericInfo == null) {
93-
// create and cache generic info repository
94-
genericInfo =
95-
ConstructorRepository.make(getSignature(),
96-
getFactory());
90+
var root = this.root;
91+
if (root != null) {
92+
genericInfo = root.getGenericInfo();
93+
} else {
94+
genericInfo = ConstructorRepository.make(getSignature(), getFactory());
95+
}
9796
this.genericInfo = genericInfo;
9897
}
99-
return genericInfo; //return cached repository
98+
return genericInfo;
10099
}
101100

102-
@Stable
103-
private ConstructorAccessor constructorAccessor;
104-
// For sharing of ConstructorAccessors. This branching structure
105-
// is currently only two levels deep (i.e., one root Constructor
106-
// and potentially many Constructor objects pointing to it.)
107-
//
108-
// If this branching structure would ever contain cycles, deadlocks can
109-
// occur in annotation code.
110-
private Constructor<T> root;
101+
/**
102+
* Constructors are mutable due to {@link AccessibleObject#setAccessible(boolean)}.
103+
* Thus, we return a new copy of a root each time a constructor is returned.
104+
* Some lazily initialized immutable states can be stored on root and shared to the copies.
105+
*/
106+
private Constructor<T> root;
107+
private transient volatile ConstructorRepository genericInfo;
108+
private @Stable ConstructorAccessor constructorAccessor;
109+
// End shared states
111110

112111
@Override
113112
Constructor<T> getRoot() {
@@ -143,13 +142,6 @@ Constructor<T> getRoot() {
143142
* "root" field points to this Constructor.
144143
*/
145144
Constructor<T> copy() {
146-
// This routine enables sharing of ConstructorAccessor objects
147-
// among Constructor objects which refer to the same underlying
148-
// method in the VM. (All of this contortion is only necessary
149-
// because of the "accessibility" bit in AccessibleObject,
150-
// which implicitly requires that new java.lang.reflect
151-
// objects be fabricated for each reflective call on Class
152-
// objects.)
153145
if (this.root != null)
154146
throw new IllegalArgumentException("Can not copy a non-root Constructor");
155147

@@ -162,6 +154,7 @@ Constructor<T> copy() {
162154
res.root = this;
163155
// Might as well eagerly propagate this if already present
164156
res.constructorAccessor = constructorAccessor;
157+
res.genericInfo = genericInfo;
165158
return res;
166159
}
167160

‎src/java.base/share/classes/java/lang/reflect/Field.java

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 1996, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 1996, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -76,22 +76,18 @@ class Field extends AccessibleObject implements Member {
7676
private final boolean trustedFinal;
7777
// Generics and annotations support
7878
private final transient String signature;
79-
// generic info repository; lazily initialized
80-
private transient volatile FieldRepository genericInfo;
8179
private final byte[] annotations;
82-
// Cached field accessor created without override
83-
@Stable
84-
private FieldAccessor fieldAccessor;
85-
// Cached field accessor created with override
86-
@Stable
87-
private FieldAccessor overrideFieldAccessor;
88-
// For sharing of FieldAccessors. This branching structure is
89-
// currently only two levels deep (i.e., one root Field and
90-
// potentially many Field objects pointing to it.)
91-
//
92-
// If this branching structure would ever contain cycles, deadlocks can
93-
// occur in annotation code.
94-
private Field root;
80+
81+
/**
82+
* Fields are mutable due to {@link AccessibleObject#setAccessible(boolean)}.
83+
* Thus, we return a new copy of a root each time a field is returned.
84+
* Some lazily initialized immutable states can be stored on root and shared to the copies.
85+
*/
86+
private Field root;
87+
private transient volatile FieldRepository genericInfo;
88+
private @Stable FieldAccessor fieldAccessor; // access control enabled
89+
private @Stable FieldAccessor overrideFieldAccessor; // access control suppressed
90+
// End shared states
9591

9692
// Generics infrastructure
9793

@@ -107,17 +103,18 @@ private GenericsFactory getFactory() {
107103
// Accessor for generic info repository
108104
private FieldRepository getGenericInfo() {
109105
var genericInfo = this.genericInfo;
110-
// lazily initialize repository if necessary
111106
if (genericInfo == null) {
112-
// create and cache generic info repository
113-
genericInfo = FieldRepository.make(getGenericSignature(),
114-
getFactory());
107+
var root = this.root;
108+
if (root != null) {
109+
genericInfo = root.getGenericInfo();
110+
} else {
111+
genericInfo = FieldRepository.make(getGenericSignature(), getFactory());
112+
}
115113
this.genericInfo = genericInfo;
116114
}
117-
return genericInfo; //return cached repository
115+
return genericInfo;
118116
}
119117

120-
121118
/**
122119
* Package-private constructor
123120
*/
@@ -162,6 +159,7 @@ Field copy() {
162159
// Might as well eagerly propagate this if already present
163160
res.fieldAccessor = fieldAccessor;
164161
res.overrideFieldAccessor = overrideFieldAccessor;
162+
res.genericInfo = genericInfo;
165163

166164
return res;
167165
}

‎src/java.base/share/classes/java/lang/reflect/Method.java

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -81,22 +81,20 @@ public final class Method extends Executable {
8181
private final int modifiers;
8282
// Generics and annotations support
8383
private final transient String signature;
84-
// generic info repository; lazily initialized
85-
private transient volatile MethodRepository genericInfo;
8684
private final byte[] annotations;
8785
private final byte[] parameterAnnotations;
8886
private final byte[] annotationDefault;
89-
@Stable
90-
private MethodAccessor methodAccessor;
91-
// For sharing of MethodAccessors. This branching structure is
92-
// currently only two levels deep (i.e., one root Method and
93-
// potentially many Method objects pointing to it.)
94-
//
95-
// If this branching structure would ever contain cycles, deadlocks can
96-
// occur in annotation code.
97-
private Method root;
98-
// Hash code of this object
99-
private int hash;
87+
88+
/**
89+
* Methods are mutable due to {@link AccessibleObject#setAccessible(boolean)}.
90+
* Thus, we return a new copy of a root each time a method is returned.
91+
* Some lazily initialized immutable states can be stored on root and shared to the copies.
92+
*/
93+
private Method root;
94+
private transient volatile MethodRepository genericInfo;
95+
private @Stable MethodAccessor methodAccessor;
96+
// End shared states
97+
private int hash; // not shared right now, eligible if expensive
10098

10199
// Generics infrastructure
102100
private String getGenericSignature() {return signature;}
@@ -111,14 +109,16 @@ private GenericsFactory getFactory() {
111109
@Override
112110
MethodRepository getGenericInfo() {
113111
var genericInfo = this.genericInfo;
114-
// lazily initialize repository if necessary
115112
if (genericInfo == null) {
116-
// create and cache generic info repository
117-
genericInfo = MethodRepository.make(getGenericSignature(),
118-
getFactory());
113+
var root = this.root;
114+
if (root != null) {
115+
genericInfo = root.getGenericInfo();
116+
} else {
117+
genericInfo = MethodRepository.make(getGenericSignature(), getFactory());
118+
}
119119
this.genericInfo = genericInfo;
120120
}
121-
return genericInfo; //return cached repository
121+
return genericInfo;
122122
}
123123

124124
/**
@@ -154,22 +154,16 @@ MethodRepository getGenericInfo() {
154154
* "root" field points to this Method.
155155
*/
156156
Method copy() {
157-
// This routine enables sharing of MethodAccessor objects
158-
// among Method objects which refer to the same underlying
159-
// method in the VM. (All of this contortion is only necessary
160-
// because of the "accessibility" bit in AccessibleObject,
161-
// which implicitly requires that new java.lang.reflect
162-
// objects be fabricated for each reflective call on Class
163-
// objects.)
164157
if (this.root != null)
165158
throw new IllegalArgumentException("Can not copy a non-root Method");
166159

167160
Method res = new Method(clazz, name, parameterTypes, returnType,
168161
exceptionTypes, modifiers, slot, signature,
169162
annotations, parameterAnnotations, annotationDefault);
170163
res.root = this;
171-
// Might as well eagerly propagate this if already present
164+
// Propagate shared states
172165
res.methodAccessor = methodAccessor;
166+
res.genericInfo = genericInfo;
173167
return res;
174168
}
175169

0 commit comments

Comments
 (0)