Improvements for singletons and .extend, for e.g. DCI.

DCI is some new-fangled pattern whereby you .extend functionality into objects rather than creating objects of a specific type to begin with. Regardless of the merits, there are serious performance implications from all that extending. In all Rubies, a singleton class and one or more module wrapper classes must be created. In MRI and Rubinius, all call sites must be flushed since the class hierarchy has changed in a drastic way. In JRuby, call sites that encounter singletons will not preserve caching from object to object. These are changes to ameliorate the cost of creating singleton classes and extending modules into them. * When creating a singleton, make it look like its parent for invalidation purposes * When including a module, reuse existing wrapper for the same parent/module relationship * When including a module, if child invalidation artifacts match parent, make child match wrapper The result of the changes is that unmodified singleton classes look identical to their parents, and unmodified singleton classes that have the same ancestors (modules or otherwise) look the same for invalidation purposes. This allows call sites to see calls against different singletons that reach the same methods as truly monomorphic, avoiding polymorphic call site effects. The improvement is pretty drastic, even in a small benchmark. Benchmark notes: * sclass with no calls just does class << obj; end * sclass with 10 calls does the above and calls 10 different methods on that object * extend with no calls just does obj.extend(Foo) * extend with 10 calls does the above and calls 10 different methods on that object * The numbers are highly variable due to heavy object churn; the singleton class creation is still a large, unavoidable cost * The numbers do not reflect longer-lived singleton objects that may travel far and wide BEFORE: user system total real sclass with no calls 0.156000 0.000000 0.156000 ( 0.156000) sclass with 10 calls 0.369000 0.000000 0.369000 ( 0.369000) extend with no calls 0.312000 0.000000 0.312000 ( 0.312000) extend with 10 calls 0.510000 0.000000 0.510000 ( 0.510000) user system total real sclass with no calls 0.108000 0.000000 0.108000 ( 0.109000) sclass with 10 calls 0.365000 0.000000 0.365000 ( 0.365000) extend with no calls 0.270000 0.000000 0.270000 ( 0.270000) extend with 10 calls 0.470000 0.000000 0.470000 ( 0.470000) AFTER: user system total real sclass with no calls 0.150000 0.000000 0.150000 ( 0.150000) sclass with 10 calls 0.169000 0.000000 0.169000 ( 0.169000) extend with no calls 0.209000 0.000000 0.209000 ( 0.209000) extend with 10 calls 0.227000 0.000000 0.227000 ( 0.227000) user system total real sclass with no calls 0.152000 0.000000 0.152000 ( 0.152000) sclass with 10 calls 0.132000 0.000000 0.132000 ( 0.132000) extend with no calls 0.252000 0.000000 0.252000 ( 0.252000) extend with 10 calls 0.238000 0.000000 0.238000 ( 0.238000) Conflicts: src/org/jruby/MetaClass.java src/org/jruby/RubyClass.java src/org/jruby/RubyModule.java
jruby · Nov 27, 2013 · 2737669 · 2737669
1 parent 54605a0
commit 2737669
Showing 9 changed files with 90 additions and 43 deletions.
diff --git a/core/src/main/java/org/jruby/IncludedModuleWrapper.java b/core/src/main/java/org/jruby/IncludedModuleWrapper.java
@@ -60,25 +60,6 @@ public IncludedModuleWrapper(Ruby runtime, RubyClass superClass, RubyModule dele
         delegate.addIncludingHierarchy(this);
     }
 
