Skip to content

Commit e4425a4

Browse files
dougxcdougxc
committed
[GR-71140] Adopt caching added to Annotated.getDeclaredAnnotationInfo
PullRequest: graal/22532
2 parents 16d66fb + 6f2a17a commit e4425a4

File tree

49 files changed

+477
-248
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+477
-248
lines changed

‎common.json‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,12 @@
4949
"graalvm-ee-25-ea": {"name": "graalvm-jdk", "version": "25.0.0", "ea": "36", "platformspecific": true },
5050

5151
"oraclejdk-latest": {"name": "jpg-jdk", "version": "25", "build_id": "jdk-25.0.1+8", "platformspecific": true, "extrabundles": ["static-libs"]},
52-
"labsjdk-ce-latest": {"name": "labsjdk", "version": "ce-25.0.1+8-jvmci-25.1-b08", "platformspecific": true },
53-
"labsjdk-ce-latestDebug": {"name": "labsjdk", "version": "ce-25.0.1+8-jvmci-25.1-b08-debug", "platformspecific": true },
54-
"labsjdk-ce-latest-llvm": {"name": "labsjdk", "version": "ce-25.0.1+8-jvmci-25.1-b08-sulong", "platformspecific": true },
55-
"labsjdk-ee-latest": {"name": "labsjdk", "version": "ee-25.0.1+8-jvmci-25.1-b08", "platformspecific": true },
56-
"labsjdk-ee-latestDebug": {"name": "labsjdk", "version": "ee-25.0.1+8-jvmci-25.1-b08-debug", "platformspecific": true },
57-
"labsjdk-ee-latest-llvm": {"name": "labsjdk", "version": "ee-25.0.1+8-jvmci-25.1-b08-sulong", "platformspecific": true }
52+
"labsjdk-ce-latest": {"name": "labsjdk", "version": "ce-25.0.1+8-jvmci-25.1-b09", "platformspecific": true },
53+
"labsjdk-ce-latestDebug": {"name": "labsjdk", "version": "ce-25.0.1+8-jvmci-25.1-b09-debug", "platformspecific": true },
54+
"labsjdk-ce-latest-llvm": {"name": "labsjdk", "version": "ce-25.0.1+8-jvmci-25.1-b09-sulong", "platformspecific": true },
55+
"labsjdk-ee-latest": {"name": "labsjdk", "version": "ee-25.0.1+8-jvmci-25.1-b09", "platformspecific": true },
56+
"labsjdk-ee-latestDebug": {"name": "labsjdk", "version": "ee-25.0.1+8-jvmci-25.1-b09-debug", "platformspecific": true },
57+
"labsjdk-ee-latest-llvm": {"name": "labsjdk", "version": "ee-25.0.1+8-jvmci-25.1-b09-sulong", "platformspecific": true }
5858
},
5959

6060
"eclipse": {

‎compiler/mx.compiler/mx_compiler.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ def _ctw_jvmci_export_args(arg_prefix='--'):
252252
'add-exports=java.base/jdk.internal.module=ALL-UNNAMED',
253253
'add-exports=jdk.internal.vm.ci/jdk.vm.ci.hotspot=ALL-UNNAMED',
254254
'add-exports=jdk.internal.vm.ci/jdk.vm.ci.meta=ALL-UNNAMED',
255+
'add-exports=jdk.internal.vm.ci/jdk.vm.ci.meta.annotation=ALL-UNNAMED',
255256
'add-exports=jdk.internal.vm.ci/jdk.vm.ci.services=ALL-UNNAMED',
256257
'add-exports=jdk.internal.vm.ci/jdk.vm.ci.runtime=ALL-UNNAMED',
257258
'add-exports=jdk.graal.compiler/jdk.graal.compiler.hotspot=ALL-UNNAMED',

‎compiler/mx.compiler/mx_graal_benchmark.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,7 @@ def extraVmArgs(self):
414414
'--add-exports=jdk.internal.vm.ci/jdk.vm.ci.services=ALL-UNNAMED',
415415
'--add-exports=jdk.internal.vm.ci/jdk.vm.ci.runtime=ALL-UNNAMED',
416416
'--add-exports=jdk.internal.vm.ci/jdk.vm.ci.meta=ALL-UNNAMED',
417+
'--add-exports=jdk.internal.vm.ci/jdk.vm.ci.meta.annotation=ALL-UNNAMED',
417418
'--add-exports=jdk.internal.vm.ci/jdk.vm.ci.code=ALL-UNNAMED',
418419
'--add-exports=jdk.graal.compiler/jdk.graal.compiler.graph=ALL-UNNAMED',
419420
'--add-exports=org.graalvm.truffle/com.oracle.truffle.api.benchmark=ALL-UNNAMED',

‎compiler/mx.compiler/suite.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,7 @@
316316
"GRAAL_PROCESSOR",
317317
],
318318
"javaCompliance" : "21+",
319+
"spotbugs": "false",
319320
"workingSets" : "Graal,HotSpot",
320321
},
321322

