# HG changeset patch # User hannesw # Date 1425897288 -3600 # Node ID 02702b17f1d8fbca11f3a008a1e5d8f71947604d # Parent 7d249c2d066aca961e9cde73601331d616bb5448 8074556: Functions should not share allocator maps Reviewed-by: lagergren, sundar diff -r 7d249c2d066a -r 02702b17f1d8 src/jdk/nashorn/internal/codegen/FindScopeDepths.java --- a/src/jdk/nashorn/internal/codegen/FindScopeDepths.java Fri Mar 06 15:26:51 2015 +0100 +++ b/src/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), diff -r 7d249c2d066a -r 02702b17f1d8 src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java --- a/src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java Fri Mar 06 15:26:51 2015 +0100 +++ b/src/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); } } diff -r 7d249c2d066a -r 02702b17f1d8 src/jdk/nashorn/internal/runtime/AllocationStrategy.java --- a/src/jdk/nashorn/internal/runtime/AllocationStrategy.java Fri Mar 06 15:26:51 2015 +0100 +++ b/src/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=" + diff -r 7d249c2d066a -r 02702b17f1d8 src/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java --- a/src/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java Fri Mar 06 15:26:51 2015 +0100 +++ b/src/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 installer, - final AllocatorDescriptor allocationDescriptor, + final AllocationStrategy allocationStrategy, final Map nestedFunctions, final Map externalScopeDepths, final Set 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)); diff -r 7d249c2d066a -r 02702b17f1d8 test/script/basic/JDK-8074556.js --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/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());