-    /**
-     * Overridden newIncludeClass implementation to allow attaching future includes to the correct module
-     * (i.e. the one to which this is attached)
-     * 
-     * @see org.jruby.RubyModule#newIncludeClass(RubyClass)
-     */
-    @Override
-    @Deprecated
-    public IncludedModuleWrapper newIncludeClass(RubyClass superClass) {
-        IncludedModuleWrapper includedModule = new IncludedModuleWrapper(getRuntime(), superClass, getNonIncludedClass());
-
-        // include its parent (and in turn that module's parents)
-        if (getSuperClass() != null) {
-            includedModule.includeModule(getSuperClass());
-        }
-
-        return includedModule;
-    }
-
     @Override
     public boolean isModule() {
         return false;

diff --git a/core/src/main/java/org/jruby/RubyBasicObject.java b/core/src/main/java/org/jruby/RubyBasicObject.java
@@ -538,7 +538,9 @@ public RubyClass getSingletonClass() {
      * class.
      */
     public RubyClass makeMetaClass(RubyClass superClass) {
+        MetaClass cachedClass = superClass.getCachedSingletonClass();
         MetaClass klass = new MetaClass(getRuntime(), superClass, this); // rb_class_boot
+        klass.invalidateLike(cachedClass);
         setMetaClass(klass);
 
         klass.setMetaClass(superClass.getRealClass().getMetaClass());

diff --git a/core/src/main/java/org/jruby/RubyClass.java b/core/src/main/java/org/jruby/RubyClass.java
@@ -41,6 +41,7 @@
 import static org.objectweb.asm.Opcodes.ACC_VARARGS;
 
 import java.io.IOException;
+import java.lang.ref.WeakReference;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
@@ -1843,6 +1844,15 @@ public IRubyObject smartLoadOldUser(IRubyObject data) {
         }
     }
 
+    public MetaClass getCachedSingletonClass() {
+         MetaClass cached = cachedSingletonClass;
+        if (cached == null || cached.generation != generation) {
+            cached = cachedSingletonClass = new MetaClass(getClassRuntime(), this, null);
+            cached.invalidateLike(this);
+        }
+        return cached;
+    }
+
     protected final Ruby runtime;
     private ObjectAllocator allocator; // the default allocator
     protected ObjectMarshal marshal;
@@ -1885,4 +1895,6 @@ public IRubyObject smartLoadOldUser(IRubyObject data) {
 
     /** Variable table manager for this class */
     private final VariableTableManager variableTableManager;
+
+    private MetaClass cachedSingletonClass;
 }
diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java
@@ -55,6 +55,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.WeakHashMap;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
 
@@ -524,24 +525,6 @@ private String calculateAnonymousName() {
         return anonymousName;
     }
 
-    /**
-     * Create a wrapper to use for including the specified module into this one.
-     * 
-     * Ruby C equivalent = "include_class_new"
-     * 
-     * @return The module wrapper
-     */
-    @Deprecated
-    public IncludedModuleWrapper newIncludeClass(RubyClass superClazz) {
-        IncludedModuleWrapper includedModule = new IncludedModuleWrapper(getRuntime(), superClazz, this);
-
-        // include its parent (and in turn that module's parents)
-        if (getSuperClass() != null) {
-            includedModule.includeModule(getSuperClass());
-        }
-
-        return includedModule;
-    }
     /**
      * Finds a class that is within the current module (or class).
      * 
@@ -584,10 +567,27 @@ public synchronized void includeModule(IRubyObject arg) {
         infectBy(module);
 
         doIncludeModule(module);
-        invalidateCoreClasses();
-        invalidateCacheDescendants();
+
+        // If this is a singleton that has no subclasses and no changes compared to parent,
+        // just mimic the new parent.
+        // The subclasses check is for metaclasses, which can have subclasses. Object-level
+        // singleton classes never have subclasses.
+        if (this.isSingleton() &&
+                ((RubyClass)this).subclasses(false).isEmpty() &&
+                generation == superClass.superClass.generation) {
+            invalidateLike(superClass);
+        } else {
+            invalidateCoreClasses();
+            invalidateCacheDescendants();
+        }
         invalidateConstantCacheForModuleInclusion(module);
     }
+
+    public void defineMethod(String name, Callback method) {
+        Visibility visibility = name.equals("initialize") ?
+                PRIVATE : PUBLIC;
+        addMethod(name, new FullFunctionCallbackMethod(this, method, visibility));
+    }
 
     public void defineAnnotatedMethod(Class clazz, String name) {
         // FIXME: This is probably not very efficient, since it loads all methods for each call
@@ -2396,12 +2396,26 @@ private RubyModule proceedWithInclude(RubyModule insertAbove, RubyModule moduleT
         // In the current logic, if we get here we know that module is not an
         // IncludedModuleWrapper, so there's no need to fish out the delegate. But just
         // in case the logic should change later, let's do it anyway
-        RubyClass wrapper = new IncludedModuleWrapper(getRuntime(), insertAbove.getSuperClass(), moduleToInclude.getNonIncludedClass());
+
+        // The caching here will reuse an existing wrapper for the same parent/module
+        // combination. This makes obj.extend nearly free, other than the initial
+        // cost of creating the singleton, and reduces the call site penalty for
+        // extended singletons.
+        IncludedModuleWrapper wrapper = moduleToInclude.cachedWrappers.get(insertAbove.getSuperClass());
+        if (wrapper == null) {
+            if (false) {
+                String name = "<top>";
+                if (insertAbove.getSuperClass() != null) name = insertAbove.getSuperClass().getBaseName();
+                System.out.println("creating new wrapper for " + moduleToInclude.getBaseName() + " below " + name);
+            }
+            wrapper = new IncludedModuleWrapper(getRuntime(), insertAbove.getSuperClass(), moduleToInclude.getNonIncludedClass());
+            moduleToInclude.cachedWrappers.put(insertAbove.getSuperClass(), wrapper);
+        }
 
         // if the insertion point is a class, update subclass lists
         if (insertAbove instanceof RubyClass) {
             RubyClass insertAboveClass = (RubyClass)insertAbove;
-            
+
             // if there's a non-null superclass, we're including into a normal class hierarchy;
             // update subclass relationships to avoid stale parent/child relationships
             if (insertAboveClass.getSuperClass() != null) {
@@ -2410,7 +2424,7 @@ private RubyModule proceedWithInclude(RubyModule insertAbove, RubyModule moduleT
 
             wrapper.addSubclass(insertAboveClass);
         }
-        
+
         insertAbove.setSuperClass(wrapper);
         insertAbove = insertAbove.getSuperClass();
         return insertAbove;
@@ -3915,6 +3929,17 @@ public void defineFastPublicModuleFunction(String name, org.jruby.runtime.callba
         getSingletonClass().defineFastMethod(name, method);
     }
 
+    /**
+     * Use generation and invalidator from the given module, to appear identical to
+     * it until physically modified.
+     *
+     * @param module the module from which to copy invalidation artifacts
+     */
+    public void invalidateLike(RubyModule module) {
+        generation = module.generation;
+        methodInvalidator.lookLike(module.methodInvalidator);
+    }
+
     private volatile Map<String, Autoload> autoloads = Collections.EMPTY_MAP;
     private volatile Map<String, DynamicMethod> methods = Collections.EMPTY_MAP;
     protected Map<String, CacheEntry> cachedMethods = Collections.EMPTY_MAP;
@@ -3961,8 +3986,10 @@ public void defineFastPublicModuleFunction(String name, org.jruby.runtime.callba
     }
 
     // Invalidator used for method caches
-    protected final Invalidator methodInvalidator;
+    protected Invalidator methodInvalidator;
 
     /** Whether this class proxies a normal Java class */
     private boolean javaProxy = false;
+
+    private WeakHashMap<RubyClass, IncludedModuleWrapper> cachedWrappers = new WeakHashMap<RubyClass, IncludedModuleWrapper>();
 }
diff --git a/core/src/main/java/org/jruby/runtime/opto/GenerationAndSwitchPointInvalidator.java b/core/src/main/java/org/jruby/runtime/opto/GenerationAndSwitchPointInvalidator.java
@@ -35,4 +35,10 @@ public void invalidateAll(List<Invalidator> invalidators) {
     public Object getData() {
         return switchPointInvalidator.getData();
     }
+
+    public void lookLike(Invalidator invalidator) {
+        if (invalidator instanceof GenerationAndSwitchPointInvalidator) {
+            switchPointInvalidator.lookLike(((GenerationAndSwitchPointInvalidator)invalidator).switchPointInvalidator);
+        }
+    }
 }
diff --git a/core/src/main/java/org/jruby/runtime/opto/GenerationInvalidator.java b/core/src/main/java/org/jruby/runtime/opto/GenerationInvalidator.java
@@ -22,5 +22,11 @@ public void invalidateAll(List<Invalidator> invalidators) {
     public Object getData() {
         return module.getGenerationObject();
     }
+
+    public void lookLike(Invalidator invalidator) {
+        if (invalidator instanceof GenerationInvalidator) {
+            // do nothing; gneration should have been copied elsewhere
+        }
+    }
 
 }
diff --git a/core/src/main/java/org/jruby/runtime/opto/Invalidator.java b/core/src/main/java/org/jruby/runtime/opto/Invalidator.java
@@ -42,4 +42,5 @@ public interface Invalidator {
     public void invalidate();
     public void invalidateAll(List<Invalidator> invalidators);
     public Object getData();
+    public void lookLike(Invalidator invalidator);
 }
diff --git a/core/src/main/java/org/jruby/runtime/opto/ObjectIdentityInvalidator.java b/core/src/main/java/org/jruby/runtime/opto/ObjectIdentityInvalidator.java
@@ -48,5 +48,11 @@ public void invalidateAll(List<Invalidator> invalidators) {
     public Object getData() {
         return generation;
     }
+
+    public void lookLike(Invalidator invalidator) {
+        if (invalidator instanceof ObjectIdentityInvalidator) {
+            generation = ((ObjectIdentityInvalidator)invalidator).generation;
+        }
+    }
 
 }
diff --git a/core/src/main/java/org/jruby/runtime/opto/SwitchPointInvalidator.java b/core/src/main/java/org/jruby/runtime/opto/SwitchPointInvalidator.java
@@ -66,4 +66,10 @@ public synchronized SwitchPoint replaceSwitchPoint() {
         switchPoint = new SwitchPoint();
         return oldSwitchPoint;
     }
+
+    public void lookLike(Invalidator invalidator) {
+        if (invalidator instanceof SwitchPointInvalidator) {
+            switchPoint = ((SwitchPointInvalidator)invalidator).switchPoint;
+        }
+    }
 }