@@ -330,6 +331,7 @@
330331
"requiresConcealed" : {
331332
"jdk.internal.vm.ci" : [
332333
"jdk.vm.ci.meta",
334+
"jdk.vm.ci.meta.annotation",
333335
"jdk.vm.ci.code"
334336
],
335337
},
@@ -400,6 +402,7 @@
400402
"checkstyle" : "jdk.graal.compiler",
401403
"javaCompliance" : "21+",
402404
"jacoco" : "exclude",
405+
"spotbugs": "false",
403406
"graalCompilerSourceEdition": "ignore",
404407
},
405408

‎compiler/src/jdk.graal.compiler.test/src/jdk/graal/compiler/hotspot/replaycomp/test/CompilationProxyTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public void checkProxyMethods() throws IllegalAccessException, InvocationTargetE
9191
assertTrue("protected method should throw UnsupportedOperationException", e.getTargetException() instanceof UnsupportedOperationException);
9292
continue;
9393
}
94-
Assert.fail("expected the protected method to throw");
94+
Assert.fail("expected the protected method to throw: " + method);
9595
}
9696

9797
Object returnValue = createValue(method.getReturnType());

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/annotation/AnnotationValue.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import java.util.Map;
3131
import java.util.Objects;
3232
import java.util.Set;
33+
import java.util.function.BiFunction;
3334

3435
import jdk.graal.compiler.util.CollectionsUtil;
3536
import jdk.vm.ci.meta.ResolvedJavaType;
@@ -375,4 +376,28 @@ public int hashCode() {
375376
}
376377
return type.hashCode() ^ elements.hashCode();
377378
}
379+
380+
/**
381+
* Result of last call to {@link #toAnnotation}.
382+
*/
383+
private volatile Annotation annotationCache;
384+
385+
/**
386+
* Converts this to an {@link Annotation} of type {@code annotationType}, utilizing the provided
387+
* converter function. The result is cached to improve performance by reducing redundant
388+
* conversions.
389+
*
390+
* @param <T> the type of the annotation to be created
391+
* @param annotationType the desired annotation type
392+
* @param converter a function that does the conversion
393+
*/
394+
@SuppressWarnings("unchecked")
395+
public <T extends Annotation> T toAnnotation(Class<T> annotationType, BiFunction<AnnotationValue, Class<T>, T> converter) {
396+
Annotation res = annotationCache;
397+
if (res == null || res.annotationType() != annotationType) {
398+
res = converter.apply(this, annotationType);
399+
annotationCache = res;
400+
}
401+
return (T) res;
402+
}
378403
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/annotation/AnnotationValueSupport.java‎

Lines changed: 34 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,13 @@
3232
import java.lang.reflect.Method;
3333
import java.lang.reflect.RecordComponent;
3434
import java.nio.ByteBuffer;
35-
import java.util.Collections;
3635
import java.util.List;
3736
import java.util.Map;
38-
import java.util.concurrent.ConcurrentHashMap;
37+
import java.util.function.Function;
3938

4039
import jdk.graal.compiler.core.common.LibGraalSupport;
4140
import jdk.graal.compiler.debug.GraalError;
41+
import jdk.graal.compiler.util.CollectionsUtil;
4242
import jdk.vm.ci.meta.ResolvedJavaMethod;
4343
import jdk.vm.ci.meta.ResolvedJavaType;
4444
import jdk.vm.ci.meta.UnresolvedJavaType;
@@ -77,12 +77,26 @@ public static AnnotationValue getDeclaredAnnotationValue(ResolvedJavaType annota
7777
* present on this element
7878
*/
7979
public static Map<ResolvedJavaType, AnnotationValue> getDeclaredAnnotationValues(Annotated annotated) {
80-
AnnotationsInfo info = annotated.getDeclaredAnnotationInfo();
80+
return annotated.getDeclaredAnnotationInfo(ANNOTATIONS_INFO_PARSER).values;
81+
}
82+
83+
/**
84+
* Result type returned by {@link AnnotationValueSupport#ANNOTATIONS_INFO_PARSER}.
85+
*
86+
* @param values the parsed annotations
87+
* @param container used to resolve type names in the annotations
88+
*/
89+
public record ParsedDeclaredAnnotationValues(Map<ResolvedJavaType, AnnotationValue> values, ResolvedJavaType container) {
90+
public static ParsedDeclaredAnnotationValues NONE = new ParsedDeclaredAnnotationValues(CollectionsUtil.mapOf(), null);
91+
}
92+
93+
public static final Function<AnnotationsInfo, ParsedDeclaredAnnotationValues> ANNOTATIONS_INFO_PARSER = info -> {
8194
if (info == null) {
82-
return Collections.emptyMap();
95+
return ParsedDeclaredAnnotationValues.NONE;
8396
}
84-
return AnnotationValueParser.parseAnnotations(info.bytes(), info.constPool(), info.container());
85-
}
97+
ResolvedJavaType container = info.container();
98+
return new ParsedDeclaredAnnotationValues(AnnotationValueParser.parseAnnotations(info.bytes(), info.constPool(), container), container);
99+
};
86100

