Reduce run-time contentions

thegridman · Aug 7, 2023 · 2d08eb3 · 2d08eb3
1 parent 1adfba1
commit 2d08eb3
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 23 deletions.
diff --git a/javatools/src/main/java/org/xvm/asm/Component.java b/javatools/src/main/java/org/xvm/asm/Component.java
@@ -833,28 +833,34 @@ public synchronized Map<String, Component> ensureChildByNameMap()
     /**
      * Make sure that any deferred child deserialization is complete
      */
-     protected synchronized void ensureChildren()
+     protected void ensureChildren()
         {
         if (m_abChildren != null)
             {
-            // first grab the deferred deserialization bytes and then make sure neither this nor any
-            // sibling retains hold of it (since it indicates that deserialization is deferred)
-            byte[] ab = m_abChildren;
-            for (Iterator<Component> siblings = siblings(); siblings.hasNext(); )
+            synchronized (this)
                 {
-                siblings.next().m_abChildren = null;
-                }
+                byte[] ab = m_abChildren;
+                if (ab != null)
+                    {
+                    // first grab the deferred deserialization bytes and then make sure neither this nor any
+                    // sibling retains hold of it (since it indicates that deserialization is deferred)
+                    for (Iterator<Component> siblings = siblings(); siblings.hasNext(); )
+                        {
+                        siblings.next().m_abChildren = null;
+                        }
 
-            // now read in the children
-            DataInput in = new DataInputStream(new ByteArrayInputStream(ab));
-            try
-                {
-                disassembleChildren(in, true);
-                }
-            catch (IOException e)
-                {
-                throw new IllegalStateException("IOException occurred in " + getIdentityConstant()
-                        + " during deferred read of child components", e);
+                    // now read in the children
+                    DataInput in = new DataInputStream(new ByteArrayInputStream(ab));
+                    try
+                        {
+                        disassembleChildren(in, true);
+                        }
+                    catch (IOException e)
+                        {
+                        throw new IllegalStateException("IOException occurred in " + getIdentityConstant()
+                                + " during deferred read of child components", e);
+                        }
+                    }
                 }
             }
         }
@@ -3925,7 +3931,7 @@ public Constant getResolvedConstant()
      * it can optionally lazily deserialize its children. This is possible because the "children"
      * block is length-encoded.
      */
-    private byte[] m_abChildren;
+    private volatile byte[] m_abChildren;
 
     /**
      * This holds all of the children of all of the siblings, except for methods (because they are

diff --git a/javatools/src/main/java/org/xvm/asm/ConstantPool.java b/javatools/src/main/java/org/xvm/asm/ConstantPool.java
@@ -3106,8 +3106,15 @@ protected void postRegisterAll(final boolean fOptimize)
      */
     private Map<Constant, Constant> ensureConstantLookup(Format format)
         {
-        ensureLookup();
-        return m_mapConstants.get(format);
+        // seemingly unsafe operation against m_mapConstants is concurrently safe, because
+        // the only possible mutations outside of "ensureLookup", such as "disassemble()" and
+        // "optimize()" are guaranteed to be single-threaded, called from either linker or compiler
+        EnumMap<Format, Map<Constant, Constant>> map = m_mapConstants;
+        if (map.isEmpty())
+            {
+            ensureLookup();
+            }
+        return map.get(format);
         }
 
     /**
@@ -3127,10 +3134,18 @@ private Map<Constant, Constant> ensureConstantLookup(Format format)
      *
      * @return the map from locator to Constant
      */
-     private synchronized Map<Object, Constant> ensureLocatorLookup(Format format)
+     private Map<Object, Constant> ensureLocatorLookup(Format format)
         {
-        // lazily instantiate the locator map for the specified type
-        return m_mapLocators.computeIfAbsent(format, _format -> new ConcurrentHashMap<>());
+        Map<Object, Constant> map = m_mapLocators.get(format);
+        if (map == null)
+            {
+            // m_mapLocators is an EnumMap, which is not thread-safe
+            synchronized (this)
+                {
+                map = m_mapLocators.computeIfAbsent(format, _format -> new ConcurrentHashMap<>());
+                }
+            }
+        return map;
         }
 
     /**