Merge pull request #4886 from jruby/improve_invalidation

Improvements to subclass management and method cache invalidation
jruby · Jan 10, 2018 · 56db7a9 · 56db7a9
2 parents 2a26055 + 9112beb
commit 56db7a9
Showing 2 changed files with 67 additions and 46 deletions.
diff --git a/core/src/main/java/org/jruby/RubyClass.java b/core/src/main/java/org/jruby/RubyClass.java
@@ -1097,19 +1097,27 @@ public final Collection<RubyClass> subclasses() {
         return subclasses(false);
     }
 
-    public synchronized Collection<RubyClass> subclasses(boolean includeDescendants) {
+    public Collection<RubyClass> subclasses(boolean includeDescendants) {
         Set<RubyClass> subclasses = this.subclasses;
         if (subclasses != null) {
-            Collection<RubyClass> mine = new ArrayList<>(subclasses);
+            Collection<RubyClass> mine = new ArrayList<>();
+            subclassesInner(mine, includeDescendants);
+
+            return mine;
+        }
+        return Collections.EMPTY_LIST;
+    }
+
+    private void subclassesInner(Collection<RubyClass> mine, boolean includeDescendants) {
+        Set<RubyClass> subclasses = this.subclasses;
+        if (subclasses != null) {
+            mine.addAll(subclasses);
             if (includeDescendants) {
                 for (RubyClass klass: subclasses) {
-                    mine.addAll(klass.subclasses(includeDescendants));
+                    klass.subclassesInner(mine, includeDescendants);
                 }
             }
-
-            return mine;
         }
-        return Collections.EMPTY_LIST;
     }
 
     /**
@@ -1121,26 +1129,31 @@ public synchronized Collection<RubyClass> subclasses(boolean includeDescendants)
      *
      * @param subclass The subclass to add
      */
-    public synchronized void addSubclass(RubyClass subclass) {
-        synchronized (runtime.getHierarchyLock()) {
-            Set<RubyClass> subclasses = this.subclasses;
-            if (subclasses == null) this.subclasses = subclasses = new WeakHashSet<>(4);
-            subclasses.add(subclass);
+    public void addSubclass(RubyClass subclass) {
+        Set<RubyClass> subclasses = this.subclasses;
+        if (subclasses == null) {
+            // check again
+            synchronized (this) {
+                subclasses = this.subclasses;
+                if (subclasses == null) {
+                    this.subclasses = subclasses = Collections.synchronizedSet(new WeakHashSet<RubyClass>(4));
+                }
+            }
         }
+
+        subclasses.add(subclass);
     }
 
     /**
      * Remove a subclass from the weak set of subclasses.
      *
      * @param subclass The subclass to remove
      */
-    public synchronized void removeSubclass(RubyClass subclass) {
-        synchronized (runtime.getHierarchyLock()) {
-            Set<RubyClass> subclasses = this.subclasses;
-            if (subclasses == null) return;
+    public void removeSubclass(RubyClass subclass) {
+        Set<RubyClass> subclasses = this.subclasses;
+        if (subclasses == null) return;
 
-            subclasses.remove(subclass);
-        }
+        subclasses.remove(subclass);
     }
 
     /**
@@ -1149,25 +1162,21 @@ public synchronized void removeSubclass(RubyClass subclass) {
      * @param subclass The subclass to remove
      * @param newSubclass The subclass to replace it with
      */
-    public synchronized void replaceSubclass(RubyClass subclass, RubyClass newSubclass) {
-        synchronized (runtime.getHierarchyLock()) {
-            Set<RubyClass> subclasses = this.subclasses;
-            if (subclasses == null) return;
+    public void replaceSubclass(RubyClass subclass, RubyClass newSubclass) {
+        Set<RubyClass> subclasses = this.subclasses;
+        if (subclasses == null) return;
 
-            subclasses.remove(subclass);
-            subclasses.add(newSubclass);
-        }
+        subclasses.remove(subclass);
+        subclasses.add(newSubclass);
     }
 
     @Override
     public void becomeSynchronized() {
         // make this class and all subclasses sync
-        synchronized (runtime.getHierarchyLock()) {
-            super.becomeSynchronized();
-            Set<RubyClass> subclasses = this.subclasses;
-            if (subclasses != null) {
-                for (RubyClass subclass : subclasses) subclass.becomeSynchronized();
-            }
+        super.becomeSynchronized();
+        Set<RubyClass> subclasses = this.subclasses;
+        if (subclasses != null) {
+            for (RubyClass subclass : subclasses) subclass.becomeSynchronized();
         }
     }
 
@@ -1187,11 +1196,9 @@ public void becomeSynchronized() {
     public void invalidateCacheDescendants() {
         super.invalidateCacheDescendants();
 
-        synchronized (runtime.getHierarchyLock()) {
-            Set<RubyClass> subclasses = this.subclasses;
-            if (subclasses != null) {
-                for (RubyClass subclass : subclasses) subclass.invalidateCacheDescendants();
-            }
+        Set<RubyClass> subclasses = this.subclasses;
+        if (subclasses != null) {
+            for (RubyClass subclass : subclasses) subclass.invalidateCacheDescendants();
         }
     }
 
@@ -1207,12 +1214,7 @@ public void addInvalidatorsAndFlush(List<Invalidator> invalidators) {
         if (subclasses == null || subclasses.isEmpty()) return;
 
         // cascade into subclasses
-        synchronized (runtime.getHierarchyLock()) {
-            subclasses = this.subclasses;
-            if (subclasses != null) {
-                for (RubyClass subclass : subclasses) subclass.addInvalidatorsAndFlush(invalidators);
-            }
-        }
+        for (RubyClass subclass : subclasses) subclass.addInvalidatorsAndFlush(invalidators);
     }
 
     public final Ruby getClassRuntime() {
@@ -2325,7 +2327,7 @@ public IRubyObject invoke(ThreadContext context, IRubyObject self, String name,
     protected final Ruby runtime;
     private ObjectAllocator allocator; // the default allocator
     protected ObjectMarshal marshal;
-    private Set<RubyClass> subclasses;
+    private volatile Set<RubyClass> subclasses;
     public static final int CS_IDX_INITIALIZE = 0;
     public enum CS_NAMES {
         INITIALIZE("initialize");

diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java
@@ -107,6 +107,7 @@
 import org.jruby.runtime.ivars.MethodData;
 import org.jruby.runtime.marshal.MarshalStream;
 import org.jruby.runtime.marshal.UnmarshalStream;
+import org.jruby.runtime.opto.ConstantInvalidator;
 import org.jruby.runtime.opto.Invalidator;
 import org.jruby.runtime.opto.OptoFactory;
 import org.jruby.runtime.profile.MethodEnhancer;
@@ -1356,13 +1357,18 @@ private CacheEntry cacheHit(String name) {
         return null;
     }
 
-    private void invalidateConstantCacheForModuleInclusion(RubyModule module)
-    {
+    private void invalidateConstantCacheForModuleInclusion(RubyModule module) {
+        Map<String, Invalidator> invalidators = null;
         for (RubyModule mod : gatherModules(module)) {
-            for (String key : mod.getConstantMap().keySet()) {
-                invalidateConstantCache(key);
+            for (String name : mod.getConstantMap().keySet()) {
+                if (invalidators == null) invalidators = new HashMap<>();
+                invalidators.put(name, getRuntime().getConstantInvalidator(name));
             }
         }
+        if (invalidators != null) {
+            List<Invalidator> values = new ArrayList(invalidators.values());
+            values.get(0).invalidateAll(values);
+        }
     }
 
     protected static abstract class CacheEntryFactory {
@@ -1559,6 +1565,19 @@ protected void invalidateConstantCache(String constantName) {
         getRuntime().getConstantInvalidator(constantName).invalidate();
     }
 
+    protected void invalidateConstantCaches(Set<String> constantNames) {
+        if (constantNames.size() > 0) {
+            Ruby runtime = getRuntime();
+
+            List<Invalidator> constantInvalidators = new ArrayList<>(constantNames.size());
+            for (String name : constantNames) {
+                constantInvalidators.add(runtime.getConstantInvalidator(name));
+            }
+
+            constantInvalidators.get(0).invalidateAll(constantInvalidators);
+        }
+    }
+
     /**
      * Search through this module and supermodules for method definitions. Cache superclass definitions in this class.
      *