From b5a9d5dfcb4aab97fa880e090437076c98fdc0c5 Mon Sep 17 00:00:00 2001 From: Sacha Coppey Date: Mon, 16 Sep 2024 12:50:28 +0200 Subject: [PATCH] Ensure the persistType and persistMethod methods are thread safe --- .../pointsto/heap/ImageLayerSnapshotUtil.java | 18 +++------- .../graal/pointsto/heap/ImageLayerWriter.java | 35 +++++++++++-------- .../svm/hosted/heap/SVMImageLayerWriter.java | 4 +-- .../heap/SVMImageLayerWriterHelper.java | 6 ++-- 4 files changed, 29 insertions(+), 34 deletions(-) diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerSnapshotUtil.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerSnapshotUtil.java index 609a55de69c0..3d30d926ac03 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerSnapshotUtil.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerSnapshotUtil.java @@ -327,9 +327,7 @@ protected AnalysisTypeBuiltIn(ImageLayerWriter imageLayerWriter, ImageLayerLoade @Override public String encode(ObjectCopier.Encoder encoder, Object obj) { AnalysisType type = (AnalysisType) obj; - if (!type.isReachable() && !imageLayerWriter.typesMap.containsKey(imageLayerWriter.imageLayerSnapshotUtil.getTypeIdentifier(type))) { - imageLayerWriter.persistType(type); - } + imageLayerWriter.persistType(type); return String.valueOf(type.getId()); } @@ -356,19 +354,13 @@ public String encode(ObjectCopier.Encoder encoder, Object obj) { AnalysisMethod method = (AnalysisMethod) obj; AnalysisType declaringClass = method.getDeclaringClass(); imageLayerWriter.elementsToPersist.add(new AnalysisFuture<>(() -> { - if (!method.isReachable() && !imageLayerWriter.methodsMap.containsKey(imageLayerWriter.imageLayerSnapshotUtil.getMethodIdentifier(method))) { - imageLayerWriter.persistAnalysisParsedGraph(method); - imageLayerWriter.persistMethod(method); - } + imageLayerWriter.persistAnalysisParsedGraph(method); + imageLayerWriter.persistMethod(method); })); for (AnalysisType parameter : method.toParameterList()) { - if (!parameter.isReachable() && !imageLayerWriter.typesMap.containsKey(imageLayerWriter.imageLayerSnapshotUtil.getTypeIdentifier(parameter))) { - imageLayerWriter.persistType(parameter); - } - } - if (!declaringClass.isReachable() && !imageLayerWriter.typesMap.containsKey(imageLayerWriter.imageLayerSnapshotUtil.getTypeIdentifier(declaringClass))) { - imageLayerWriter.persistType(declaringClass); + imageLayerWriter.persistType(parameter); } + imageLayerWriter.persistType(declaringClass); return String.valueOf(method.getId()); } diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerWriter.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerWriter.java index 9823cf2876b1..7921b8ed1f6c 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerWriter.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerWriter.java @@ -104,7 +104,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.EnumSet; -import java.util.HashSet; import java.util.IdentityHashMap; import java.util.List; import java.util.Map; @@ -150,6 +149,7 @@ public class ImageLayerWriter { protected final EconomicMap jsonMap; protected final List constantsToRelink; private final Set persistedTypeIds; + private final Set persistedMethodIds; protected final Map> typesMap; protected final Map> methodsMap; protected final Map> fieldsMap; @@ -212,7 +212,8 @@ public ImageLayerWriter(boolean useSharedLayerGraphs, ImageLayerSnapshotUtil ima this.imageLayerSnapshotUtil = imageLayerSnapshotUtil; this.jsonMap = EconomicMap.create(); this.constantsToRelink = new ArrayList<>(); - this.persistedTypeIds = new HashSet<>(); + this.persistedTypeIds = ConcurrentHashMap.newKeySet(); + this.persistedMethodIds = ConcurrentHashMap.newKeySet(); this.typesMap = new ConcurrentHashMap<>(); this.methodsMap = new ConcurrentHashMap<>(); this.fieldsMap = new ConcurrentHashMap<>(); @@ -325,9 +326,7 @@ protected void persistType(AnalysisType type) { * Some persisted types are not reachable. In this case, the super class has to be * persisted manually as well. */ - if (!superclass.isReachable()) { - persistType(superclass); - } + persistType(superclass); } EconomicMap typeMap = EconomicMap.create(); @@ -384,7 +383,14 @@ public void checkTypeStability(AnalysisType type) { } public void persistMethod(AnalysisMethod method) { + if (!persistedMethodIds.add(method.getId())) { + return; + } EconomicMap methodMap = getMethodMap(method); + persistMethod(method, methodMap); + } + + protected void persistMethod(AnalysisMethod method, EconomicMap methodMap) { Executable executable = method.getJavaMethod(); if (methodMap.containsKey(ID_TAG)) { @@ -424,11 +430,6 @@ public void persistMethod(AnalysisMethod method) { imageLayerWriterHelper.persistMethod(method, methodMap); } - public boolean isMethodPersisted(AnalysisMethod method) { - String name = imageLayerSnapshotUtil.getMethodIdentifier(method); - return methodsMap.containsKey(name); - } - public void persistMethodGraphs() { for (AnalysisMethod method : aUniverse.getMethods()) { if (method.isReachable()) { @@ -442,18 +443,22 @@ public void persistAnalysisParsedGraph(AnalysisMethod method) { Object analyzedGraph = method.getGraph(); if (analyzedGraph instanceof AnalysisParsedGraph analysisParsedGraph) { - if (!persistGraph(analysisParsedGraph.getEncodedGraph(), methodMap, ANALYSIS_PARSED_GRAPH_TAG)) { - return; + if (!methodMap.containsKey(INTRINSIC_TAG)) { + if (!persistGraph(analysisParsedGraph.getEncodedGraph(), methodMap, ANALYSIS_PARSED_GRAPH_TAG)) { + return; + } + methodMap.put(INTRINSIC_TAG, analysisParsedGraph.isIntrinsic()); } - methodMap.put(INTRINSIC_TAG, analysisParsedGraph.isIntrinsic()); } } public void persistMethodStrengthenedGraph(AnalysisMethod method) { EconomicMap methodMap = getMethodMap(method); - EncodedGraph analyzedGraph = method.getAnalyzedGraph(); - persistGraph(analyzedGraph, methodMap, STRENGTHENED_GRAPH_TAG); + if (!methodMap.containsKey(STRENGTHENED_GRAPH_TAG)) { + EncodedGraph analyzedGraph = method.getAnalyzedGraph(); + persistGraph(analyzedGraph, methodMap, STRENGTHENED_GRAPH_TAG); + } } private boolean persistGraph(EncodedGraph analyzedGraph, EconomicMap methodMap, String graphTag) { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriter.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriter.java index bbf99fe756c8..d9ebd7c99d98 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriter.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriter.java @@ -216,8 +216,8 @@ protected boolean shouldPersistMethod(AnalysisMethod method) { } @Override - public void persistMethod(AnalysisMethod method) { - super.persistMethod(method); + public void persistMethod(AnalysisMethod method, EconomicMap methodMap) { + super.persistMethod(method, methodMap); // register this method as persisted for name resolution HostedDynamicLayerInfo.singleton().recordPersistedMethod(hUniverse.lookup(method)); diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriterHelper.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriterHelper.java index 9ce455f23d23..5168c4d762fe 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriterHelper.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/heap/SVMImageLayerWriterHelper.java @@ -64,10 +64,8 @@ protected void persistMethod(AnalysisMethod method, EconomicMap if (method.wrapped instanceof FactoryMethod factoryMethod) { methodMap.put(WRAPPED_METHOD_TAG, FACTORY_TAG); AnalysisMethod targetConstructor = method.getUniverse().lookup(factoryMethod.getTargetConstructor()); - if (!method.isReachable() && !imageLayerWriter.isMethodPersisted(targetConstructor)) { - imageLayerWriter.persistAnalysisParsedGraph(targetConstructor); - imageLayerWriter.persistMethod(targetConstructor); - } + imageLayerWriter.persistAnalysisParsedGraph(targetConstructor); + imageLayerWriter.persistMethod(targetConstructor); methodMap.put(TARGET_CONSTRUCTOR_TAG, targetConstructor.getId()); methodMap.put(THROW_ALLOCATED_OBJECT_TAG, factoryMethod.throwAllocatedObject()); }