8074556: Functions should not share allocator maps

Mon, 09 Mar 2015 11:34:48 +0100

author
hannesw
date
Mon, 09 Mar 2015 11:34:48 +0100
changeset 1249
02702b17f1d8
parent 1248
7d249c2d066a
child 1250
9ee1fc3f6136

8074556: Functions should not share allocator maps
Reviewed-by: lagergren, sundar

src/jdk/nashorn/internal/codegen/FindScopeDepths.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/runtime/AllocationStrategy.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java file | annotate | diff | comparison | revisions
test/script/basic/JDK-8074556.js file | annotate | diff | comparison | revisions
     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());

mercurial