From 945618756f6cd89f01fa8246b3e09c4abafbf986 Mon Sep 17 00:00:00 2001 From: Christian Wimmer Date: Tue, 18 Jun 2024 16:52:29 -0700 Subject: [PATCH] Fix scalability bottlenecks in the static analysis --- .../StandaloneAnalysisFeatureImpl.java | 2 +- .../graal/pointsto/PointsToAnalysis.java | 10 +- .../graal/pointsto/meta/AnalysisMethod.java | 134 +++++++----------- .../graal/pointsto/meta/AnalysisType.java | 105 ++++++-------- .../graal/pointsto/meta/AnalysisUniverse.java | 89 +++++------- .../pointsto/reports/CallTreePrinter.java | 2 +- .../pointsto/results/StrengthenGraphs.java | 30 +++- .../pointsto/util/ConcurrentLightHashSet.java | 3 +- .../ReachabilityAnalysisEngine.java | 9 +- .../reachability/ReachabilityInvokeInfo.java | 3 +- .../svm/core/heap/SubstrateReferenceMap.java | 13 +- .../hotspot/libgraal/LibGraalFeature.java | 2 +- .../GraalGraphObjectReplacer.java | 12 +- .../RuntimeCompiledMethodSupport.java | 6 +- .../com/oracle/svm/hosted/FeatureImpl.java | 2 +- .../code/RestrictHeapAccessCalleesImpl.java | 6 +- .../image/NativeImageDebugInfoProvider.java | 15 +- .../oracle/svm/hosted/meta/HostedMethod.java | 7 +- .../svm/hosted/meta/UniverseBuilder.java | 2 +- 19 files changed, 218 insertions(+), 234 deletions(-) diff --git a/substratevm/src/com.oracle.graal.pointsto.standalone/src/com/oracle/graal/pointsto/standalone/features/StandaloneAnalysisFeatureImpl.java b/substratevm/src/com.oracle.graal.pointsto.standalone/src/com/oracle/graal/pointsto/standalone/features/StandaloneAnalysisFeatureImpl.java index 310d5e62a9a7..3578a0313baa 100644 --- a/substratevm/src/com.oracle.graal.pointsto.standalone/src/com/oracle/graal/pointsto/standalone/features/StandaloneAnalysisFeatureImpl.java +++ b/substratevm/src/com.oracle.graal.pointsto.standalone/src/com/oracle/graal/pointsto/standalone/features/StandaloneAnalysisFeatureImpl.java @@ -158,7 +158,7 @@ public Set reachableMethodOverrides(Executable baseMethod) { } Set reachableMethodOverrides(AnalysisMethod baseMethod) { - return AnalysisUniverse.getMethodImplementations(baseMethod, true); + return baseMethod.collectMethodImplementations(true); } } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/PointsToAnalysis.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/PointsToAnalysis.java index cbe9ec753d4d..50e072eb1b87 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/PointsToAnalysis.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/PointsToAnalysis.java @@ -597,10 +597,12 @@ public TypeFlow getTypeFlow() { @Override public boolean finish() throws InterruptedException { try (Indent indent = debug.logAndIndent("starting analysis in BigBang.finish")) { - universe.setAnalysisDataValid(false); - boolean didSomeWork = doTypeflow(); - assert executor.getPostedOperations() == 0 : executor.getPostedOperations(); - universe.setAnalysisDataValid(true); + boolean didSomeWork = false; + do { + didSomeWork |= doTypeflow(); + assert executor.getPostedOperations() == 0 : executor.getPostedOperations(); + universe.runAtFixedPoint(); + } while (executor.getPostedOperations() > 0); return didSomeWork; } } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java index 1d7031f07617..0d04dfdd3b00 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java @@ -34,6 +34,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -107,6 +108,9 @@ public abstract class AnalysisMethod extends AnalysisElement implements WrappedJ private static final AtomicReferenceFieldUpdater isInlinedUpdater = AtomicReferenceFieldUpdater .newUpdater(AnalysisMethod.class, Object.class, "isInlined"); + static final AtomicReferenceFieldUpdater allImplementationsUpdater = AtomicReferenceFieldUpdater + .newUpdater(AnalysisMethod.class, Object.class, "allImplementations"); + public record Signature(String name, AnalysisType[] parameterTypes) { } @@ -120,6 +124,7 @@ public record Signature(String name, AnalysisType[] parameterTypes) { private final LocalVariableTable localVariableTable; private final String name; private final String qualifiedName; + private final int modifiers; protected final AnalysisType declaringClass; protected final ResolvedSignature signature; @@ -164,10 +169,12 @@ public record Signature(String name, AnalysisType[] parameterTypes) { private EncodedGraph analyzedGraph; /** - * All concrete methods that can actually be called when calling this method. This includes all - * overridden methods in subclasses, as well as this method if it is non-abstract. + * Concrete methods that could possibly be called when calling this method. This also includes + * methods that are not reachable yet, i.e., this set must be filtered before it can be used. It + * never includes the method itself to reduce the size. See + * {@link AnalysisMethod#collectMethodImplementations} for more details. */ - protected AnalysisMethod[] implementations; + @SuppressWarnings("unused") private volatile Object allImplementations; /** * Indicates that this method returns all instantiated types. This is necessary when there are @@ -189,6 +196,7 @@ protected AnalysisMethod(AnalysisUniverse universe, ResolvedJavaMethod wrapped, name = createName(wrapped, multiMethodKey); qualifiedName = format("%H.%n(%P)"); + modifiers = wrapped.getModifiers(); if (universe.hostVM().useBaseLayer()) { int mid = universe.getImageLayerLoader().lookupHostedMethodInBaseLayer(this); @@ -258,6 +266,7 @@ protected AnalysisMethod(AnalysisMethod original, MultiMethodKey multiMethodKey) name = createName(wrapped, multiMethodKey); qualifiedName = format("%H.%n(%P)"); + modifiers = original.modifiers; this.multiMethodKey = multiMethodKey; assert original.multiMethodMap != null; @@ -550,81 +559,10 @@ public void onImplementationInvoked() { @Override public void onReachable() { notifyReachabilityCallbacks(declaringClass.getUniverse(), new ArrayList<>()); - processMethodOverrides(); - } - - private void processMethodOverrides() { - if (wrapped.canBeStaticallyBound() || isConstructor()) { - notifyMethodOverride(this); - } else if (declaringClass.isAnySubtypeInstantiated()) { - /* - * If neither the declaring class nor a subtype is instantiated then this method cannot - * be marked as invoked, so it cannot be an override. - */ - declaringClass.forAllSuperTypes(superType -> { - /* - * Iterate all the super types (including this type itself) looking for installed - * override notifications. If this method is found in a super type, and it has an - * override handler installed in that type, pass this method to the callback. It - * doesn't matter if the superMethod is actually reachable, only if it has any - * override handlers installed. Note that ResolvedJavaType.resolveMethod() cannot be - * used here because it only resolves methods declared by the type itself or if the - * method's declaring class is assignable from the type. - */ - AnalysisMethod superMethod = findInType(superType); - if (superMethod != null) { - superMethod.notifyMethodOverride(AnalysisMethod.this); - } - }); - } - } - - /** Find if the type declares a method with the same name and signature as this method. */ - private AnalysisMethod findInType(AnalysisType type) { - try { - return type.findMethod(wrapped.getName(), getSignature()); - } catch (UnsupportedFeatureException | LinkageError e) { - /* Ignore linking errors and deleted methods. */ - return null; - } - } - - protected void notifyMethodOverride(AnalysisMethod override) { - declaringClass.getOverrideReachabilityNotifications(this).forEach(n -> n.notifyCallback(getUniverse(), override)); } public void registerOverrideReachabilityNotification(MethodOverrideReachableNotification notification) { - declaringClass.registerOverrideReachabilityNotification(this, notification); - } - - /** - * Resolves this method in the provided type, but only if the type or any of its subtypes is - * marked as instantiated. - */ - protected AnalysisMethod resolveInType(AnalysisType holder) { - return resolveInType(holder, holder.isAnySubtypeInstantiated()); - } - - protected AnalysisMethod resolveInType(AnalysisType holder, boolean holderOrSubtypeInstantiated) { - /* - * If the holder and all subtypes are not instantiated, then we do not need to resolve the - * method. The method cannot be marked as invoked. - */ - if (holderOrSubtypeInstantiated || isIntrinsicMethod()) { - AnalysisMethod resolved; - try { - resolved = holder.resolveConcreteMethod(this, null); - } catch (UnsupportedFeatureException e) { - /* An unsupported overriding method is not reachable. */ - resolved = null; - } - /* - * resolved == null means that the method in the base class was called, but never with - * this holder. - */ - return resolved; - } - return null; + getUniverse().registerOverrideReachabilityNotification(this, notification); } @Override @@ -699,7 +637,7 @@ public Parameter[] getParameters() { @Override public int getModifiers() { - return wrapped.getModifiers(); + return modifiers; } @Override @@ -735,12 +673,46 @@ public boolean canBeStaticallyBound() { } - public AnalysisMethod[] getImplementations() { - assert getUniverse().analysisDataValid : this; - if (implementations == null) { - return new AnalysisMethod[0]; + /** + * Returns all methods that override (= implement) this method. If the + * {@code includeInlinedMethods} parameter is true, all reachable overrides are returned; if it + * is false, only invoked methods are returned (and methods that are already inlined at all call + * sites are excluded). + * + * In the parallel static analysis, it is difficult to have this information always available: + * when a method becomes reachable or invoked, it is not known which other methods it overrides. + * Therefore, we collect all possible implementations in {@link #allImplementations} without + * taking reachability into account, and then filter this too-large set of methods here on + * demand. + */ + public Set collectMethodImplementations(boolean includeInlinedMethods) { + /* + * To keep the allImplementations set as small as possible (and empty for most methods), the + * set never includes this method itself. It is clear that every method is always an + * implementation of itself. + */ + boolean includeOurselfs = (isStatic() || getDeclaringClass().isAnySubtypeInstantiated()) && + (includeInlinedMethods ? isReachable() : isImplementationInvoked()); + + int allImplementationsSize = ConcurrentLightHashSet.size(this, allImplementationsUpdater); + if (allImplementationsSize == 0) { + /* Fast-path that avoids allocation of a full HashSet. */ + return includeOurselfs ? Set.of(this) : Set.of(); + } + + Set result = new HashSet<>(allImplementationsSize + 1); + if (includeOurselfs) { + result.add(this); } - return implementations; + ConcurrentLightHashSet.forEach(this, allImplementationsUpdater, (AnalysisMethod override) -> { + if (override.getDeclaringClass().isAnySubtypeInstantiated()) { + if (includeInlinedMethods ? override.isReachable() : override.isImplementationInvoked()) { + result.add(override); + } + } + }); + + return result; } @Override diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java index 657f9b9966f9..b213b49501a9 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java @@ -24,13 +24,12 @@ */ package com.oracle.graal.pointsto.meta; +import java.lang.invoke.VarHandle; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -58,7 +57,6 @@ import com.oracle.graal.pointsto.util.AnalysisError; import com.oracle.graal.pointsto.util.AnalysisFuture; import com.oracle.graal.pointsto.util.AtomicUtils; -import com.oracle.graal.pointsto.util.ConcurrentLightHashMap; import com.oracle.graal.pointsto.util.ConcurrentLightHashSet; import com.oracle.svm.util.LogUtils; @@ -88,9 +86,6 @@ public abstract class AnalysisType extends AnalysisElement implements WrappedJav private static final AtomicReferenceFieldUpdater subtypeReachableNotificationsUpdater = AtomicReferenceFieldUpdater .newUpdater(AnalysisType.class, Object.class, "subtypeReachableNotifications"); - private static final AtomicReferenceFieldUpdater overrideReachableNotificationsUpdater = AtomicReferenceFieldUpdater - .newUpdater(AnalysisType.class, Object.class, "overrideReachableNotifications"); - private static final AtomicReferenceFieldUpdater instantiatedNotificationsUpdater = AtomicReferenceFieldUpdater .newUpdater(AnalysisType.class, Object.class, "typeInstantiatedNotifications"); @@ -109,6 +104,9 @@ public abstract class AnalysisType extends AnalysisElement implements WrappedJav private static final AtomicIntegerFieldUpdater isAnySubtypeInstantiatedUpdater = AtomicIntegerFieldUpdater .newUpdater(AnalysisType.class, "isAnySubtypeInstantiated"); + static final AtomicReferenceFieldUpdater overrideableMethodsUpdater = AtomicReferenceFieldUpdater + .newUpdater(AnalysisType.class, Object.class, "overrideableMethods"); + protected final AnalysisUniverse universe; private final ResolvedJavaType wrapped; private final String qualifiedName; @@ -164,6 +162,7 @@ public abstract class AnalysisType extends AnalysisElement implements WrappedJav private final AnalysisType elementalType; private final AnalysisType[] interfaces; + private AnalysisMethod[] declaredMethods; /* isArray is an expensive operation so we eagerly compute it */ private final boolean isArray; @@ -188,13 +187,6 @@ public abstract class AnalysisType extends AnalysisElement implements WrappedJav */ @SuppressWarnings("unused") private volatile Object subtypeReachableNotifications; - /** - * Contains reachability handlers that are notified when any of the method override becomes - * reachable *and* the declaring class of the override (or any subtype) is instantiated. Each - * handler is notified only once per method override. - */ - @SuppressWarnings("unused") private volatile Object overrideReachableNotifications; - /** * The reachability handler that were registered at the time the type was marked as reachable. * Reachability handler that are added by the user later on are not included in this list, i.e., @@ -214,6 +206,14 @@ public abstract class AnalysisType extends AnalysisElement implements WrappedJav */ @SuppressWarnings("unused") private volatile Object objectReachableCallbacks; + /** + * All methods of this type that can be overridden, i.e., that are not statically bound. Note + * that this set can contain methods that are not returned by {@link #getDeclaredMethods}: For + * classes that contain interfaces, the VM creates synthetic bridge methods that are not in the + * class file and therefore not in the list of declared methods. + */ + @SuppressWarnings("unused") private volatile Object overrideableMethods; + @SuppressWarnings("this-escape") public AnalysisType(AnalysisUniverse universe, ResolvedJavaType javaType, JavaKind storageKind, AnalysisType objectType, AnalysisType cloneableType) { this.universe = universe; @@ -526,9 +526,10 @@ public boolean registerAsInstantiated(Object reason) { } protected void onInstantiated() { + forAllSuperTypes(superType -> AtomicUtils.atomicMark(superType, isAnySubtypeInstantiatedUpdater)); + universe.onTypeInstantiated(this); notifyInstantiatedCallbacks(); - processMethodOverrides(); } public boolean registerAsUnsafeAllocated(Object reason) { @@ -536,37 +537,6 @@ public boolean registerAsUnsafeAllocated(Object reason) { return AtomicUtils.atomicSet(this, reason, isUnsafeAllocatedUpdater); } - private void processMethodOverrides() { - /* - * Walk up the type hierarchy from this type keeping track of all processed types. For each - * superType iterate all the override notifications and resolve the methods in all the seen - * types, which are subtypes from the point of view of the superType itself. Thus, although - * only *this* type may be marked as instantiated, any *intermediate* types between this - * type and a super type that declares override notifications will be processed and any - * overrides, if reachable, will be passed to the callback. These intermediate types, i.e., - * the seenSubtypes set, may not be instantiated but the overrides that they declare, if - * reachable, could be specially invoked, e.g., via super calls. Note that the baseMethod is - * not actually an override of itself, but the `registerMethodOverrideReachabilityHandler` - * API explicitly includes the base method too. - */ - Set seenSubtypes = new HashSet<>(); - forAllSuperTypes(superType -> { - AtomicUtils.atomicMark(superType, isAnySubtypeInstantiatedUpdater); - seenSubtypes.add(superType); - Map> overrides = ConcurrentLightHashMap.getEntries(superType, overrideReachableNotificationsUpdater); - for (var entry : overrides.entrySet()) { - AnalysisMethod baseMethod = entry.getKey(); - Set overrideNotifications = entry.getValue(); - for (AnalysisType subType : seenSubtypes) { - AnalysisMethod override = baseMethod.resolveInType(subType); - if (override != null && override.isReachable()) { - overrideNotifications.forEach(n -> n.notifyCallback(universe, override)); - } - } - } - }); - } - /** * Register the type as assignable with all its super types. This is a blocking call to ensure * that the type is registered with all its super types before it is propagated by the analysis @@ -626,22 +596,26 @@ protected void onReachable() { */ registerAsInstantiated("All array types are marked as instantiated eagerly."); } + + universe.getBigbang().postTask(unused -> forAllSuperTypes(this::prepareMethodImplementations, false)); + ensureOnTypeReachableTaskDone(); } - public void registerSubtypeReachabilityNotification(SubtypeReachableNotification notification) { - ConcurrentLightHashSet.addElement(this, subtypeReachableNotificationsUpdater, notification); - } + /** Prepare information that {@link AnalysisMethod#collectMethodImplementations} needs. */ + private void prepareMethodImplementations(AnalysisType superType) { + ConcurrentLightHashSet.forEach(superType, overrideableMethodsUpdater, (AnalysisMethod method) -> { + assert !method.canBeStaticallyBound() && !method.isConstructor() : method; - public void registerOverrideReachabilityNotification(AnalysisMethod declaredMethod, MethodOverrideReachableNotification notification) { - assert declaredMethod.getDeclaringClass() == this : declaredMethod; - Set overrideNotifications = ConcurrentLightHashMap.computeIfAbsent(this, - overrideReachableNotificationsUpdater, declaredMethod, m -> ConcurrentHashMap.newKeySet()); - overrideNotifications.add(notification); + AnalysisMethod override = resolveConcreteMethod(method, null); + if (override != null && !override.equals(method)) { + ConcurrentLightHashSet.addElement(method, AnalysisMethod.allImplementationsUpdater, override); + } + }); } - public Set getOverrideReachabilityNotifications(AnalysisMethod method) { - return ConcurrentLightHashMap.getOrDefault(this, overrideReachableNotificationsUpdater, method, Collections.emptySet()); + public void registerSubtypeReachabilityNotification(SubtypeReachableNotification notification) { + ConcurrentLightHashSet.addElement(this, subtypeReachableNotificationsUpdater, notification); } public void registerObjectReachableCallback(ObjectReachableCallback callback) { @@ -1096,9 +1070,15 @@ public AnalysisMethod resolveConcreteMethod(ResolvedJavaMethod method, ResolvedJ */ ResolvedJavaType originalCallerType = originalMethod.getDeclaringClass(); - newResolvedMethod = universe.lookup(wrapped.resolveConcreteMethod(originalMethod, originalCallerType)); - if (newResolvedMethod == null) { - newResolvedMethod = getUniverse().getBigbang().fallbackResolveConcreteMethod(this, (AnalysisMethod) method); + try { + newResolvedMethod = universe.lookup(wrapped.resolveConcreteMethod(originalMethod, originalCallerType)); + if (newResolvedMethod == null) { + newResolvedMethod = getUniverse().getBigbang().fallbackResolveConcreteMethod(this, (AnalysisMethod) method); + } + + } catch (UnsupportedFeatureException e) { + /* An unsupported overriding method is not reachable. */ + newResolvedMethod = null; } } @@ -1256,7 +1236,14 @@ public ResolvedJavaMethod[] getDeclaredMethods() { @Override public AnalysisMethod[] getDeclaredMethods(boolean forceLink) { GraalError.guarantee(forceLink == false, "only use getDeclaredMethods without forcing to link, because linking can throw LinkageError"); - return universe.lookup(wrapped.getDeclaredMethods(forceLink)); + AnalysisMethod[] result = declaredMethods; + if (result == null) { + result = universe.lookup(wrapped.getDeclaredMethods(forceLink)); + /* Ensure array element initializations are published before publishing the array. */ + VarHandle.storeStoreFence(); + declaredMethods = result; + } + return result; } @Override diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisUniverse.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisUniverse.java index dabb2a1fbf98..3027be4e6b90 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisUniverse.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisUniverse.java @@ -27,7 +27,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -56,7 +55,9 @@ import com.oracle.graal.pointsto.infrastructure.Universe; import com.oracle.graal.pointsto.infrastructure.WrappedConstantPool; import com.oracle.graal.pointsto.infrastructure.WrappedJavaType; +import com.oracle.graal.pointsto.meta.AnalysisElement.MethodOverrideReachableNotification; import com.oracle.graal.pointsto.util.AnalysisError; +import com.oracle.graal.pointsto.util.ConcurrentLightHashSet; import jdk.graal.compiler.api.replacements.SnippetReflectionProvider; import jdk.graal.compiler.core.common.SuppressFBWarnings; @@ -99,13 +100,6 @@ public class AnalysisUniverse implements Universe { final AtomicInteger nextMethodId = new AtomicInteger(1); final AtomicInteger nextFieldId = new AtomicInteger(1); - /** - * True if the analysis has converged and the analysis data is valid. This is similar to - * {@link #sealed} but in contrast to {@link #sealed}, the analysis data can be set to invalid - * again, e.g. if features modify the universe. - */ - boolean analysisDataValid; - protected final SubstitutionProcessor substitutions; private Function[] objectReplacers; @@ -129,6 +123,8 @@ public class AnalysisUniverse implements Universe { private BigBang bb; private DuringAnalysisAccess concurrentAnalysisAccess; + private final Map> methodOverrideReachableNotifications = new ConcurrentHashMap<>(); + public JavaKind getWordKind() { return wordKind; } @@ -179,13 +175,6 @@ public boolean sealed() { return sealed; } - public void setAnalysisDataValid(boolean dataIsValid) { - if (dataIsValid) { - collectMethodImplementations(); - } - analysisDataValid = dataIsValid; - } - public AnalysisType optionalLookup(ResolvedJavaType type) { ResolvedJavaType actualType = substitutions.lookup(type); Object claim = types.get(actualType); @@ -425,12 +414,29 @@ private AnalysisMethod createMethod(ResolvedJavaMethod method) { } AnalysisMethod newValue = analysisFactory.createMethod(this, method); AnalysisMethod oldValue = methods.putIfAbsent(method, newValue); - if (oldValue == null && newValue.isInBaseLayer()) { - getImageLayerLoader().patchBaseLayerMethod(newValue); + + if (oldValue == null) { + if (newValue.isInBaseLayer()) { + getImageLayerLoader().patchBaseLayerMethod(newValue); + } + prepareMethodImplementations(newValue); } return oldValue != null ? oldValue : newValue; } + /** Prepare information that {@link AnalysisMethod#collectMethodImplementations} needs. */ + private static void prepareMethodImplementations(AnalysisMethod method) { + if (!method.canBeStaticallyBound() && !method.isConstructor()) { + ConcurrentLightHashSet.addElement(method.declaringClass, AnalysisType.overrideableMethodsUpdater, method); + for (AnalysisType subtype : method.declaringClass.getAllSubtypes()) { + AnalysisMethod override = subtype.resolveConcreteMethod(method, null); + if (override != null && !override.equals(method)) { + ConcurrentLightHashSet.addElement(method, AnalysisMethod.allImplementationsUpdater, override); + } + } + } + } + public AnalysisMethod[] lookup(JavaMethod[] inputs) { List result = new ArrayList<>(inputs.length); for (JavaMethod method : inputs) { @@ -599,45 +605,24 @@ public Object replaceObject(Object source) { return destination; } - public static Set reachableMethodOverrides(AnalysisMethod baseMethod) { - return getMethodImplementations(baseMethod, true); - } - - private void collectMethodImplementations() { - for (AnalysisMethod method : methods.values()) { - Set implementations = getMethodImplementations(method, false); - method.implementations = implementations.toArray(new AnalysisMethod[implementations.size()]); - } - } - - public static Set getMethodImplementations(AnalysisMethod method, boolean includeInlinedMethods) { - Set implementations = new LinkedHashSet<>(); - if (method.wrapped.canBeStaticallyBound() || method.isConstructor()) { - if (includeInlinedMethods ? method.isReachable() : method.isImplementationInvoked()) { - implementations.add(method); - } - } else { - collectMethodImplementations(method, method.getDeclaringClass(), implementations, includeInlinedMethods); - } - return implementations; + public void registerOverrideReachabilityNotification(AnalysisMethod declaredMethod, MethodOverrideReachableNotification notification) { + methodOverrideReachableNotifications.computeIfAbsent(declaredMethod, m -> ConcurrentHashMap.newKeySet()).add(notification); } - private static boolean collectMethodImplementations(AnalysisMethod method, AnalysisType holder, Set implementations, boolean includeInlinedMethods) { - boolean holderOrSubtypeInstantiated = holder.isInstantiated(); - for (AnalysisType subClass : holder.getSubTypes()) { - if (subClass.equals(holder)) { - /* Subtypes include the holder type itself. The holder is processed below. */ - continue; + public void runAtFixedPoint() { + /* + * It is too expensive to immediately send out override notifications when a new method + * becomes reachable. We therefore send them out after the analysis has reached a fixed + * point. When new tasks are scheduled, the parallel analysis must be restarted. + */ + for (var entry : methodOverrideReachableNotifications.entrySet()) { + var method = entry.getKey(); + for (var override : method.collectMethodImplementations(true)) { + for (var notification : entry.getValue()) { + notification.notifyCallback(this, override); + } } - holderOrSubtypeInstantiated |= collectMethodImplementations(method, subClass, implementations, includeInlinedMethods); } - - AnalysisMethod resolved = method.resolveInType(holder, holderOrSubtypeInstantiated); - if (resolved != null && (includeInlinedMethods ? resolved.isReachable() : resolved.isImplementationInvoked())) { - implementations.add(resolved); - } - - return holderOrSubtypeInstantiated; } /** diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/CallTreePrinter.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/CallTreePrinter.java index a0be812c268b..db223a78457c 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/CallTreePrinter.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/reports/CallTreePrinter.java @@ -188,7 +188,7 @@ public void buildCallTree() { roots.add(m); } if (m.isVirtualRootMethod()) { - for (AnalysisMethod impl : m.getImplementations()) { + for (AnalysisMethod impl : m.collectMethodImplementations(false)) { AnalysisError.guarantee(impl.isImplementationInvoked()); roots.add(impl); } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/results/StrengthenGraphs.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/results/StrengthenGraphs.java index 96191859aba3..b76b6a752b90 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/results/StrengthenGraphs.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/results/StrengthenGraphs.java @@ -145,6 +145,26 @@ public static class Options { public static final OptionKey StrengthenGraphWithConstants = new OptionKey<>(true); } + /** + * The hashCode implementation of {@link JavaMethodProfile} is bad because it does not take the + * actual methods into account, only the number of methods in the profile. This wrapper class + * provides a proper hashCode. + */ + record CachedJavaMethodProfile(JavaMethodProfile profile, int hash) { + @Override + public int hashCode() { + return hash; + } + + @Override + public boolean equals(Object obj) { + if (obj instanceof CachedJavaMethodProfile other) { + return profile.equals(other.profile); + } + return false; + } + } + protected final BigBang bb; /** * The universe used to convert analysis metadata to hosted metadata, or {@code null} if no @@ -153,7 +173,7 @@ public static class Options { protected final Universe converter; private final Map cachedTypeProfiles = new ConcurrentHashMap<>(); - private final Map cachedMethodProfiles = new ConcurrentHashMap<>(); + private final Map cachedMethodProfiles = new ConcurrentHashMap<>(); /* Cached option values to avoid repeated option lookup. */ private final int analysisSizeCutoff; @@ -1010,19 +1030,21 @@ protected JavaMethodProfile makeMethodProfile(Collection callees } var created = createMethodProfile(callees); var existing = cachedMethodProfiles.putIfAbsent(created, created); - return existing != null ? existing : created; + return existing != null ? existing.profile : created.profile; } - private JavaMethodProfile createMethodProfile(Collection callees) { + private CachedJavaMethodProfile createMethodProfile(Collection callees) { JavaMethodProfile.ProfiledMethod[] pitems = new JavaMethodProfile.ProfiledMethod[callees.size()]; + int hashCode = 0; double probability = 1d / pitems.length; int idx = 0; for (AnalysisMethod aMethod : callees) { ResolvedJavaMethod convertedMethod = converter == null ? aMethod : converter.lookup(aMethod); pitems[idx++] = new JavaMethodProfile.ProfiledMethod(convertedMethod, probability); + hashCode = hashCode * 31 + convertedMethod.hashCode(); } - return new JavaMethodProfile(0, pitems); + return new CachedJavaMethodProfile(new JavaMethodProfile(0, pitems), hashCode); } } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/util/ConcurrentLightHashSet.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/util/ConcurrentLightHashSet.java index 9d83bbff6fe8..79a4501ddc22 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/util/ConcurrentLightHashSet.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/util/ConcurrentLightHashSet.java @@ -55,7 +55,8 @@ private ConcurrentLightHashSet() { } - public static int size(Object elements) { + public static int size(U holder, AtomicReferenceFieldUpdater updater) { + Object elements = updater.get(holder); if (elements == null) { /* No elements. */ return 0; diff --git a/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/ReachabilityAnalysisEngine.java b/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/ReachabilityAnalysisEngine.java index 00083a9e6be5..51777d1c055b 100644 --- a/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/ReachabilityAnalysisEngine.java +++ b/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/ReachabilityAnalysisEngine.java @@ -292,10 +292,11 @@ public void markMethodInvoked(ReachabilityAnalysisMethod method, Object reason) @Override public boolean finish() throws InterruptedException { - universe.setAnalysisDataValid(false); - runReachability(); - assert executor.getPostedOperations() == 0 : executor.getPostedOperations(); - universe.setAnalysisDataValid(true); + do { + runReachability(); + assert executor.getPostedOperations() == 0 : executor.getPostedOperations(); + universe.runAtFixedPoint(); + } while (executor.getPostedOperations() > 0); return true; } diff --git a/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/ReachabilityInvokeInfo.java b/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/ReachabilityInvokeInfo.java index 7e7209868ff8..3e25c6cbfef5 100644 --- a/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/ReachabilityInvokeInfo.java +++ b/substratevm/src/com.oracle.graal.reachability/src/com/oracle/graal/reachability/ReachabilityInvokeInfo.java @@ -24,7 +24,6 @@ */ package com.oracle.graal.reachability; -import java.util.Arrays; import java.util.Collection; import java.util.List; @@ -63,7 +62,7 @@ public Collection getOriginalCallees() { if (isDirectInvoke) { return List.of(targetMethod); } - return Arrays.asList(targetMethod.getImplementations()); + return targetMethod.collectMethodImplementations(false); } @Override diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/SubstrateReferenceMap.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/SubstrateReferenceMap.java index 952f9a7a0c07..860b5ea1f601 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/SubstrateReferenceMap.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/SubstrateReferenceMap.java @@ -34,15 +34,15 @@ import org.graalvm.collections.EconomicMap; import org.graalvm.collections.Equivalence; -import jdk.graal.compiler.core.common.NumUtil; +import org.graalvm.nativeimage.ImageInfo; import com.oracle.svm.core.FrameAccess; import com.oracle.svm.core.config.ConfigurationValues; +import jdk.graal.compiler.core.common.NumUtil; import jdk.vm.ci.code.ReferenceMap; import jdk.vm.ci.code.StackSlot; import jdk.vm.ci.meta.Value; -import org.graalvm.nativeimage.ImageInfo; public class SubstrateReferenceMap extends ReferenceMap implements ReferenceMapEncoder.Input { /** @@ -220,7 +220,14 @@ public Set getDerivedOffsets(int baseOffset) { @Override public int hashCode() { - return shift ^ (shiftedOffsets == null ? 0 : shiftedOffsets.hashCode()) ^ (derived == null ? 0 : derived.hashCode()); + int result = shift * 31 + (derived == null ? 42 : derived.hashCode()); + if (shiftedOffsets != null) { + /* We do not use BitSet.hashCode because it has a too high collision rate. */ + for (int idx = shiftedOffsets.nextSetBit(0); idx != -1; idx = shiftedOffsets.nextSetBit(idx + 1)) { + result = result * 31 + idx + 42; + } + } + return result; } @Override diff --git a/substratevm/src/com.oracle.svm.graal.hotspot.libgraal/src/com/oracle/svm/graal/hotspot/libgraal/LibGraalFeature.java b/substratevm/src/com.oracle.svm.graal.hotspot.libgraal/src/com/oracle/svm/graal/hotspot/libgraal/LibGraalFeature.java index cc8510e0480b..3993cf1d4060 100644 --- a/substratevm/src/com.oracle.svm.graal.hotspot.libgraal/src/com/oracle/svm/graal/hotspot/libgraal/LibGraalFeature.java +++ b/substratevm/src/com.oracle.svm.graal.hotspot.libgraal/src/com/oracle/svm/graal/hotspot/libgraal/LibGraalFeature.java @@ -591,7 +591,7 @@ private static void verifyReachableTruffleClasses(AfterAnalysisAccess access) { seen.put(analysisMethod, "direct root"); } if (analysisMethod.isVirtualRootMethod()) { - for (AnalysisMethod impl : analysisMethod.getImplementations()) { + for (AnalysisMethod impl : analysisMethod.collectMethodImplementations(false)) { VMError.guarantee(impl.isImplementationInvoked()); seen.put(impl, "virtual root"); } diff --git a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/runtimecompilation/GraalGraphObjectReplacer.java b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/runtimecompilation/GraalGraphObjectReplacer.java index 67843f9cbb74..fea5a930a1ee 100644 --- a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/runtimecompilation/GraalGraphObjectReplacer.java +++ b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/runtimecompilation/GraalGraphObjectReplacer.java @@ -398,16 +398,16 @@ private synchronized SubstrateSignature createSignature(ResolvedSignature entry : methods.entrySet()) { AnalysisMethod aMethod = entry.getKey(); SubstrateMethod sMethod = entry.getValue(); - AnalysisMethod[] aMethodImplementations = aMethod.getImplementations(); - SubstrateMethod[] implementations = new SubstrateMethod[aMethodImplementations.length]; + HostedMethod hMethod = hUniverse.lookup(aMethod); + SubstrateMethod[] implementations = new SubstrateMethod[hMethod.getImplementations().length]; int idx = 0; - for (AnalysisMethod impl : aMethodImplementations) { - SubstrateMethod sImpl = methods.get(impl); - VMError.guarantee(sImpl != null, "SubstrateMethod for %s missing.", impl); + for (var hImplementation : hMethod.getImplementations()) { + SubstrateMethod sImpl = methods.get(hImplementation.getWrapped()); + VMError.guarantee(sImpl != null, "SubstrateMethod for %s missing.", hImplementation); implementations[idx++] = sImpl; } sMethod.setImplementations(implementations); diff --git a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/runtimecompilation/RuntimeCompiledMethodSupport.java b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/runtimecompilation/RuntimeCompiledMethodSupport.java index 77b1c7221479..0521c173606d 100644 --- a/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/runtimecompilation/RuntimeCompiledMethodSupport.java +++ b/substratevm/src/com.oracle.svm.graal/src/com/oracle/svm/graal/hosted/runtimecompilation/RuntimeCompiledMethodSupport.java @@ -163,7 +163,7 @@ public static void onCompileQueueCreation(BigBang bb, HostedUniverse hUniverse, VMError.shouldNotReachHere(exception); } - encodeRuntimeCompiledMethods(compilationState); + encodeRuntimeCompiledMethods(hUniverse, compilationState); /* * For Deoptimization Targets add a custom phase which removes all deoptimization @@ -299,7 +299,7 @@ static boolean verifyNodes(StructuredGraph graph) { } @SuppressWarnings("try") - private static void encodeRuntimeCompiledMethods(CompilationState compilationState) { + private static void encodeRuntimeCompiledMethods(HostedUniverse hUniverse, CompilationState compilationState) { compilationState.graphEncoder.finishPrepare(); // at this point no new deoptimization entrypoints can be registered. @@ -323,7 +323,7 @@ private static void encodeRuntimeCompiledMethods(CompilationState compilationSta com.oracle.svm.graal.TruffleRuntimeCompilationSupport.setGraphEncoding(null, compilationState.graphEncoder.getEncoding(), compilationState.graphEncoder.getObjects(), compilationState.graphEncoder.getNodeClasses()); - compilationState.objectReplacer.setMethodsImplementations(); + compilationState.objectReplacer.setMethodsImplementations(hUniverse); } /** diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/FeatureImpl.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/FeatureImpl.java index b8830cf8ee19..eb8ceb54d361 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/FeatureImpl.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/FeatureImpl.java @@ -249,7 +249,7 @@ public Set reachableMethodOverrides(Executable baseMethod) { } Set reachableMethodOverrides(AnalysisMethod baseMethod) { - return AnalysisUniverse.reachableMethodOverrides(baseMethod); + return baseMethod.collectMethodImplementations(true); } public void rescanObject(Object obj) { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/RestrictHeapAccessCalleesImpl.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/RestrictHeapAccessCalleesImpl.java index 13c69d9c774c..22fc61f68288 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/RestrictHeapAccessCalleesImpl.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/RestrictHeapAccessCalleesImpl.java @@ -31,6 +31,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import org.graalvm.nativeimage.ImageSingletons; @@ -131,7 +132,8 @@ private static void setMethodRestrictionInfo(AnalysisMethod method, Map implementations = method.collectMethodImplementations(false); + for (AnalysisMethod impl : implementations) { if (impl.isAnnotationPresent(RestrictHeapAccess.class) && !impl.equals(method)) { /* Annotated overrides take precedence, so process them first. */ setMethodRestrictionInfo(impl, aggregation); @@ -140,7 +142,7 @@ private static void setMethodRestrictionInfo(AnalysisMethod method, Map