Improvements to instance variable shaping

core/src/main/java/org/jruby/RubyBasicObject.java

@@ -466,6 +466,10 @@ public static RubyClass getMetaClass(IRubyObject arg) {
         return ((RubyBasicObject) arg).metaClass;
     }
 
+    public VariableTableManager getShape() {
+        return metaClass.getVariableTableManager();
+    }
+
     /** rb_singleton_class
      *
      * Note: this method is specialized for RubyFixnum, RubySymbol,
@@ -1057,7 +1061,7 @@ public IRubyObject id() {
      * object is frozen.
      */
     protected long getObjectId() {
-        return metaClass.getRealClass().getVariableTableManager().getObjectId(this);
+        return getShape().getObjectId(this);
     }
 
     /** rb_obj_inspect
@@ -1135,7 +1139,7 @@ private RubyString inspectObj(final Ruby runtime, RubyString part) {
         final ThreadContext context = runtime.getCurrentContext();
 
         boolean first = true;
-        for (Map.Entry<String, VariableAccessor> entry : metaClass.getVariableTableManager().getVariableAccessorsForRead().entrySet()) {
+        for (Map.Entry<String, VariableAccessor> entry : getShape().getVariableAccessorsForRead().entrySet()) {
             Object value = entry.getValue().get(this);
             if (!(value instanceof IRubyObject)) continue;
             RubySymbol symbol = runtime.newSymbol(entry.getKey());
@@ -1309,15 +1313,15 @@ public Object getVariable(int index) {
     public void setVariable(int index, Object value) {
         ensureInstanceVariablesSettable();
         if (index < 0) return;
-        metaClass.getVariableTableManager().setVariableInternal(this, index, value);
+        getShape().setVariableInternal(this, index, value);
     }
 
     public final Object getFFIHandle() {
-        return metaClass.getVariableTableManager().getFFIHandle(this);
+        return getShape().getFFIHandle(this);
     }
 
     public final void setFFIHandle(Object value) {
-        metaClass.getVariableTableManager().setFFIHandle(this, value);
+        getShape().setFFIHandle(this, value);
     }
 
     //
@@ -1331,7 +1335,7 @@ public final void setFFIHandle(Object value) {
      */
     @Override
     public boolean hasVariables() {
-        return metaClass.getVariableTableManager().hasVariables(this);
+        return getShape().hasVariables(this);
     }
 
     /**
@@ -1341,7 +1345,7 @@ public boolean hasVariables() {
      * @return true if there are set instance variables, false otherwise
      */
     protected boolean hasInstanceVariables() {
-        return metaClass.getVariableTableManager().hasInstanceVariables(this);
+        return getShape().hasInstanceVariables(this);
     }
 
     /**
@@ -1418,7 +1422,7 @@ protected Object variableTableStore(String name, Object value) {
      * table, and returning the removed value.
      */
     protected Object variableTableRemove(String name) {
-        return metaClass.getVariableTableManager().clearVariable(this, name);
+        return getShape().clearVariable(this, name);
     }
 
     /**
@@ -1489,7 +1493,7 @@ public Object removeInternalVariable(String name) {
      */
     @Override
     public void syncVariables(IRubyObject other) {
-        metaClass.getVariableTableManager().syncVariables(this, other);
+        getShape().syncVariables(this, other);
     }
 
     //
