# HG changeset patch # User Doug Simon # Date 1365022331 -7200 # Node ID f32fa4cdfbb1982e59bc3de72b6d107697898e0c # Parent b648515abd0a743f865a9514631afe130853e6ea fixed concurrency issues in ReplacementsImpl diff -r b648515abd0a -r f32fa4cdfbb1 graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/HotSpotReplacementsImpl.java --- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/HotSpotReplacementsImpl.java Wed Apr 03 21:55:41 2013 +0200 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/HotSpotReplacementsImpl.java Wed Apr 03 22:52:11 2013 +0200 @@ -42,7 +42,7 @@ } @Override - protected void installMethodSubstitution(Member originalMethod, Method substituteMethod) { + protected void registerMethodSubstitution(Member originalMethod, Method substituteMethod) { if (substituteMethod.getDeclaringClass() == IntegerSubstitutions.class || substituteMethod.getDeclaringClass() == LongSubstitutions.class) { if (substituteMethod.getName().equals("bitCount")) { if (!config.usePopCountInstruction) { @@ -58,6 +58,6 @@ assert config.cipherBlockChainingEncryptAESCryptStub != 0L; assert config.cipherBlockChainingDecryptAESCryptStub != 0L; } - super.installMethodSubstitution(originalMethod, substituteMethod); + super.registerMethodSubstitution(originalMethod, substituteMethod); } } diff -r b648515abd0a -r f32fa4cdfbb1 graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java --- a/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java Wed Apr 03 21:55:41 2013 +0200 +++ b/graal/com.oracle.graal.replacements/src/com/oracle/graal/replacements/ReplacementsImpl.java Wed Apr 03 22:52:11 2013 +0200 @@ -54,24 +54,22 @@ private BoxingMethodPool pool; - /** - * A graph cache used by this installer to avoid using the compiler storage for each method - * processed during snippet installation. Without this, all processed methods are to be - * determined as {@linkplain InliningUtil#canIntrinsify intrinsifiable}. - */ private final ConcurrentMap graphCache; - private final ConcurrentMap originalToSubstitute; - - private final ConcurrentMap> macroNodeClasses; + // These data structures are all fully initialized during single-threaded + // compiler startup and so do not need to be concurrent. + private final Map registeredMethodSubstitutions; + private final Set registeredSnippets; + private final Map> registerMacroSubstitutions; public ReplacementsImpl(MetaAccessProvider runtime, Assumptions assumptions, TargetDescription target) { this.runtime = runtime; this.target = target; this.assumptions = assumptions; this.graphCache = new ConcurrentHashMap<>(); - this.originalToSubstitute = new ConcurrentHashMap<>(); - this.macroNodeClasses = new ConcurrentHashMap<>(); + this.registeredMethodSubstitutions = new HashMap<>(); + this.registeredSnippets = new HashSet<>(); + this.registerMacroSubstitutions = new HashMap<>(); } public void registerSnippets(Class snippets) { @@ -83,36 +81,40 @@ throw new RuntimeException("Snippet must not be abstract or native"); } ResolvedJavaMethod snippet = runtime.lookupJavaMethod(method); - graphCache.putIfAbsent(snippet, placeholder); + registeredSnippets.add(snippet); } } } - private final StructuredGraph placeholder = new StructuredGraph(); - public StructuredGraph getSnippet(ResolvedJavaMethod method) { + if (!registeredSnippets.contains(method)) { + return null; + } StructuredGraph graph = graphCache.get(method); - if (graph == placeholder) { + if (graph == null) { graph = createGraphMaker(null, null).makeGraph(method, inliningPolicy(method)); assert graph == graphCache.get(method); } return graph; + } public StructuredGraph getMethodSubstitution(ResolvedJavaMethod original) { + ResolvedJavaMethod substitute = registeredMethodSubstitutions.get(original); + if (substitute == null) { + return null; + } StructuredGraph graph = graphCache.get(original); - if (graph == placeholder) { - ResolvedJavaMethod substitute = originalToSubstitute.get(original); - if (substitute != null) { - graph = createGraphMaker(substitute, original).makeGraph(substitute, inliningPolicy(substitute)); - assert graph == graphCache.get(substitute); - } + if (graph == null) { + graph = createGraphMaker(substitute, original).makeGraph(substitute, inliningPolicy(substitute)); + assert graph == graphCache.get(substitute); } return graph; + } public Class getMacroSubstitution(ResolvedJavaMethod method) { - return macroNodeClasses.get(method); + return registerMacroSubstitutions.get(method); } public Assumptions getAssumptions() { @@ -143,7 +145,7 @@ Class[] originalParameters = originalParameters(substituteMethod, methodSubstitution.signature(), methodSubstitution.isStatic()); Member originalMethod = originalMethod(classSubstitution, originalName, originalParameters); if (originalMethod != null) { - installMethodSubstitution(originalMethod, substituteMethod); + registerMethodSubstitution(originalMethod, substituteMethod); } } if (macroSubstitution != null) { @@ -151,19 +153,19 @@ Class[] originalParameters = originalParameters(substituteMethod, macroSubstitution.signature(), macroSubstitution.isStatic()); Member originalMethod = originalMethod(classSubstitution, originalName, originalParameters); if (originalMethod != null) { - installMacroSubstitution(originalMethod, macroSubstitution.macro()); + registerMacroSubstitution(originalMethod, macroSubstitution.macro()); } } } } /** - * Installs a method substitution. + * Registers a method substitution. * * @param originalMember a method or constructor being substituted * @param substituteMethod the substitute method */ - protected void installMethodSubstitution(Member originalMember, Method substituteMethod) { + protected void registerMethodSubstitution(Member originalMember, Method substituteMethod) { ResolvedJavaMethod substitute = runtime.lookupJavaMethod(substituteMethod); ResolvedJavaMethod original; if (originalMember instanceof Method) { @@ -173,24 +175,23 @@ } Debug.log("substitution: " + MetaUtil.format("%H.%n(%p)", original) + " --> " + MetaUtil.format("%H.%n(%p)", substitute)); - graphCache.putIfAbsent(original, placeholder); - originalToSubstitute.put(original, substitute); + registeredMethodSubstitutions.put(original, substitute); } /** - * Installs a macro substitution. + * Registers a macro substitution. * * @param originalMethod a method or constructor being substituted * @param macro the substitute macro node class */ - protected void installMacroSubstitution(Member originalMethod, Class macro) { + protected void registerMacroSubstitution(Member originalMethod, Class macro) { ResolvedJavaMethod originalJavaMethod; if (originalMethod instanceof Method) { originalJavaMethod = runtime.lookupJavaMethod((Method) originalMethod); } else { originalJavaMethod = runtime.lookupJavaConstructor((Constructor) originalMethod); } - macroNodeClasses.put(originalJavaMethod, macro); + registerMacroSubstitutions.put(originalJavaMethod, macro); } private SnippetInliningPolicy inliningPolicy(ResolvedJavaMethod method) { @@ -264,9 +265,10 @@ private StructuredGraph parseGraph(final ResolvedJavaMethod method, final SnippetInliningPolicy policy) { StructuredGraph graph = graphCache.get(method); - if (graph == null || graph == placeholder) { - graph = buildGraph(method, policy == null ? inliningPolicy(method) : policy); - graphCache.put(method, graph); + if (graph == null) { + graphCache.putIfAbsent(method, buildGraph(method, policy == null ? inliningPolicy(method) : policy)); + graph = graphCache.get(method); + assert graph != null; } return graph; }