From 3e6b14405217bed1b38b25efbce8b2e015fd0b66 Mon Sep 17 00:00:00 2001 From: hannesw Date: Fri, 5 Jul 2013 14:36:54 +0200 Subject: 8017084: Use spill properties for large object literals Reviewed-by: lagergren, sundar --- .../nashorn/internal/codegen/CodeGenerator.java | 137 ++++++++++----------- .../internal/codegen/FieldObjectCreator.java | 69 +++++++++-- .../nashorn/internal/codegen/FinalizeTypes.java | 8 ++ src/jdk/nashorn/internal/codegen/MapCreator.java | 38 ++++-- .../internal/codegen/ObjectClassGenerator.java | 15 ++- .../nashorn/internal/codegen/ObjectCreator.java | 74 ++--------- .../internal/codegen/SpillObjectCreator.java | 134 ++++++++++++++++++++ 7 files changed, 317 insertions(+), 158 deletions(-) create mode 100644 src/jdk/nashorn/internal/codegen/SpillObjectCreator.java (limited to 'src/jdk/nashorn/internal/codegen') diff --git a/src/jdk/nashorn/internal/codegen/CodeGenerator.java b/src/jdk/nashorn/internal/codegen/CodeGenerator.java index 52241265..fe4e3676 100644 --- a/src/jdk/nashorn/internal/codegen/CodeGenerator.java +++ b/src/jdk/nashorn/internal/codegen/CodeGenerator.java @@ -179,6 +179,8 @@ final class CodeGenerator extends NodeOperatorVisitor foc = new FieldObjectCreator(this, nameList, newSymbols, values, true, hasArguments) { + new FieldObjectCreator(this, nameList, newSymbols, values, true, hasArguments) { @Override protected void loadValue(final Symbol value) { method.load(value); @@ -956,8 +958,7 @@ final class CodeGenerator extends NodeOperatorVisitor(this, keys, symbols, values) { - @Override - protected void loadValue(final Node node) { - load(node); - } + if (elements.size() > OBJECT_SPILL_THRESHOLD) { + new SpillObjectCreator(this, keys, symbols, values).makeObject(method); + } else { + new FieldObjectCreator(this, keys, symbols, values) { + @Override + protected void loadValue(final Node node) { + load(node); + } - /** - * Ensure that the properties start out as object types so that - * we can do putfield initializations instead of dynamicSetIndex - * which would be the case to determine initial property type - * otherwise. - * - * Use case, it's very expensive to do a million var x = {a:obj, b:obj} - * just to have to invalidate them immediately on initialization - * - * see NASHORN-594 - */ - @Override - protected MapCreator newMapCreator(final Class fieldObjectClass) { - return new MapCreator(fieldObjectClass, keys, symbols) { - @Override - protected int getPropertyFlags(final Symbol symbol, final boolean isVarArg) { - return super.getPropertyFlags(symbol, isVarArg) | Property.IS_ALWAYS_OBJECT; - } - }; - } + /** + * Ensure that the properties start out as object types so that + * we can do putfield initializations instead of dynamicSetIndex + * which would be the case to determine initial property type + * otherwise. + * + * Use case, it's very expensive to do a million var x = {a:obj, b:obj} + * just to have to invalidate them immediately on initialization + * + * see NASHORN-594 + */ + @Override + protected MapCreator newMapCreator(final Class fieldObjectClass) { + return new MapCreator(fieldObjectClass, keys, symbols) { + @Override + protected int getPropertyFlags(final Symbol symbol, final boolean hasArguments) { + return super.getPropertyFlags(symbol, hasArguments) | Property.IS_ALWAYS_OBJECT; + } + }; + } - }.makeObject(method); + }.makeObject(method); + } method.dup(); globalObjectPrototype(); method.invoke(ScriptObject.SET_PROTO); - if (!hasGettersSetters) { - method.store(objectNode.getSymbol()); - return false; - } + if (hasGettersSetters) { + for (final PropertyNode propertyNode : elements) { + final FunctionNode getter = propertyNode.getGetter(); + final FunctionNode setter = propertyNode.getSetter(); - for (final Node element : elements) { - final PropertyNode propertyNode = (PropertyNode)element; - final Object key = propertyNode.getKey(); - final FunctionNode getter = propertyNode.getGetter(); - final FunctionNode setter = propertyNode.getSetter(); + if (getter == null && setter == null) { + continue; + } - if (getter == null && setter == null) { - continue; - } + method.dup().loadKey(propertyNode.getKey()); - method.dup().loadKey(key); + if (getter == null) { + method.loadNull(); + } else { + getter.accept(this); + } - if (getter == null) { - method.loadNull(); - } else { - getter.accept(this); - } + if (setter == null) { + method.loadNull(); + } else { + setter.accept(this); + } - if (setter == null) { - method.loadNull(); - } else { - setter.accept(this); + method.invoke(ScriptObject.SET_USER_ACCESSORS); } - - method.invoke(ScriptObject.SET_USER_ACCESSORS); } method.store(objectNode.getSymbol()); - return false; } @@ -3183,24 +3181,21 @@ final class CodeGenerator extends NodeOperatorVisitor(), new ArrayList(), false, false) { - @Override - protected void makeObject(final MethodEmitter m) { - final String className = SCRIPTFUNCTION_IMPL_OBJECT; + // Generate the object class and property map in case this function is ever used as constructor + final String className = SCRIPTFUNCTION_IMPL_OBJECT; + final int fieldCount = ObjectClassGenerator.getPaddedFieldCount(functionNode.countThisProperties()); + final String allocatorClassName = Compiler.binaryName(ObjectClassGenerator.getClassName(fieldCount)); + final PropertyMap allocatorMap = PropertyMap.newMap(null, 0, fieldCount, 0); - m._new(className).dup(); - loadConstant(new RecompilableScriptFunctionData(functionNode, compiler.getCodeInstaller(), Compiler.binaryName(getClassName()), makeMap())); + method._new(className).dup(); + loadConstant(new RecompilableScriptFunctionData(functionNode, compiler.getCodeInstaller(), allocatorClassName, allocatorMap)); - if (isLazy || functionNode.needsParentScope()) { - m.loadCompilerConstant(SCOPE); - } else { - m.loadNull(); - } - m.invoke(constructorNoLookup(className, RecompilableScriptFunctionData.class, ScriptObject.class)); - } - }.makeObject(method); + if (functionNode.isLazy() || functionNode.needsParentScope()) { + method.loadCompilerConstant(SCOPE); + } else { + method.loadNull(); + } + method.invoke(constructorNoLookup(className, RecompilableScriptFunctionData.class, ScriptObject.class)); } // calls on Global class. diff --git a/src/jdk/nashorn/internal/codegen/FieldObjectCreator.java b/src/jdk/nashorn/internal/codegen/FieldObjectCreator.java index 974b5ba8..9e15b497 100644 --- a/src/jdk/nashorn/internal/codegen/FieldObjectCreator.java +++ b/src/jdk/nashorn/internal/codegen/FieldObjectCreator.java @@ -26,15 +26,16 @@ package jdk.nashorn.internal.codegen; import static jdk.nashorn.internal.codegen.CompilerConstants.ARGUMENTS; -import static jdk.nashorn.internal.codegen.CompilerConstants.SCOPE; import static jdk.nashorn.internal.codegen.CompilerConstants.constructorNoLookup; import static jdk.nashorn.internal.codegen.CompilerConstants.typeDescriptor; +import static jdk.nashorn.internal.codegen.ObjectClassGenerator.getPaddedFieldCount; import static jdk.nashorn.internal.codegen.types.Type.OBJECT; import java.util.Iterator; import java.util.List; import jdk.nashorn.internal.codegen.types.Type; import jdk.nashorn.internal.ir.Symbol; +import jdk.nashorn.internal.runtime.Context; import jdk.nashorn.internal.runtime.PropertyMap; import jdk.nashorn.internal.runtime.ScriptObject; import jdk.nashorn.internal.runtime.arrays.ArrayIndex; @@ -48,6 +49,13 @@ import jdk.nashorn.internal.runtime.arrays.ArrayIndex; * @see jdk.nashorn.internal.ir.Node */ public abstract class FieldObjectCreator extends ObjectCreator { + + private String fieldObjectClassName; + private Class fieldObjectClass; + private int fieldCount; + private int paddedFieldCount; + private int paramCount; + /** array of corresponding values to symbols (null for no values) */ private final List values; @@ -80,14 +88,9 @@ public abstract class FieldObjectCreator extends ObjectCreator { super(codegen, keys, symbols, isScope, hasArguments); this.values = values; this.callSiteFlags = codegen.getCallSiteFlags(); - } - /** - * Loads the scope on the stack through the passed method emitter. - * @param method the method emitter to use - */ - protected void loadScope(final MethodEmitter method) { - method.loadCompilerConstant(SCOPE); + countFields(); + findClass(); } /** @@ -137,6 +140,13 @@ public abstract class FieldObjectCreator extends ObjectCreator { } } + @Override + protected PropertyMap makeMap() { + assert propertyMap == null : "property map already initialized"; + propertyMap = newMapCreator(fieldObjectClass).makeFieldMap(hasArguments(), fieldCount, paddedFieldCount); + return propertyMap; + } + /** * Technique for loading an initial value. Defined by anonymous subclasses in code gen. * @@ -173,4 +183,47 @@ public abstract class FieldObjectCreator extends ObjectCreator { loadValue(value); method.dynamicSetIndex(callSiteFlags); } + + /** + * Locate (or indirectly create) the object container class. + */ + private void findClass() { + fieldObjectClassName = isScope() ? + ObjectClassGenerator.getClassName(fieldCount, paramCount) : + ObjectClassGenerator.getClassName(paddedFieldCount); + + try { + this.fieldObjectClass = Context.forStructureClass(Compiler.binaryName(fieldObjectClassName)); + } catch (final ClassNotFoundException e) { + throw new AssertionError("Nashorn has encountered an internal error. Structure can not be created."); + } + } + + /** + * Get the class name for the object class, + * e.g. {@code com.nashorn.oracle.scripts.JO2P0} + * + * @return script class name + */ + String getClassName() { + return fieldObjectClassName; + } + + /** + * Tally the number of fields and parameters. + */ + private void countFields() { + for (final Symbol symbol : this.symbols) { + if (symbol != null) { + if (hasArguments() && symbol.isParam()) { + symbol.setFieldIndex(paramCount++); + } else { + symbol.setFieldIndex(fieldCount++); + } + } + } + + paddedFieldCount = getPaddedFieldCount(fieldCount); + } + } diff --git a/src/jdk/nashorn/internal/codegen/FinalizeTypes.java b/src/jdk/nashorn/internal/codegen/FinalizeTypes.java index f9d64322..14bec19d 100644 --- a/src/jdk/nashorn/internal/codegen/FinalizeTypes.java +++ b/src/jdk/nashorn/internal/codegen/FinalizeTypes.java @@ -175,6 +175,14 @@ final class FinalizeTypes extends NodeOperatorVisitor { if (destType == null) { destType = specBinaryNode.getType(); } + // Register assignments to this object in case this is used as constructor + if (binaryNode.lhs() instanceof AccessNode) { + AccessNode accessNode = (AccessNode) binaryNode.lhs(); + + if (accessNode.getBase().getSymbol().isThis()) { + lc.getCurrentFunction().addThisProperty(accessNode.getProperty().getName()); + } + } return specBinaryNode.setRHS(convert(specBinaryNode.rhs(), destType)); } diff --git a/src/jdk/nashorn/internal/codegen/MapCreator.java b/src/jdk/nashorn/internal/codegen/MapCreator.java index 609fac9b..6ad03c73 100644 --- a/src/jdk/nashorn/internal/codegen/MapCreator.java +++ b/src/jdk/nashorn/internal/codegen/MapCreator.java @@ -41,10 +41,10 @@ public class MapCreator { private final Class structure; /** key set for object map */ - private final String[] keys; + final List keys; /** corresponding symbol set for object map */ - private final Symbol[] symbols; + final List symbols; /** * Constructor @@ -54,11 +54,9 @@ public class MapCreator { * @param symbols list of symbols for map */ MapCreator(final Class structure, final List keys, final List symbols) { - final int size = keys.size(); - this.structure = structure; - this.keys = keys.toArray(new String[size]); - this.symbols = symbols.toArray(new Symbol[size]); + this.keys = keys; + this.symbols = symbols; } /** @@ -70,21 +68,37 @@ public class MapCreator { * * @return New map populated with accessor properties. */ - PropertyMap makeMap(final boolean hasArguments, final int fieldCount, final int fieldMaximum) { + PropertyMap makeFieldMap(final boolean hasArguments, final int fieldCount, final int fieldMaximum) { final List properties = new ArrayList<>(); - assert keys != null; - for (int i = 0; i < keys.length; i++) { - final String key = keys[i]; - final Symbol symbol = symbols[i]; + for (int i = 0, length = keys.size(); i < length; i++) { + final String key = keys.get(i); + final Symbol symbol = symbols.get(i); if (symbol != null && !ArrayIndex.isIntArrayIndex(key)) { properties.add(new AccessorProperty(key, getPropertyFlags(symbol, hasArguments), structure, symbol.getFieldIndex())); } } - return PropertyMap.newMap(structure, properties, fieldCount, fieldMaximum); + return PropertyMap.newMap(properties, fieldCount, fieldMaximum, 0); + } + + PropertyMap makeSpillMap(final boolean hasArguments) { + final List properties = new ArrayList<>(); + int spillIndex = 0; + assert keys != null; + + for (int i = 0, length = keys.size(); i < length; i++) { + final String key = keys.get(i); + final Symbol symbol = symbols.get(i); + + if (symbol != null && !ArrayIndex.isIntArrayIndex(key)) { + properties.add(new AccessorProperty(key, getPropertyFlags(symbol, hasArguments), spillIndex++)); + } + } + + return PropertyMap.newMap(properties, 0, 0, spillIndex); } /** diff --git a/src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java b/src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java index 934df7a6..7d61db1f 100644 --- a/src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java +++ b/src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java @@ -73,11 +73,6 @@ public final class ObjectClassGenerator { */ static final int FIELD_PADDING = 4; - /** - * Rounding when calculating the number of fields. - */ - static final int FIELD_ROUNDING = 4; - /** * Debug field logger * Should we print debugging information for fields when they are generated and getters/setters are called? @@ -325,7 +320,6 @@ public final class ObjectClassGenerator { final List initFields = addFields(classEmitter, fieldCount); final MethodEmitter init = newInitMethod(classEmitter); - initializeToUndefined(init, className, initFields); init.returnVoid(); init.end(); @@ -709,6 +703,15 @@ public final class ObjectClassGenerator { } } + /** + * Add padding to field count to avoid creating too many classes and have some spare fields + * @param count the field count + * @return the padded field count + */ + static int getPaddedFieldCount(final int count) { + return count / FIELD_PADDING * FIELD_PADDING + FIELD_PADDING; + } + // // Provide generic getters and setters for undefined types. If a type is undefined, all // and marshals the set to the correct setter depending on the type of the value being set. diff --git a/src/jdk/nashorn/internal/codegen/ObjectCreator.java b/src/jdk/nashorn/internal/codegen/ObjectCreator.java index b0e5a773..2a8ebba7 100644 --- a/src/jdk/nashorn/internal/codegen/ObjectCreator.java +++ b/src/jdk/nashorn/internal/codegen/ObjectCreator.java @@ -25,10 +25,10 @@ package jdk.nashorn.internal.codegen; +import static jdk.nashorn.internal.codegen.CompilerConstants.SCOPE; + import java.util.List; -import static jdk.nashorn.internal.codegen.ObjectClassGenerator.FIELD_PADDING; import jdk.nashorn.internal.ir.Symbol; -import jdk.nashorn.internal.runtime.Context; import jdk.nashorn.internal.runtime.PropertyMap; /** @@ -36,9 +36,6 @@ import jdk.nashorn.internal.runtime.PropertyMap; */ public abstract class ObjectCreator { - /** Compile unit for this ObjectCreator, see CompileUnit */ - //protected final CompileUnit compileUnit; - /** List of keys to initiate in this ObjectCreator */ protected final List keys; @@ -50,12 +47,7 @@ public abstract class ObjectCreator { private final boolean isScope; private final boolean hasArguments; - private int fieldCount; - private int paddedFieldCount; - private int paramCount; - private String fieldObjectClassName; - private Class fieldObjectClass; - private PropertyMap propertyMap; + protected PropertyMap propertyMap; /** * Constructor @@ -72,41 +64,6 @@ public abstract class ObjectCreator { this.symbols = symbols; this.isScope = isScope; this.hasArguments = hasArguments; - - countFields(); - findClass(); - } - - /** - * Tally the number of fields and parameters. - */ - private void countFields() { - for (final Symbol symbol : this.symbols) { - if (symbol != null) { - if (hasArguments() && symbol.isParam()) { - symbol.setFieldIndex(paramCount++); - } else { - symbol.setFieldIndex(fieldCount++); - } - } - } - - paddedFieldCount = fieldCount + FIELD_PADDING; - } - - /** - * Locate (or indirectly create) the object container class. - */ - private void findClass() { - fieldObjectClassName = isScope() ? - ObjectClassGenerator.getClassName(fieldCount, paramCount) : - ObjectClassGenerator.getClassName(paddedFieldCount); - - try { - this.fieldObjectClass = Context.forStructureClass(Compiler.binaryName(fieldObjectClassName)); - } catch (final ClassNotFoundException e) { - throw new AssertionError("Nashorn has encountered an internal error. Structure can not be created."); - } } /** @@ -115,6 +72,12 @@ public abstract class ObjectCreator { */ protected abstract void makeObject(final MethodEmitter method); + /** + * Construct the property map appropriate for the object. + * @return the newly created property map + */ + protected abstract PropertyMap makeMap(); + /** * Create a new MapCreator * @param clazz type of MapCreator @@ -125,12 +88,11 @@ public abstract class ObjectCreator { } /** - * Construct the property map appropriate for the object. - * @return the newly created property map + * Loads the scope on the stack through the passed method emitter. + * @param method the method emitter to use */ - protected PropertyMap makeMap() { - propertyMap = newMapCreator(fieldObjectClass).makeMap(hasArguments(), fieldCount, paddedFieldCount); - return propertyMap; + protected void loadScope(final MethodEmitter method) { + method.loadCompilerConstant(SCOPE); } /** @@ -143,16 +105,6 @@ public abstract class ObjectCreator { return method; } - /** - * Get the class name for the object class, - * e.g. {@code com.nashorn.oracle.scripts.JO2P0} - * - * @return script class name - */ - String getClassName() { - return fieldObjectClassName; - } - /** * Is this a scope object * @return true if scope diff --git a/src/jdk/nashorn/internal/codegen/SpillObjectCreator.java b/src/jdk/nashorn/internal/codegen/SpillObjectCreator.java new file mode 100644 index 00000000..33e1d432 --- /dev/null +++ b/src/jdk/nashorn/internal/codegen/SpillObjectCreator.java @@ -0,0 +1,134 @@ +/* + * Copyright (c) 2010-2013, 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. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * 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. + */ + +package jdk.nashorn.internal.codegen; + +import jdk.nashorn.internal.codegen.types.Type; +import jdk.nashorn.internal.ir.LiteralNode; +import jdk.nashorn.internal.ir.Node; +import jdk.nashorn.internal.ir.Symbol; +import jdk.nashorn.internal.runtime.Property; +import jdk.nashorn.internal.runtime.PropertyMap; +import jdk.nashorn.internal.runtime.ScriptObject; +import jdk.nashorn.internal.scripts.JO; + +import java.util.List; + +import static jdk.nashorn.internal.codegen.CompilerConstants.constructorNoLookup; +import static jdk.nashorn.internal.codegen.types.Type.OBJECT; + +/** + * An object creator that uses spill properties. + */ +public class SpillObjectCreator extends ObjectCreator { + + private final List values; + + /** + * Constructor + * + * @param codegen code generator + * @param keys keys for fields in object + * @param symbols symbols for fields in object + * @param values list of values corresponding to keys + */ + protected SpillObjectCreator(final CodeGenerator codegen, final List keys, final List symbols, final List values) { + super(codegen, keys, symbols, false, false); + this.values = values; + makeMap(); + } + + @Override + protected void makeObject(final MethodEmitter method) { + assert !isScope() : "spill scope objects are not currently supported"; + + final int length = keys.size(); + final Object[] presetValues = new Object[propertyMap.size()]; + final Class clazz = JO.class; + + // Compute constant values + for (int i = 0; i < length; i++) { + final String key = keys.get(i); + final Property property = propertyMap.findProperty(key); + + if (property != null) { + presetValues[property.getSlot()] = LiteralNode.objectAsConstant(values.get(i)); + } + } + + method._new(clazz).dup(); + codegen.loadConstant(propertyMap); + + method.invoke(constructorNoLookup(JO.class, PropertyMap.class)); + + method.dup(); + codegen.loadConstant(presetValues); + + // Create properties with non-constant values + for (int i = 0; i < length; i++) { + final String key = keys.get(i); + final Property property = propertyMap.findProperty(key); + + if (property != null && presetValues[property.getSlot()] == LiteralNode.POSTSET_MARKER) { + method.dup(); + method.load(property.getSlot()); + codegen.load(values.get(i)).convert(OBJECT); + method.arraystore(); + presetValues[property.getSlot()] = null; + } + } + + method.putField(Type.typeFor(ScriptObject.class).getInternalName(), "spill", Type.OBJECT_ARRAY.getDescriptor()); + final int callSiteFlags = codegen.getCallSiteFlags(); + + // Assign properties with valid array index keys + for (int i = 0; i < length; i++) { + final String key = keys.get(i); + final Property property = propertyMap.findProperty(key); + final Node value = values.get(i); + + if (property == null && value != null) { + method.dup(); + method.load(keys.get(i)); + codegen.load(value); + method.dynamicSetIndex(callSiteFlags); + } + } + } + + @Override + protected PropertyMap makeMap() { + assert propertyMap == null : "property map already initialized"; + + propertyMap = new MapCreator(JO.class, keys, symbols) { + @Override + protected int getPropertyFlags(Symbol symbol, boolean hasArguments) { + return super.getPropertyFlags(symbol, hasArguments) | Property.IS_SPILL | Property.IS_ALWAYS_OBJECT; + } + }.makeSpillMap(false); + + return propertyMap; + } +} -- cgit v1.2.3