@@ -2881,7 +2885,7 @@ private void writeObject(ObjectOutputStream oos) throws IOException {
         oos.defaultWriteObject();
         oos.writeUTF(metaClass.getName());
 
-        metaClass.getVariableTableManager().serializeVariables(this, oos);
+        getShape().serializeVariables(this, oos);
     }
 
     /**
@@ -2907,7 +2911,7 @@ private void readObject(ObjectInputStream ois) throws IOException, ClassNotFound
         ois.defaultReadObject();
         metaClass = (RubyClass)ruby.getClassFromPath(ois.readUTF());
 
-        metaClass.getVariableTableManager().deserializeVariables(this, ois);
+        getShape().deserializeVariables(this, ois);
     }
 
     /**

core/src/main/java/org/jruby/RubyModule.java

@@ -52,9 +52,11 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.IdentityHashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.TreeSet;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
 
@@ -6036,15 +6038,28 @@ public <T> T toJava(Class<T> target) {
     }
 
     public Set<String> discoverInstanceVariables() {
-        HashSet<String> set = new HashSet();
-        RubyModule cls = this;
-        while (cls != null) {
-            Map<String, DynamicMethod> methods = cls.getOrigin().getMethodLocation().getMethods();
+        // preserve insertion order, so parent variables always precede child variables
+        return discoverInstanceVariablesInner(new LinkedHashSet<>());
+    }
+
+    protected Set<String> discoverInstanceVariablesInner(Set<String> set) {
+        // recurse to superclasses so they get first go at the table layout
+        RubyClass superClass = getSuperClass();
+        if (superClass != null) {
+            superClass.discoverInstanceVariablesInner(set);
+        }
 
-            methods.forEach((name, method) -> set.addAll(method.getInstanceVariableNames()));
+        Map<String, DynamicMethod> methods = getOrigin().getMethodLocation().getMethods();
 
-            cls = cls.getSuperClass();
+        // gather vars for this class and sort alphabetically
+        TreeSet<String> sorted = new TreeSet<>();
+        for (Map.Entry<String, DynamicMethod> entry : methods.entrySet()) {
+            sorted.addAll(entry.getValue().getInstanceVariableNames());
         }
+
+        // add all new vars from this class to the set
+        set.addAll(sorted);
+
         return set;
     }
 

core/src/main/java/org/jruby/RubyObjectShaped.java

@@ -0,0 +1,58 @@
+/*
+ ***** BEGIN LICENSE BLOCK *****
+ * Version: EPL 2.0/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Eclipse Public
+ * License Version 2.0 (the "License"); you may not use this file
+ * except in compliance with the License. You may obtain a copy of
+ * the License at http://www.eclipse.org/legal/epl-v20.html
+ *
+ * Software distributed under the License is distributed on an "AS
+ * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+ * implied. See the License for the specific language governing
+ * rights and limitations under the License.
+ *
+ * Copyright (C) 2001 Chad Fowler <chadfowler@chadfowler.com>
+ * Copyright (C) 2001 Alan Moore <alan_moore@gmx.net>
+ * Copyright (C) 2001-2002 Benoit Cerrina <b.cerrina@wanadoo.fr>
+ * Copyright (C) 2001-2004 Jan Arne Petersen <jpetersen@uni-bonn.de>
+ * Copyright (C) 2002-2004 Anders Bengtsson <ndrsbngtssn@yahoo.se>
+ * Copyright (C) 2004-2006 Thomas E Enebo <enebo@acm.org>
+ * Copyright (C) 2004-2005 Charles O Nutter <headius@headius.com>
+ * Copyright (C) 2004 Stefan Matthias Aust <sma@3plus4.de>
+ * Copyright (C) 2006 Ola Bini <ola.bini@ki.se>
+ * Copyright (C) 2006 Miguel Covarrubias <mlcovarrubias@gmail.com>
+ * Copyright (C) 2007 MenTaLguY <mental@rydia.net>
+ * Copyright (C) 2007 William N Dortch <bill.dortch@gmail.com>
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either of the GNU General Public License Version 2 or later (the "GPL"),
+ * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the EPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the EPL, the GPL or the LGPL.
+ ***** END LICENSE BLOCK *****/
+
+package org.jruby;
+
+import org.jruby.runtime.ivars.VariableTableManager;
+
+public abstract class RubyObjectShaped extends RubyObject {
+    public RubyObjectShaped(Ruby runtime, RubyClass metaClass) {
+        super(runtime, metaClass);
+
+        variableTableManager = metaClass.getVariableTableManager();
+    }
+
+    @Override
+    public VariableTableManager getShape() {
+        return variableTableManager;
+    }
+
+    private final VariableTableManager variableTableManager;
+}

core/src/main/java/org/jruby/specialized/RubyObjectSpecializer.java

@@ -32,6 +32,7 @@
 import org.jruby.Ruby;
 import org.jruby.RubyClass;
 import org.jruby.RubyObject;
+import org.jruby.RubyObjectShaped;
 import org.jruby.runtime.Helpers;
 import org.jruby.runtime.ObjectAllocator;
 import org.jruby.runtime.builtin.IRubyObject;
@@ -157,7 +158,7 @@ public static ObjectAllocator specializeForVariables(RubyClass klass, Set<String
 
     private static Class generateInternal(int size, final String clsPath) {
         // ensure only one thread will attempt to generate and define the new class
-        final String baseName = p(RubyObject.class);
+        final String baseName = p(RubyObjectShaped.class);
 
         final JiteClass jiteClass = new JiteClass(clsPath, baseName, new String[0]) {{
             for (int i = 0; i < size; i++) {