Skip to content

Commit 80c2051

Browse files
loicottetloicottet
committed
[GR-54550] Pool reachability handlers for reflection registrations
PullRequest: graal/17958
2 parents 7a64ba9 + fcaf28b commit 80c2051

File tree

3 files changed

+93
-41
lines changed

3 files changed

+93
-41
lines changed

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageCodeCache.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -379,32 +379,34 @@ protected void buildRuntimeMetadata(DebugContext debug, SnippetReflectionProvide
379379

380380
Set<AnalysisField> includedFields = new HashSet<>();
381381
Set<AnalysisMethod> includedMethods = new HashSet<>();
382-
Map<AnalysisField, ConditionalRuntimeValue<Field>> configurationFields = reflectionSupport.getReflectionFields();
383-
Map<AnalysisMethod, ConditionalRuntimeValue<Executable>> configurationExecutables = reflectionSupport.getReflectionExecutables();
382+
Map<AnalysisType, Map<AnalysisField, ConditionalRuntimeValue<Field>>> configurationFields = reflectionSupport.getReflectionFields();
383+
Map<AnalysisType, Map<AnalysisMethod, ConditionalRuntimeValue<Executable>>> configurationExecutables = reflectionSupport.getReflectionExecutables();
384384

385385
reflectionSupport.getHeapReflectionFields().forEach(((analysisField, reflectField) -> {
386386
if (includedFields.add(analysisField)) {
387387
HostedField hostedField = hUniverse.lookup(analysisField);
388-
runtimeMetadataEncoder.addHeapAccessibleObjectMetadata(hMetaAccess, hostedField, reflectField, configurationFields.containsKey(analysisField));
388+
runtimeMetadataEncoder.addHeapAccessibleObjectMetadata(hMetaAccess, hostedField, reflectField,
389+
configurationFields.getOrDefault(analysisField.getDeclaringClass(), Collections.emptyMap()).containsKey(analysisField));
389390
}
390391
}));
391392

392393
reflectionSupport.getHeapReflectionExecutables().forEach(((analysisMethod, reflectMethod) -> {
393394
if (includedMethods.add(analysisMethod)) {
394395
HostedMethod hostedMethod = hUniverse.lookup(analysisMethod);
395-
runtimeMetadataEncoder.addHeapAccessibleObjectMetadata(hMetaAccess, hostedMethod, reflectMethod, configurationExecutables.containsKey(analysisMethod));
396+
runtimeMetadataEncoder.addHeapAccessibleObjectMetadata(hMetaAccess, hostedMethod, reflectMethod,
397+
configurationExecutables.getOrDefault(analysisMethod.getDeclaringClass(), Collections.emptyMap()).containsKey(analysisMethod));
396398
}
397399
}));
398400

399-
configurationFields.forEach(((analysisField, reflectField) -> {
401+
configurationFields.forEach((declaringClass, classFields) -> classFields.forEach((analysisField, reflectField) -> {
400402
if (includedFields.add(analysisField)) {
401403
HostedField hostedField = hUniverse.lookup(analysisField);
402404
runtimeMetadataEncoder.addReflectionFieldMetadata(hMetaAccess, hostedField, reflectField);
403405
}
404406
}));
405407

406408
var symbolTracker = PriorLayerSymbolTracker.singletonOrNull();
407-
configurationExecutables.forEach(((analysisMethod, reflectMethod) -> {
409+
configurationExecutables.forEach((declaringClass, classMethods) -> classMethods.forEach((analysisMethod, reflectMethod) -> {
408410
if (includedMethods.add(analysisMethod)) {
409411
HostedMethod method = hUniverse.lookup(analysisMethod);
410412
if (analysisMethod.isInBaseLayer()) {

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionDataBuilder.java

Lines changed: 83 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import java.util.Arrays;
5757
import java.util.Collection;
5858
import java.util.Collections;
59+
import java.util.HashSet;
5960
import java.util.List;
6061
import java.util.Map;
6162
import java.util.Objects;
@@ -100,7 +101,6 @@
100101

101102
import jdk.vm.ci.meta.ResolvedJavaField;
102103
import jdk.vm.ci.meta.ResolvedJavaMethod;
103-
import jdk.vm.ci.meta.Signature;
104104
import sun.reflect.annotation.ExceptionProxy;
105105

106106
public class ReflectionDataBuilder extends ConditionalConfigurationRegistry implements RuntimeReflectionSupport, ReflectionHostedSupport {
@@ -121,9 +121,9 @@ public class ReflectionDataBuilder extends ConditionalConfigurationRegistry impl
121121
*/
122122
private final Map<Class<?>, Set<Class<?>>> innerClasses = new ConcurrentHashMap<>();
123123
private final Map<Class<?>, Integer> enabledQueriesFlags = new ConcurrentHashMap<>();
124-
private final Map<AnalysisField, ConditionalRuntimeValue<Field>> registeredFields = new ConcurrentHashMap<>();
124+
private final Map<AnalysisType, Map<AnalysisField, ConditionalRuntimeValue<Field>>> registeredFields = new ConcurrentHashMap<>();
125125
private final Set<AnalysisField> hidingFields = ConcurrentHashMap.newKeySet();
126-
private final Map<AnalysisMethod, ConditionalRuntimeValue<Executable>> registeredMethods = new ConcurrentHashMap<>();
126+
private final Map<AnalysisType, Map<AnalysisMethod, ConditionalRuntimeValue<Executable>>> registeredMethods = new ConcurrentHashMap<>();
127127
private final Map<AnalysisMethod, Object> methodAccessors = new ConcurrentHashMap<>();
128128
private final Set<AnalysisMethod> hidingMethods = ConcurrentHashMap.newKeySet();
129129

@@ -193,6 +193,10 @@ private void setQueryFlag(Class<?> clazz, int flag) {
193193
enabledQueriesFlags.compute(clazz, (key, oldValue) -> (oldValue == null) ? flag : (oldValue | flag));
194194
}
195195

196+
private boolean isQueryFlagSet(Class<?> clazz, int flag) {
197+
return (enabledQueriesFlags.getOrDefault(clazz, 0) & flag) != 0;
198+
}
199+
196200
@Override
197201
public void register(ConfigurationCondition condition, boolean unsafeInstantiated, Class<?> clazz) {
198202
Objects.requireNonNull(clazz, () -> nullErrorMessage("class"));
@@ -419,11 +423,15 @@ private void registerMethod(ConfigurationCondition cnd, boolean queriedOnly, Exe
419423
}
420424

421425
AnalysisMethod analysisMethod = metaAccess.lookupJavaMethod(reflectExecutable);
426+
AnalysisType declaringType = analysisMethod.getDeclaringClass();
427+
var classMethods = registeredMethods.computeIfAbsent(declaringType, t -> new ConcurrentHashMap<>());
428+
var shouldRegisterReachabilityHandler = classMethods.isEmpty();
429+
422430
boolean registered = false;
423-
ConditionalRuntimeValue<Executable> conditionalValue = registeredMethods.get(analysisMethod);
431+
ConditionalRuntimeValue<Executable> conditionalValue = classMethods.get(analysisMethod);
424432
if (conditionalValue == null) {
425433
var newConditionalValue = new ConditionalRuntimeValue<>(RuntimeConditionSet.emptySet(), reflectExecutable);
426-
conditionalValue = registeredMethods.putIfAbsent(analysisMethod, newConditionalValue);
434+
conditionalValue = classMethods.putIfAbsent(analysisMethod, newConditionalValue);
427435
if (conditionalValue == null) {
428436
conditionalValue = newConditionalValue;
429437
registered = true;
@@ -436,15 +444,26 @@ private void registerMethod(ConfigurationCondition cnd, boolean queriedOnly, Exe
436444

437445
if (registered) {
438446
registerTypesForMethod(analysisMethod, reflectExecutable);
439-
AnalysisType declaringType = analysisMethod.getDeclaringClass();
440447
Class<?> declaringClass = declaringType.getJavaClass();
441448

442449
/*
443450
* The image needs to know about subtypes shadowing methods registered for reflection to
444451
* ensure the correctness of run-time reflection queries.
445452
*/
446-
analysisAccess.registerSubtypeReachabilityHandler(
447-
(access, subType) -> universe.getBigbang().postTask(debug -> checkSubtypeForOverridingMethod(analysisMethod, metaAccess.lookupJavaType(subType))), declaringClass);
453+
if (shouldRegisterReachabilityHandler) {
454+
analysisAccess.registerSubtypeReachabilityHandler(
455+
(access, subType) -> universe.getBigbang()
456+
.postTask(debug -> checkSubtypeForOverridingMethods(metaAccess.lookupJavaType(subType), registeredMethods.get(declaringType).keySet())),
457+
declaringClass);
458+
} else {
459+
/*
460+
* We need to perform the check for already reachable subtypes since the
461+
* reachability handler was already called for them.
462+
*/
463+
for (AnalysisType subtype : AnalysisUniverse.reachableSubtypes(declaringType)) {
464+
universe.getBigbang().postTask(debug -> checkSubtypeForOverridingMethods(subtype, Collections.singleton(analysisMethod)));
465+
}
466+
}
448467

449468
if (declaringType.isAnnotation() && !analysisMethod.isConstructor()) {
450469
processAnnotationMethod(queriedOnly, (Method) reflectExecutable);
@@ -552,30 +571,45 @@ private void registerField(ConfigurationCondition cnd, boolean queriedOnly, Fiel
552571
}
553572

554573
AnalysisField analysisField = metaAccess.lookupJavaField(reflectField);
574+
AnalysisType declaringClass = analysisField.getDeclaringClass();
555575

556-
if (!registeredFields.containsKey(analysisField)) {
576+
var classFields = registeredFields.computeIfAbsent(declaringClass, t -> new ConcurrentHashMap<>());
577+
boolean exists = classFields.containsKey(analysisField);
578+
boolean shouldRegisterReachabilityHandler = classFields.isEmpty();
579+
var cndValue = classFields.computeIfAbsent(analysisField, f -> new ConditionalRuntimeValue<>(RuntimeConditionSet.emptySet(), reflectField));
580+
if (!queriedOnly) {
581+
/* queryOnly methods are conditioned by the type itself */
582+
cndValue.getConditions().addCondition(cnd);
583+
}
584+
585+
if (!exists) {
557586
registerTypesForField(analysisField, reflectField, true);
558-
AnalysisType declaringClass = analysisField.getDeclaringClass();
559587

560588
/*
561589
* The image needs to know about subtypes shadowing fields registered for reflection to
562590
* ensure the correctness of run-time reflection queries.
563591
*/
564-
analysisAccess.registerSubtypeReachabilityHandler(
565-
(access, subType) -> universe.getBigbang().postTask(debug -> checkSubtypeForOverridingField(analysisField, metaAccess.lookupJavaType(subType))),
566-
declaringClass.getJavaClass());
592+
if (shouldRegisterReachabilityHandler) {
593+
analysisAccess.registerSubtypeReachabilityHandler(
594+
(access, subType) -> universe.getBigbang()
595+
.postTask(debug -> checkSubtypeForOverridingFields(metaAccess.lookupJavaType(subType),
596+
registeredFields.get(declaringClass).keySet())),
597+
declaringClass.getJavaClass());
598+
} else {
599+
/*
600+
* We need to perform the check for already reachable subtypes since the
601+
* reachability handler was already called for them.
602+
*/
603+
for (AnalysisType subtype : AnalysisUniverse.reachableSubtypes(declaringClass)) {
604+
universe.getBigbang().postTask(debug -> checkSubtypeForOverridingFields(subtype, Collections.singleton(analysisField)));
605+
}
606+
}
567607

568608
if (declaringClass.isAnnotation()) {
569609
processAnnotationField(cnd, reflectField);
570610
}
571611
}
572612

573-
var cndValue = registeredFields.computeIfAbsent(analysisField, f -> new ConditionalRuntimeValue<>(RuntimeConditionSet.emptySet(), reflectField));
574-
if (!queriedOnly) {
575-
/* queryOnly methods are conditioned by the type itself */
576-
cndValue.getConditions().addCondition(cnd);
577-
}
578-
579613
/*
580614
* We need to run this even if the method has already been registered, in case it was only
581615
* registered as queried.
@@ -635,13 +669,21 @@ private void processAnnotationField(ConfigurationCondition cnd, Field field) {
635669
/**
636670
* @see ReflectionHostedSupport#getHidingReflectionFields()
637671
*/
638-
private void checkSubtypeForOverridingField(AnalysisField field, AnalysisType subtype) {
672+
private void checkSubtypeForOverridingFields(AnalysisType subtype, Collection<AnalysisField> superclassFields) {
673+
if (isQueryFlagSet(subtype.getJavaClass(), ALL_DECLARED_FIELDS_FLAG)) {
674+
/* All fields are already registered, no need for hiding fields */
675+
return;
676+
}
639677
try {
640-
ResolvedJavaField[] subClassFields = field.isStatic() ? subtype.getStaticFields() : subtype.getInstanceFields(false);
678+
Set<ResolvedJavaField> subClassFields = new HashSet<>();
679+
subClassFields.addAll(Arrays.asList(subtype.getInstanceFields(false)));
680+
subClassFields.addAll(Arrays.asList(subtype.getStaticFields()));
641681
for (ResolvedJavaField javaField : subClassFields) {
642-
AnalysisField subclassField = (AnalysisField) javaField;
643-
if (subclassField.getName().equals(field.getName())) {
644-
hidingFields.add(subclassField);
682+
for (AnalysisField registeredField : superclassFields) {
683+
AnalysisField subclassField = (AnalysisField) javaField;
684+
if (subclassField.getName().equals(registeredField.getName())) {
685+
hidingFields.add(subclassField);
686+
}
645687
}
646688
}
647689
} catch (UnsupportedFeatureException | LinkageError e) {
@@ -653,8 +695,7 @@ private void checkSubtypeForOverridingField(AnalysisField field, AnalysisType su
653695
}
654696

655697
/**
656-
* Using {@link AnalysisType#findMethod(String, Signature)} here which uses
657-
* {@link Class#getDeclaredMethods()} internally, instead of
698+
* Filtering {@link Class#getDeclaredMethods()} here instead of directly calling
658699
* {@link AnalysisType#resolveConcreteMethod(ResolvedJavaMethod)} which gives different results
659700
* in at least two scenarios:
660701
* <p>
@@ -669,11 +710,19 @@ private void checkSubtypeForOverridingField(AnalysisField field, AnalysisType su
669710
*
670711
* @see ReflectionHostedSupport#getHidingReflectionMethods()
671712
*/
672-
private void checkSubtypeForOverridingMethod(AnalysisMethod method, AnalysisType subtype) {
713+
private void checkSubtypeForOverridingMethods(AnalysisType subtype, Collection<AnalysisMethod> superclassMethods) {
714+
if (isQueryFlagSet(subtype.getJavaClass(), ALL_DECLARED_METHODS_FLAG)) {
715+
/* All methods are already registered, no need for hiding methods */
716+
return;
717+
}
673718
try {
674-
AnalysisMethod subClassMethod = subtype.findMethod(method.getName(), method.getSignature());
675-
if (subClassMethod != null) {
676-
hidingMethods.add(subClassMethod);
719+
for (AnalysisMethod subClassMethod : subtype.getDeclaredMethods(false)) {
720+
for (AnalysisMethod registeredMethod : superclassMethods) {
721+
if (registeredMethod.getName().equals(subClassMethod.getName()) &&
722+
registeredMethod.getSignature().equals(subClassMethod.getSignature())) {
723+
hidingMethods.add(subClassMethod);
724+
}
725+
}
677726
}
678727
} catch (UnsupportedFeatureException | LinkageError e) {
679728
/*
@@ -1040,7 +1089,8 @@ private void maybeRegisterRecordComponents(Class<?> clazz) {
10401089
}
10411090
pendingRecordClasses.put(clazz, unregisteredAccessors);
10421091

1043-
unregisteredAccessors.removeIf(accessor -> registeredMethods.containsKey(metaAccess.lookupJavaMethod(accessor)));
1092+
AnalysisType analysisType = metaAccess.lookupJavaType(clazz);
1093+
unregisteredAccessors.removeIf(accessor -> registeredMethods.getOrDefault(analysisType, Collections.emptyMap()).containsKey(metaAccess.lookupJavaMethod(accessor)));
10441094
if (unregisteredAccessors.isEmpty()) {
10451095
registerRecordComponents(clazz);
10461096
}
@@ -1091,13 +1141,13 @@ public int getEnabledReflectionQueries(Class<?> clazz) {
10911141
}
10921142

10931143
@Override
1094-
public Map<AnalysisField, ConditionalRuntimeValue<Field>> getReflectionFields() {
1144+
public Map<AnalysisType, Map<AnalysisField, ConditionalRuntimeValue<Field>>> getReflectionFields() {
10951145
assert sealed;
10961146
return Collections.unmodifiableMap(registeredFields);
10971147
}
10981148

10991149
@Override
1100-
public Map<AnalysisMethod, ConditionalRuntimeValue<Executable>> getReflectionExecutables() {
1150+
public Map<AnalysisType, Map<AnalysisMethod, ConditionalRuntimeValue<Executable>>> getReflectionExecutables() {
11011151
assert sealed;
11021152
return Collections.unmodifiableMap(registeredMethods);
11031153
}

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionHostedSupport.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,9 @@
3939
public interface ReflectionHostedSupport {
4040
Map<Class<?>, Set<Class<?>>> getReflectionInnerClasses();
4141

42-
Map<AnalysisField, ConditionalRuntimeValue<Field>> getReflectionFields();
42+
Map<AnalysisType, Map<AnalysisField, ConditionalRuntimeValue<Field>>> getReflectionFields();
4343

44-
Map<AnalysisMethod, ConditionalRuntimeValue<Executable>> getReflectionExecutables();
44+
Map<AnalysisType, Map<AnalysisMethod, ConditionalRuntimeValue<Executable>>> getReflectionExecutables();
4545

4646
Object getAccessor(AnalysisMethod method);
4747

0 commit comments

Comments
 (0)