Mon, 09 Mar 2015 11:34:48 +0100
8074556: Functions should not share allocator maps
Reviewed-by: lagergren, sundar
1.1 --- a/src/jdk/nashorn/internal/codegen/FindScopeDepths.java Fri Mar 06 15:26:51 2015 +0100 1.2 +++ b/src/jdk/nashorn/internal/codegen/FindScopeDepths.java Mon Mar 09 11:34:48 2015 +0100 1.3 @@ -32,7 +32,6 @@ 1.4 import java.util.Iterator; 1.5 import java.util.Map; 1.6 import java.util.Set; 1.7 -import jdk.nashorn.internal.codegen.ObjectClassGenerator.AllocatorDescriptor; 1.8 import jdk.nashorn.internal.ir.Block; 1.9 import jdk.nashorn.internal.ir.FunctionNode; 1.10 import jdk.nashorn.internal.ir.FunctionNode.CompilationState; 1.11 @@ -208,7 +207,7 @@ 1.12 final RecompilableScriptFunctionData data = new RecompilableScriptFunctionData( 1.13 newFunctionNode, 1.14 compiler.getCodeInstaller(), 1.15 - new AllocatorDescriptor(newFunctionNode.getThisProperties()), 1.16 + ObjectClassGenerator.createAllocationStrategy(newFunctionNode.getThisProperties()), 1.17 nestedFunctions, 1.18 externalSymbolDepths.get(fnId), 1.19 internalSymbols.get(fnId),
2.1 --- a/src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java Fri Mar 06 15:26:51 2015 +0100 2.2 +++ b/src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java Mon Mar 09 11:34:48 2015 +0100 2.3 @@ -56,6 +56,7 @@ 2.4 import jdk.nashorn.internal.codegen.ClassEmitter.Flag; 2.5 import jdk.nashorn.internal.codegen.types.Type; 2.6 import jdk.nashorn.internal.runtime.AccessorProperty; 2.7 +import jdk.nashorn.internal.runtime.AllocationStrategy; 2.8 import jdk.nashorn.internal.runtime.Context; 2.9 import jdk.nashorn.internal.runtime.FunctionScope; 2.10 import jdk.nashorn.internal.runtime.JSType; 2.11 @@ -826,44 +827,15 @@ 2.12 } 2.13 2.14 /** 2.15 - * Describes the allocator class name and property map for a constructor function with the specified 2.16 + * Creates the allocator class name and property map for a constructor function with the specified 2.17 * number of "this" properties that it initializes. 2.18 - * 2.19 + * @param thisProperties number of properties assigned to "this" 2.20 + * @return the allocation strategy 2.21 */ 2.22 - public static class AllocatorDescriptor { 2.23 - private final String allocatorClassName; 2.24 - private final PropertyMap allocatorMap; 2.25 - 2.26 - /** 2.27 - * Creates a new allocator descriptor 2.28 - * @param thisProperties the number of "this" properties that the function initializes 2.29 - */ 2.30 - public AllocatorDescriptor(final int thisProperties) { 2.31 - final int paddedFieldCount = getPaddedFieldCount(thisProperties); 2.32 - this.allocatorClassName = Compiler.binaryName(getClassName(paddedFieldCount)); 2.33 - this.allocatorMap = PropertyMap.newMap(null, allocatorClassName, 0, paddedFieldCount, 0); 2.34 - } 2.35 - 2.36 - /** 2.37 - * Returns the name of the class that the function allocates 2.38 - * @return the name of the class that the function allocates 2.39 - */ 2.40 - public String getAllocatorClassName() { 2.41 - return allocatorClassName; 2.42 - } 2.43 - 2.44 - /** 2.45 - * Returns the allocator map for the function. 2.46 - * @return the allocator map for the function. 2.47 - */ 2.48 - public PropertyMap getAllocatorMap() { 2.49 - return allocatorMap; 2.50 - } 2.51 - 2.52 - @Override 2.53 - public String toString() { 2.54 - return "AllocatorDescriptor[allocatorClassName=" + allocatorClassName + ", allocatorMap.size=" + 2.55 - allocatorMap.size() + "]"; 2.56 - } 2.57 + static AllocationStrategy createAllocationStrategy(final int thisProperties) { 2.58 + final int paddedFieldCount = getPaddedFieldCount(thisProperties); 2.59 + final String allocatorClassName = Compiler.binaryName(getClassName(paddedFieldCount)); 2.60 + final PropertyMap allocatorMap = PropertyMap.newMap(null, allocatorClassName, 0, paddedFieldCount, 0); 2.61 + return new AllocationStrategy(allocatorMap, allocatorClassName); 2.62 } 2.63 }
3.1 --- a/src/jdk/nashorn/internal/runtime/AllocationStrategy.java Fri Mar 06 15:26:51 2015 +0100 3.2 +++ b/src/jdk/nashorn/internal/runtime/AllocationStrategy.java Mon Mar 09 11:34:48 2015 +0100 3.3 @@ -30,22 +30,15 @@ 3.4 import java.lang.invoke.MethodHandle; 3.5 import java.lang.invoke.MethodHandles; 3.6 import jdk.nashorn.internal.codegen.CompilerConstants; 3.7 -import jdk.nashorn.internal.codegen.ObjectClassGenerator.AllocatorDescriptor; 3.8 3.9 /** 3.10 - * Encapsulates the allocation strategy for a function when used as a constructor. Basically the same as 3.11 - * {@link AllocatorDescriptor}, but with an additionally cached resolved method handle. There is also a 3.12 - * canonical default allocation strategy for functions that don't assign any "this" properties (vast majority 3.13 - * of all functions), therefore saving some storage space in {@link RecompilableScriptFunctionData} that would 3.14 - * otherwise be lost to identical tuples of (map, className, handle) fields. 3.15 + * Encapsulates the allocation strategy for a function when used as a constructor. 3.16 */ 3.17 -final class AllocationStrategy implements Serializable { 3.18 +final public class AllocationStrategy implements Serializable { 3.19 private static final long serialVersionUID = 1L; 3.20 3.21 private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup(); 3.22 3.23 - private static final AllocationStrategy DEFAULT_STRATEGY = new AllocationStrategy(new AllocatorDescriptor(0)); 3.24 - 3.25 /** Allocator map from allocator descriptor */ 3.26 private final PropertyMap allocatorMap; 3.27 3.28 @@ -55,19 +48,15 @@ 3.29 /** lazily generated allocator */ 3.30 private transient MethodHandle allocator; 3.31 3.32 - private AllocationStrategy(final AllocatorDescriptor desc) { 3.33 - this.allocatorMap = desc.getAllocatorMap(); 3.34 + /** 3.35 + * Construct an allocation strategy with the given map and class name. 3.36 + * @param allocatorMap the property map 3.37 + * @param className the class name 3.38 + */ 3.39 + public AllocationStrategy(final PropertyMap allocatorMap, final String className) { 3.40 + this.allocatorMap = allocatorMap; 3.41 // These classes get loaded, so an interned variant of their name is most likely around anyway. 3.42 - this.allocatorClassName = desc.getAllocatorClassName().intern(); 3.43 - } 3.44 - 3.45 - private boolean matches(final AllocatorDescriptor desc) { 3.46 - return desc.getAllocatorMap().size() == allocatorMap.size() && 3.47 - desc.getAllocatorClassName().equals(allocatorClassName); 3.48 - } 3.49 - 3.50 - static AllocationStrategy get(final AllocatorDescriptor desc) { 3.51 - return DEFAULT_STRATEGY.matches(desc) ? DEFAULT_STRATEGY : new AllocationStrategy(desc); 3.52 + this.allocatorClassName = className.intern(); 3.53 } 3.54 3.55 PropertyMap getAllocatorMap() { 3.56 @@ -88,14 +77,6 @@ 3.57 } 3.58 } 3.59 3.60 - private Object readResolve() { 3.61 - if(allocatorMap.size() == DEFAULT_STRATEGY.allocatorMap.size() && 3.62 - allocatorClassName.equals(DEFAULT_STRATEGY.allocatorClassName)) { 3.63 - return DEFAULT_STRATEGY; 3.64 - } 3.65 - return this; 3.66 - } 3.67 - 3.68 @Override 3.69 public String toString() { 3.70 return "AllocationStrategy[allocatorClassName=" + allocatorClassName + ", allocatorMap.size=" +
4.1 --- a/src/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java Fri Mar 06 15:26:51 2015 +0100 4.2 +++ b/src/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java Mon Mar 09 11:34:48 2015 +0100 4.3 @@ -43,7 +43,6 @@ 4.4 import jdk.nashorn.internal.codegen.CompilerConstants; 4.5 import jdk.nashorn.internal.codegen.FunctionSignature; 4.6 import jdk.nashorn.internal.codegen.Namespace; 4.7 -import jdk.nashorn.internal.codegen.ObjectClassGenerator.AllocatorDescriptor; 4.8 import jdk.nashorn.internal.codegen.OptimisticTypesPersistence; 4.9 import jdk.nashorn.internal.codegen.TypeMap; 4.10 import jdk.nashorn.internal.codegen.types.Type; 4.11 @@ -126,7 +125,7 @@ 4.12 * 4.13 * @param functionNode functionNode that represents this function code 4.14 * @param installer installer for code regeneration versions of this function 4.15 - * @param allocationDescriptor descriptor for the allocation behavior when this function is used as a constructor 4.16 + * @param allocationStrategy strategy for the allocation behavior when this function is used as a constructor 4.17 * @param nestedFunctions nested function map 4.18 * @param externalScopeDepths external scope depths 4.19 * @param internalSymbols internal symbols to method, defined in its scope 4.20 @@ -135,7 +134,7 @@ 4.21 public RecompilableScriptFunctionData( 4.22 final FunctionNode functionNode, 4.23 final CodeInstaller<ScriptEnvironment> installer, 4.24 - final AllocatorDescriptor allocationDescriptor, 4.25 + final AllocationStrategy allocationStrategy, 4.26 final Map<Integer, RecompilableScriptFunctionData> nestedFunctions, 4.27 final Map<String, Integer> externalScopeDepths, 4.28 final Set<String> internalSymbols, 4.29 @@ -153,7 +152,7 @@ 4.30 this.endParserState = functionNode.getEndParserState(); 4.31 this.token = tokenFor(functionNode); 4.32 this.installer = installer; 4.33 - this.allocationStrategy = AllocationStrategy.get(allocationDescriptor); 4.34 + this.allocationStrategy = allocationStrategy; 4.35 this.nestedFunctions = smallMap(nestedFunctions); 4.36 this.externalScopeDepths = smallMap(externalScopeDepths); 4.37 this.internalSymbols = smallSet(new HashSet<>(internalSymbols));
5.1 --- /dev/null Thu Jan 01 00:00:00 1970 +0000 5.2 +++ b/test/script/basic/JDK-8074556.js Mon Mar 09 11:34:48 2015 +0100 5.3 @@ -0,0 +1,52 @@ 5.4 +/* 5.5 + * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved. 5.6 + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. 5.7 + * 5.8 + * This code is free software; you can redistribute it and/or modify it 5.9 + * under the terms of the GNU General Public License version 2 only, as 5.10 + * published by the Free Software Foundation. 5.11 + * 5.12 + * This code is distributed in the hope that it will be useful, but WITHOUT 5.13 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or 5.14 + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License 5.15 + * version 2 for more details (a copy is included in the LICENSE file that 5.16 + * accompanied this code). 5.17 + * 5.18 + * You should have received a copy of the GNU General Public License version 5.19 + * 2 along with this work; if not, write to the Free Software Foundation, 5.20 + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. 5.21 + * 5.22 + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA 5.23 + * or visit www.oracle.com if you need additional information or have any 5.24 + * questions. 5.25 + */ 5.26 + 5.27 +/** 5.28 + * JDK-8074556: Functions should not share allocator maps 5.29 + * 5.30 + * @test 5.31 + * @run 5.32 + */ 5.33 + 5.34 +function A () { 5.35 + return this; 5.36 +} 5.37 + 5.38 +function B() { 5.39 + return this; 5.40 +} 5.41 + 5.42 +A.prototype.x = "x"; 5.43 +A.prototype.y = "y"; 5.44 +B.prototype.y = "y"; // same properties but different order 5.45 +B.prototype.x = "x"; 5.46 + 5.47 +function test(o) { 5.48 + Assert.assertEquals(o.x, "x"); 5.49 + Assert.assertEquals(o.y, "y"); 5.50 +} 5.51 + 5.52 +test(new A()); 5.53 +test(new B()); 5.54 +test(new A()); 5.55 +test(new B());