8074556: Functions should not share allocator maps
Reviewed-by: lagergren, sundar
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/FindScopeDepths.java Fri Mar 06 15:26:51 2015 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/FindScopeDepths.java Mon Mar 09 11:34:48 2015 +0100
@@ -32,7 +32,6 @@
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
-import jdk.nashorn.internal.codegen.ObjectClassGenerator.AllocatorDescriptor;
import jdk.nashorn.internal.ir.Block;
import jdk.nashorn.internal.ir.FunctionNode;
import jdk.nashorn.internal.ir.FunctionNode.CompilationState;
@@ -208,7 +207,7 @@
final RecompilableScriptFunctionData data = new RecompilableScriptFunctionData(
newFunctionNode,
compiler.getCodeInstaller(),
- new AllocatorDescriptor(newFunctionNode.getThisProperties()),
+ ObjectClassGenerator.createAllocationStrategy(newFunctionNode.getThisProperties()),
nestedFunctions,
externalSymbolDepths.get(fnId),
internalSymbols.get(fnId),
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/ObjectClassGenerator.java Fri Mar 06 15:26:51 2015 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/ObjectClassGenerator.java Mon Mar 09 11:34:48 2015 +0100
@@ -56,6 +56,7 @@
import jdk.nashorn.internal.codegen.ClassEmitter.Flag;
import jdk.nashorn.internal.codegen.types.Type;
import jdk.nashorn.internal.runtime.AccessorProperty;
+import jdk.nashorn.internal.runtime.AllocationStrategy;
import jdk.nashorn.internal.runtime.Context;
import jdk.nashorn.internal.runtime.FunctionScope;
import jdk.nashorn.internal.runtime.JSType;
@@ -826,44 +827,15 @@
}
/**
- * Describes the allocator class name and property map for a constructor function with the specified
+ * Creates the allocator class name and property map for a constructor function with the specified
* number of "this" properties that it initializes.
- *
+ * @param thisProperties number of properties assigned to "this"
+ * @return the allocation strategy
*/
- public static class AllocatorDescriptor {
- private final String allocatorClassName;
- private final PropertyMap allocatorMap;
-
- /**
- * Creates a new allocator descriptor
- * @param thisProperties the number of "this" properties that the function initializes
- */
- public AllocatorDescriptor(final int thisProperties) {
- final int paddedFieldCount = getPaddedFieldCount(thisProperties);
- this.allocatorClassName = Compiler.binaryName(getClassName(paddedFieldCount));
- this.allocatorMap = PropertyMap.newMap(null, allocatorClassName, 0, paddedFieldCount, 0);
- }
-
- /**
- * Returns the name of the class that the function allocates
- * @return the name of the class that the function allocates
- */
- public String getAllocatorClassName() {
- return allocatorClassName;
- }
-
- /**
- * Returns the allocator map for the function.
- * @return the allocator map for the function.
- */
- public PropertyMap getAllocatorMap() {
- return allocatorMap;
- }
-
- @Override
- public String toString() {
- return "AllocatorDescriptor[allocatorClassName=" + allocatorClassName + ", allocatorMap.size=" +
- allocatorMap.size() + "]";
- }
+ 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);
}
}
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/AllocationStrategy.java Fri Mar 06 15:26:51 2015 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/AllocationStrategy.java Mon Mar 09 11:34:48 2015 +0100
@@ -30,22 +30,15 @@
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import jdk.nashorn.internal.codegen.CompilerConstants;
-import jdk.nashorn.internal.codegen.ObjectClassGenerator.AllocatorDescriptor;
/**
- * Encapsulates the allocation strategy for a function when used as a constructor. Basically the same as
- * {@link AllocatorDescriptor}, but with an additionally cached resolved method handle. There is also a
- * canonical default allocation strategy for functions that don't assign any "this" properties (vast majority
- * of all functions), therefore saving some storage space in {@link RecompilableScriptFunctionData} that would
- * otherwise be lost to identical tuples of (map, className, handle) fields.
+ * Encapsulates the allocation strategy for a function when used as a constructor.
*/
-final class AllocationStrategy implements Serializable {
+final public class AllocationStrategy implements Serializable {
private static final long serialVersionUID = 1L;
private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup();
- private static final AllocationStrategy DEFAULT_STRATEGY = new AllocationStrategy(new AllocatorDescriptor(0));
-
/** Allocator map from allocator descriptor */
private final PropertyMap allocatorMap;
@@ -55,19 +48,15 @@
/** lazily generated allocator */
private transient MethodHandle allocator;
- private AllocationStrategy(final AllocatorDescriptor desc) {
- this.allocatorMap = desc.getAllocatorMap();
+ /**
+ * Construct an allocation strategy with the given map and class name.
+ * @param allocatorMap the property map
+ * @param className the class name
+ */
+ 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 = desc.getAllocatorClassName().intern();
- }
-
- private boolean matches(final AllocatorDescriptor desc) {
- return desc.getAllocatorMap().size() == allocatorMap.size() &&
- desc.getAllocatorClassName().equals(allocatorClassName);
- }
-
- static AllocationStrategy get(final AllocatorDescriptor desc) {
- return DEFAULT_STRATEGY.matches(desc) ? DEFAULT_STRATEGY : new AllocationStrategy(desc);
+ this.allocatorClassName = className.intern();
}
PropertyMap getAllocatorMap() {
@@ -88,14 +77,6 @@
}
}
- private Object readResolve() {
- if(allocatorMap.size() == DEFAULT_STRATEGY.allocatorMap.size() &&
- allocatorClassName.equals(DEFAULT_STRATEGY.allocatorClassName)) {
- return DEFAULT_STRATEGY;
- }
- return this;
- }
-
@Override
public String toString() {
return "AllocationStrategy[allocatorClassName=" + allocatorClassName + ", allocatorMap.size=" +
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java Fri Mar 06 15:26:51 2015 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java Mon Mar 09 11:34:48 2015 +0100
@@ -43,7 +43,6 @@
import jdk.nashorn.internal.codegen.CompilerConstants;
import jdk.nashorn.internal.codegen.FunctionSignature;
import jdk.nashorn.internal.codegen.Namespace;
-import jdk.nashorn.internal.codegen.ObjectClassGenerator.AllocatorDescriptor;
import jdk.nashorn.internal.codegen.OptimisticTypesPersistence;
import jdk.nashorn.internal.codegen.TypeMap;
import jdk.nashorn.internal.codegen.types.Type;
@@ -126,7 +125,7 @@
*
* @param functionNode functionNode that represents this function code
* @param installer installer for code regeneration versions of this function
- * @param allocationDescriptor descriptor for the allocation behavior when this function is used as a constructor
+ * @param allocationStrategy strategy for the allocation behavior when this function is used as a constructor
* @param nestedFunctions nested function map
* @param externalScopeDepths external scope depths
* @param internalSymbols internal symbols to method, defined in its scope
@@ -135,7 +134,7 @@
public RecompilableScriptFunctionData(
final FunctionNode functionNode,
final CodeInstaller<ScriptEnvironment> installer,
- final AllocatorDescriptor allocationDescriptor,
+ final AllocationStrategy allocationStrategy,
final Map<Integer, RecompilableScriptFunctionData> nestedFunctions,
final Map<String, Integer> externalScopeDepths,
final Set<String> internalSymbols,
@@ -153,7 +152,7 @@
this.endParserState = functionNode.getEndParserState();
this.token = tokenFor(functionNode);
this.installer = installer;
- this.allocationStrategy = AllocationStrategy.get(allocationDescriptor);
+ this.allocationStrategy = allocationStrategy;
this.nestedFunctions = smallMap(nestedFunctions);
this.externalScopeDepths = smallMap(externalScopeDepths);
this.internalSymbols = smallSet(new HashSet<>(internalSymbols));
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8074556.js Mon Mar 09 11:34:48 2015 +0100
@@ -0,0 +1,52 @@
+/*
+ * 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-8074556: Functions should not share allocator maps
+ *
+ * @test
+ * @run
+ */
+
+function A () {
+ return this;
+}
+
+function B() {
+ return this;
+}
+
+A.prototype.x = "x";
+A.prototype.y = "y";
+B.prototype.y = "y"; // same properties but different order
+B.prototype.x = "x";
+
+function test(o) {
+ Assert.assertEquals(o.x, "x");
+ Assert.assertEquals(o.y, "y");
+}
+
+test(new A());
+test(new B());
+test(new A());
+test(new B());