87101
/**
88102
* Gets the type annotations for {@code annotated} that back the implementation of
@@ -122,9 +136,9 @@ public static List<List<AnnotationValue>> getParameterAnnotationValues(ResolvedJ
122136
* null if no default is associated with {@code method}, or if {@code method} does not represent
123137
* a declared member of an annotation type.
124138
*
125-
* @see Method#getDefaultValue()
126139
* @return the default value for the annotation member represented by this object. The type of
127140
* the returned value is specified by {@link AnnotationValue#get}
141+
* @see Method#getDefaultValue()
128142
*/
129143
public static Object getAnnotationDefaultValue(ResolvedJavaMethod method) {
130144
AnnotationsInfo info = method.getAnnotationDefaultInfo();
@@ -150,47 +164,14 @@ public static AnnotationValue getAnnotationValue(Annotated annotated, Class<? ex
150164
return getAnnotationValue0(annotated, annotationType, inherited);
151165
}
152166

153-
/**
154-
* Cache for {@link #getAnnotationValue}. Building libgraal-ee shows that this cache grows to
155-
* about 3K entries so the LRU cache is sized just above that (4096). This cache must not grow
156-
* too large as there are Native Image tests that build numerous images in the one JVM process.
157-
*/
158-
@LibGraalSupport.HostedOnly //
159-
private static final Map<Annotated, Map<ResolvedJavaType, AnnotationValue>> declaredAnnotations;
160-
static {
161-
final int cacheMaxSize = 4096;
162-
if (LibGraalSupport.INSTANCE == null) {
163-
declaredAnnotations = Collections.synchronizedMap(new java.util.LinkedHashMap<>(cacheMaxSize, 0.75f, true) {
164-
@Override
165-
protected boolean removeEldestEntry(Map.Entry<Annotated, Map<ResolvedJavaType, AnnotationValue>> eldest) {
166-
return size() > cacheMaxSize;
167-
}
168-
});
169-
} else {
170-
declaredAnnotations = null;
171-
}
172-
}
173-
174167
@LibGraalSupport.HostedOnly
175168
private static AnnotationValue getAnnotationValue0(Annotated annotated, Class<? extends Annotation> annotationType, boolean inherited) {
176-
AnnotationsInfo info = annotated.getDeclaredAnnotationInfo();
177-
if (info == null && !inherited) {
169+
ParsedDeclaredAnnotationValues parsed = annotated.getDeclaredAnnotationInfo(ANNOTATIONS_INFO_PARSER);
170+
if (parsed.values == null && !inherited) {
178171
return null;
179172
}
180-
Map<ResolvedJavaType, AnnotationValue> map = declaredAnnotations.get(annotated);
181-
if (map == null) {
182-
/*
183-
* Do not use Map#computeIfAbsent as Collections.SynchronizedMap#computeIfAbsent blocks
184-
* readers during the creation of the cached value.
185-
*/
186-
map = AnnotationValueParser.parseAnnotations(info.bytes(), info.constPool(), info.container());
187-
var existing = declaredAnnotations.putIfAbsent(annotated, map);
188-
if (existing != null) {
189-
map = existing;
190-
}
191-
}
192173

193-
AnnotationValue res = lookup(annotationType, map, info);
174+
AnnotationValue res = lookup(annotationType, parsed.values, parsed.container);
194175
if (res != null) {
195176
return res;
196177
}
@@ -201,19 +182,22 @@ private static AnnotationValue getAnnotationValue0(Annotated annotated, Class<?
201182
return null;
202183
}
203184

204-
@LibGraalSupport.HostedOnly //
205-
private static final Map<Class<? extends Annotation>, ResolvedJavaType> resolvedAnnotationTypeCache = LibGraalSupport.INSTANCE != null ? null
206-
: new ConcurrentHashMap<>();
207-
185+
/**
186+
* Looks up the annotation value of type {@code annotationType} in {@code map}.
187+
*
188+
* @param annotationType the type of annotation to look up
189+
* @param map the map of annotation values to search
190+
* @param container used to resolve the annotation type
191+
* @return the annotation value if found, or null if not found
192+
*/
208193
@LibGraalSupport.HostedOnly
209-
private static AnnotationValue lookup(Class<? extends Annotation> annotationType, Map<ResolvedJavaType, AnnotationValue> map, AnnotationsInfo info) {
194+
private static AnnotationValue lookup(Class<? extends Annotation> annotationType, Map<ResolvedJavaType, AnnotationValue> map, ResolvedJavaType container) {
210195
String internalName = "L" + annotationType.getName().replace(".", "/") + ";";
211196
for (var e : map.entrySet()) {
212197
ResolvedJavaType type = e.getKey();
213198
if (type.getName().equals(internalName)) {
214199
// The name matches so now double-check the resolved type matches
215-
ResolvedJavaType resolved = resolvedAnnotationTypeCache.computeIfAbsent(annotationType, a -> UnresolvedJavaType.create(internalName).resolve(info.container()));
216-
if (resolved.equals(type)) {
200+
if (UnresolvedJavaType.create(internalName).resolve(container).equals(type)) {
217201
return e.getValue();
218202
}
219203
}

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/JVMCIVersionCheck.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ public final class JVMCIVersionCheck {
5858
// Checkstyle: stop stable iteration order check
5959
private static final Map<String, Map<String, Version>> JVMCI_MIN_VERSIONS = Map.of(
6060
"25", Map.of(
61-
"Oracle Corporation", createLabsJDKVersion("25.0.1+8", "25.1", 8),
62-
DEFAULT_VENDOR_ENTRY, createLabsJDKVersion("25.0.1+8", "25.1", 8)));
61+
"Oracle Corporation", createLabsJDKVersion("25.0.1+8", "25.1", 9),
62+
DEFAULT_VENDOR_ENTRY, createLabsJDKVersion("25.0.1+8", "25.1", 9)));
6363
// Checkstyle: resume stable iteration order check
6464

6565
private static final int NA = 0;

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/SnippetResolvedJavaMethod.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.lang.annotation.Annotation;
2828
import java.lang.reflect.Type;
2929
import java.util.Objects;
30+
import java.util.function.Function;
3031

3132
import jdk.vm.ci.meta.Constant;
3233
import jdk.vm.ci.meta.ConstantPool;
@@ -38,6 +39,7 @@
3839
import jdk.vm.ci.meta.ResolvedJavaType;
3940
import jdk.vm.ci.meta.Signature;
4041
import jdk.vm.ci.meta.SpeculationLog;
42+
import jdk.vm.ci.meta.annotation.AnnotationsInfo;
4143

4244
/**
4345
* A minimal implementation of {@link ResolvedJavaMethod} for use by libgraal.
@@ -225,6 +227,16 @@ public Annotation[] getDeclaredAnnotations() {
225227
throw new UnsupportedOperationException();
226228
}
227229

230+
@Override
231+
public <T> T getDeclaredAnnotationInfo(Function<AnnotationsInfo, T> parser) {
232+
throw new UnsupportedOperationException();
233+
}
234+
235+
@Override
236+
public AnnotationsInfo getTypeAnnotationInfo() {
237+
throw new UnsupportedOperationException();
238+
}
239+
228240
@Override
229241
public int getModifiers() {
230242
return modifiers;

‎compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/hotspot/SnippetResolvedJavaType.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.Arrays;
2929
import java.util.List;
3030
import java.util.Objects;
31+
import java.util.function.Function;
3132

3233
import jdk.graal.compiler.core.common.LibGraalSupport;
3334
import jdk.graal.compiler.debug.GraalError;
@@ -41,6 +42,7 @@
4142
import jdk.vm.ci.meta.ResolvedJavaRecordComponent;
4243
import jdk.vm.ci.meta.ResolvedJavaType;
4344
import jdk.vm.ci.meta.UnresolvedJavaType;
45+
import jdk.vm.ci.meta.annotation.AnnotationsInfo;
4446

4547
/**
4648
* A minimal implementation of {@link ResolvedJavaType} for use by libgraal.
@@ -381,6 +383,16 @@ public Annotation[] getDeclaredAnnotations() {
381383
throw new UnsupportedOperationException();
382384
}
383385

386+
@Override
387+
public <T> T getDeclaredAnnotationInfo(Function<AnnotationsInfo, T> parser) {
388+
throw new UnsupportedOperationException();
389+
}
390+
391+
@Override
392+
public AnnotationsInfo getTypeAnnotationInfo() {
393+
throw new UnsupportedOperationException();
394+
}
395+
384396
@Override
385397
public boolean isArray() {
386398
return javaClass.isArray();

0 commit comments

Comments
 (0)