8074693: Different instances of same function use same allocator map
authorhannesw
Wed, 11 Mar 2015 11:08:22 +0100
changeset 29411 79e9da5fffbc
parent 29410 cdfd8fbb2b1d
child 29412 6dfd487fbad4
8074693: Different instances of same function use same allocator map Reviewed-by: attila, lagergren
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/ObjectClassGenerator.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/ScriptFunctionImpl.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/AllocationStrategy.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptFunction.java
nashorn/test/script/basic/JDK-8074693.js
nashorn/test/script/basic/JDK-8074693.js.EXPECTED
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/ObjectClassGenerator.java	Wed Mar 11 11:03:21 2015 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/ObjectClassGenerator.java	Wed Mar 11 11:08:22 2015 +0100
@@ -834,8 +834,6 @@
      */
     static AllocationStrategy createAllocationStrategy(final int thisProperties) {
         final int paddedFieldCount = getPaddedFieldCount(thisProperties);
-        final String allocatorClassName = Compiler.binaryName(getClassName(paddedFieldCount));
-        final PropertyMap allocatorMap = PropertyMap.newMap(null, allocatorClassName, 0, paddedFieldCount, 0);
-        return new AllocationStrategy(allocatorMap, allocatorClassName);
+        return new AllocationStrategy(paddedFieldCount);
     }
 }
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/ScriptFunctionImpl.java	Wed Mar 11 11:03:21 2015 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/objects/ScriptFunctionImpl.java	Wed Mar 11 11:08:22 2015 +0100
@@ -305,7 +305,7 @@
         final ScriptFunction typeErrorThrower = global.getTypeErrorThrower();
         if (findProperty("arguments", true) != null) {
             initUserAccessors("arguments", Property.NOT_CONFIGURABLE | Property.NOT_ENUMERABLE, typeErrorThrower, typeErrorThrower);
-       }
+        }
         if (findProperty("caller", true) != null) {
             initUserAccessors("caller", Property.NOT_CONFIGURABLE | Property.NOT_ENUMERABLE, typeErrorThrower, typeErrorThrower);
        }
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/AllocationStrategy.java	Wed Mar 11 11:03:21 2015 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/AllocationStrategy.java	Wed Mar 11 11:08:22 2015 +0100
@@ -29,7 +29,9 @@
 import java.io.Serializable;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
+import jdk.nashorn.internal.codegen.Compiler;
 import jdk.nashorn.internal.codegen.CompilerConstants;
+import jdk.nashorn.internal.codegen.ObjectClassGenerator;
 
 /**
  * Encapsulates the allocation strategy for a function when used as a constructor.
@@ -39,34 +41,40 @@
 
     private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup();
 
-    /** Allocator map from allocator descriptor */
-    private final PropertyMap allocatorMap;
+    /** Number of fields in the allocated object */
+    private final int fieldCount;
 
     /** Name of class where allocator function resides */
-    private final String allocatorClassName;
+    private transient String allocatorClassName;
 
     /** lazily generated allocator */
     private transient MethodHandle allocator;
 
     /**
      * Construct an allocation strategy with the given map and class name.
-     * @param allocatorMap the property map
-     * @param className the class name
+     * @param fieldCount number of fields in the allocated object
      */
-    public AllocationStrategy(final PropertyMap allocatorMap, final String className) {
-        this.allocatorMap = allocatorMap;
-        // These classes get loaded, so an interned variant of their name is most likely around anyway.
-        this.allocatorClassName = className.intern();
+    public AllocationStrategy(final int fieldCount) {
+        this.fieldCount = fieldCount;
+    }
+
+    private String getAllocatorClassName() {
+        if (allocatorClassName == null) {
+            // These classes get loaded, so an interned variant of their name is most likely around anyway.
+            allocatorClassName = Compiler.binaryName(ObjectClassGenerator.getClassName(fieldCount)).intern();
+        }
+        return allocatorClassName;
     }
 
     PropertyMap getAllocatorMap() {
-        return allocatorMap;
+        // Create a new map for each function instance
+        return PropertyMap.newMap(null, getAllocatorClassName(), 0, fieldCount, 0);
     }
 
     ScriptObject allocate(final PropertyMap map) {
         try {
             if (allocator == null) {
-                allocator = MH.findStatic(LOOKUP, Context.forStructureClass(allocatorClassName),
+                allocator = MH.findStatic(LOOKUP, Context.forStructureClass(getAllocatorClassName()),
                         CompilerConstants.ALLOCATE.symbolName(), MH.type(ScriptObject.class, PropertyMap.class));
             }
             return (ScriptObject)allocator.invokeExact(map);
@@ -79,7 +87,6 @@
 
     @Override
     public String toString() {
-        return "AllocationStrategy[allocatorClassName=" + allocatorClassName + ", allocatorMap.size=" +
-                allocatorMap.size() + "]";
+        return "AllocationStrategy[fieldCount=" + fieldCount + "]";
     }
 }
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptFunction.java	Wed Mar 11 11:03:21 2015 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptFunction.java	Wed Mar 11 11:08:22 2015 +0100
@@ -143,7 +143,6 @@
 
         this.data  = data;
         this.scope = scope;
-        this.allocatorMap = data.getAllocatorMap();
     }
 
     @Override
@@ -253,7 +252,7 @@
 
         assert !isBoundFunction(); // allocate never invoked on bound functions
 
-        final ScriptObject object = data.allocate(allocatorMap);
+        final ScriptObject object = data.allocate(getAllocatorMap());
 
         if (object != null) {
             final Object prototype = getPrototype();
@@ -269,6 +268,13 @@
         return object;
     }
 
+    private PropertyMap getAllocatorMap() {
+        if (allocatorMap == null) {
+            allocatorMap = data.getAllocatorMap();
+        }
+        return allocatorMap;
+    }
+
     /**
      * Return Object.prototype - used by "allocate"
      * @return Object.prototype
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8074693.js	Wed Mar 11 11:08:22 2015 +0100
@@ -0,0 +1,74 @@
+/*
+ * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ * 
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ * 
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ * 
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ * 
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * JDK-8074693: Different instances of same function use same allocator map
+ *
+ * @test
+ * @run
+ */
+
+var lib = {};
+
+lib.mixin = function(target, source) {
+    for (var p in source) {
+        if (source.hasOwnProperty(p) && !target.hasOwnProperty(p)) {
+            target.prototype[p] = source[p];
+        }
+    }
+};
+
+lib.declare = function(def) {
+    var className = def.name;
+
+    lib[className] = function() {
+        this.init.apply(this, arguments);
+    };
+
+    lib.mixin(lib[className], def.members);
+};
+
+
+lib.declare({
+    name: "ClassA",
+    members: {
+        init : function () {
+            print("init A called");
+        }
+    }
+});
+
+lib.declare({
+    name: "ClassB",
+    members: {
+        util : function () {
+            print("util called")
+        },
+        init : function() {
+            print("init B called");
+        }
+    }
+});
+
+var objA = new lib.ClassA();
+var objB = new lib.ClassB();
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8074693.js.EXPECTED	Wed Mar 11 11:08:22 2015 +0100
@@ -0,0 +1,2 @@
+init A called
+init B called