Better cleanup of thread-safety

javatools/src/main/java/org/xvm/asm/Component.java

@@ -833,40 +833,67 @@ public synchronized Map<String, Component> ensureChildByNameMap()
     /**
      * Make sure that any deferred child deserialization is complete
      */
-    protected void ensureChildren() {
-        for (byte[] ab = m_abChildren; ab != null; ab = m_abChildren) {
-            synchronized (ab) { // sync on object shared by all siblings
-                if (ab.length == 0) {
+    protected void ensureChildren()
+        {
+        if (m_abChildren != null)
+            {
+            ensureChildrenComplex();
+            }
+        // else; common path
+        }
+
+    /**
+     * Complex portion of {@link #ensureChildren()} extracted for hot-spotting.
+     */
+    private void ensureChildrenComplex()
+        {
+        for (byte[] ab = m_abChildren; ab != null; ab = m_abChildren)
+            {
+            synchronized (ab) // sync on an object shared by all siblings
+                {
+                if (ab.length == 0)
+                    {
                     break; // we've recursed or the deserialization thread just completed
-                } else if (ab == m_abChildren) {
+                    }
+                else if (ab == m_abChildren)
+                    {
                     byte[] empty = new byte[0];
 
-                    synchronized (empty) { // other threads may sync and wait on this object from sync(ab) above
+                    synchronized (empty) // other threads may sync and wait on this object from sync(ab) above
+                        {
                         // first mark all siblings as in active serialization; this allows other threads to still sync
                         // and wait while ensuring that this thread doesn't endless recurse
-                        for (Iterator<Component> siblings = siblings(); siblings.hasNext(); ) {
+                        for (Iterator<Component> siblings = siblings(); siblings.hasNext(); )
+                            {
                             siblings.next().m_abChildren = empty;
-                        }
+                            }
 
                         // now read in the children
                         DataInput in = new DataInputStream(new ByteArrayInputStream(ab));
-                        try {
-                            disassembleChildren(in, false);
-                        } catch (IOException e) {
-                            throw new IllegalStateException("IOException occurred in " + getIdentityConstant()
-                                    + " during deferred read of child components", e);
-                        } finally {
-                            // finally 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(); ) {
+                        try
+                            {
+                            disassembleChildren(in, true);
+                            }
+                        catch (IOException e)
+                            {
+                            throw new IllegalStateException(
+                                    "IOException occurred in " + getIdentityConstant() + " " + "during"
+                                    + " deferred read of child components", e);
+                            }
+                        finally
+                            {
+                            // finally 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;
+                                }
                             }
                         }
                     }
                 }
             }
         }
-    }
 
     /**
      * Visitor pattern for children of this component, optionally including all siblings, and

javatools/src/main/java/org/xvm/asm/ConstantPool.java

@@ -2636,7 +2636,7 @@ protected void disassemble(DataInput in)
         {
         m_listConst.clear();
         m_mapConstants.clear();
-            m_mapLocators = new EnumMap<>(Format.class);
+        m_mapLocators = new EnumMap<>(Format.class); // cow "clear"
 
         // read the number of constants in the pool
         int cConst = readMagnitude(in);
@@ -3134,21 +3134,31 @@ private Map<Constant, Constant> ensureConstantLookup(Format format)
      *
      * @return the map from locator to Constant
      */
-    private Map<Object, Constant> ensureLocatorLookup(Format format) {
+    private Map<Object, Constant> ensureLocatorLookup(Format format)
+        {
         Map<Object, Constant> map = m_mapLocators.get(format);
-        if (map == null) {
-            // m_mapLocators is an EnumMap, which is not thread-safe; use copy-on-write
-            synchronized (this) {
-                map = m_mapLocators.get(format);
-                if (map == null) {
-                    EnumMap<Format, Map<Object, Constant>> mapNew = new EnumMap<>(m_mapLocators);
-                    mapNew.put(format, map = new ConcurrentHashMap<>());
-                    m_mapLocators = mapNew;
-                }
-            }
+        return map == null ? ensureLocatorLookupComplex(format) : map;
         }
+
+    /**
+     * Double check locking portion of {@link #ensureLocatorLookup(Format)}, extracted for hot-spotting.
+     *
+     * @param format the Constant Type
+     * @return the map from locator to Constant
+     */
+    private synchronized Map<Object, Constant> ensureLocatorLookupComplex(Format format)
+        {
+        // m_mapLocators is an EnumMap, which is not thread-safe; use copy-on-write
+        Map<Object, Constant> map = m_mapLocators.get(format);
+        if (map == null)
+            {
+            var mapLocNew = new EnumMap<>(m_mapLocators); // cow
+            mapLocNew.put(format, map = new ConcurrentHashMap<>());
+            m_mapLocators = mapLocNew;
+            }
+
         return map;
-    }
+        }
 
     /**
      * Create the necessary structures for looking up Constant objects quickly, and populate those
@@ -3845,12 +3855,12 @@ public static Auto withPool(ConstantPool pool) {
      */
     private final EnumMap<Format, Map<Constant, Constant>> m_mapConstants = new EnumMap<>(Format.class);
 
-        /**
-         * Reverse lookup structure to find a particular constant by locator.
-         * <p>
-         * This map is not thread-safe and safety is provided via copy-on-write
-         */
-        private volatile EnumMap<Format, Map<Object, Constant>> m_mapLocators = new EnumMap<>(Format.class);
+    /**
+     * Reverse lookup structure to find a particular constant by locator.
+     * <p>
+     * This map is not thread-safe and safety is provided via copy-on-write
+     */
+    private volatile EnumMap<Format, Map<Object, Constant>> m_mapLocators = new EnumMap<>(Format.class);
 
     /**
      * Set of references to ConstantPool instances, defining the only ConstantPool references that
@@ -4175,10 +4185,11 @@ private void optimize()
             }
 
         // discard any previous lookup structures, since contents may have changed
-            m_mapConstants.clear();
-            m_mapLocators = new EnumMap<>(Format.class);
-            f_implicits.clear();
+        m_mapConstants.clear();
+        m_mapLocators = new EnumMap<>(Format.class); // cow "clear"
+        f_implicits.clear();
         }
+
     private transient TypeConstant      m_typeRange;
     private transient TypeConstant      m_typeInterval;
     private transient TypeConstant      m_typeIterable;