# HG changeset patch # User lagergren # Date 1410345464 -7200 # Node ID 698280da463ab0020c258137c18a506cc7a10606 # Parent 3f49db18721f0c2e7408ebedfff5bf8dbba00490 8057703: More empty classes generated by Nashorn Reviewed-by: attila, sundar diff -r 3f49db18721f -r 698280da463a src/jdk/nashorn/internal/codegen/ClassEmitter.java --- a/src/jdk/nashorn/internal/codegen/ClassEmitter.java Fri Sep 05 16:28:17 2014 +0200 +++ b/src/jdk/nashorn/internal/codegen/ClassEmitter.java Wed Sep 10 12:37:44 2014 +0200 @@ -59,6 +59,7 @@ import java.util.EnumSet; import java.util.HashSet; import java.util.Set; + import jdk.internal.org.objectweb.asm.ClassWriter; import jdk.internal.org.objectweb.asm.MethodVisitor; import jdk.internal.org.objectweb.asm.util.TraceClassVisitor; @@ -135,6 +136,16 @@ /** Set of constants access methods required. */ private Set> constantMethodNeeded; + private int methodCount; + + private int initCount; + + private int clinitCount; + + private int fieldCount; + + private final Set methodNames; + /** * Constructor - only used internally in this class as it breaks * abstraction towards ASM or other code generator below @@ -146,6 +157,11 @@ this.context = context; this.cw = cw; this.methodsStarted = new HashSet<>(); + this.methodNames = new HashSet<>(); + } + + public Set getMethodNames() { + return methodNames; } /** @@ -209,6 +225,38 @@ } /** + * Get the method count, including init and clinit methods + * @return method count + */ + public int getMethodCount() { + return methodCount; + } + + /** + * Get the clinit count + * @return clinit count + */ + public int getClinitCount() { + return clinitCount; + } + + /** + * Get the init count + * @return init count + */ + public int getInitCount() { + return initCount; + } + + /** + * Get the field count + * @return field count + */ + public int getFieldCount() { + return fieldCount; + } + + /** * Convert a binary name to a package/class name. * * @param name Binary name. @@ -359,9 +407,16 @@ */ @Override public void end() { - assert classStarted; + assert classStarted : "class not started for " + unitClassName; if (unitClassName != null) { + final MethodEmitter initMethod = init(EnumSet.of(Flag.PRIVATE)); + initMethod.begin(); + initMethod.load(Type.OBJECT, 0); + initMethod.newInstance(jdk.nashorn.internal.scripts.JS.class); + initMethod.returnVoid(); + initMethod.end(); + defineCommonUtilities(); } @@ -419,6 +474,8 @@ } SplitMethodEmitter method(final SplitNode splitNode, final String methodName, final Class rtype, final Class... ptypes) { + methodCount++; + methodNames.add(methodName); return new SplitMethodEmitter(this, methodVisitor(EnumSet.of(Flag.PUBLIC, Flag.STATIC), methodName, rtype, ptypes), splitNode); } @@ -446,6 +503,8 @@ * @return method emitter to use for weaving this method */ MethodEmitter method(final EnumSet methodFlags, final String methodName, final Class rtype, final Class... ptypes) { + methodCount++; + methodNames.add(methodName); return new MethodEmitter(this, methodVisitor(methodFlags, methodName, rtype, ptypes)); } @@ -471,6 +530,8 @@ * @return method emitter to use for weaving this method */ MethodEmitter method(final EnumSet methodFlags, final String methodName, final String descriptor) { + methodCount++; + methodNames.add(methodName); return new MethodEmitter(this, cw.visitMethod(Flag.getValue(methodFlags), methodName, descriptor, null, null)); } @@ -481,6 +542,8 @@ * @return method emitter to use for weaving this method */ MethodEmitter method(final FunctionNode functionNode) { + methodCount++; + methodNames.add(functionNode.getName()); final FunctionSignature signature = new FunctionSignature(functionNode); final MethodVisitor mv = cw.visitMethod( ACC_PUBLIC | ACC_STATIC | (functionNode.isVarArg() ? ACC_VARARGS : 0), @@ -499,6 +562,8 @@ * @return method emitter to use for weaving this method */ MethodEmitter restOfMethod(final FunctionNode functionNode) { + methodCount++; + methodNames.add(functionNode.getName()); final MethodVisitor mv = cw.visitMethod( ACC_PUBLIC | ACC_STATIC, functionNode.getName(), @@ -516,6 +581,7 @@ * @return method emitter to use for weaving */ MethodEmitter clinit() { + clinitCount++; return method(EnumSet.of(Flag.STATIC), CLINIT.symbolName(), void.class); } @@ -525,6 +591,7 @@ * @return method emitter to use for weaving ()V */ MethodEmitter init() { + initCount++; return method(INIT.symbolName(), void.class); } @@ -535,6 +602,7 @@ * @return method emitter to use for weaving ()V */ MethodEmitter init(final Class... ptypes) { + initCount++; return method(INIT.symbolName(), void.class, ptypes); } @@ -547,6 +615,7 @@ * @return method emitter to use for weaving (...)V */ MethodEmitter init(final EnumSet flags, final Class... ptypes) { + initCount++; return method(flags, INIT.symbolName(), void.class, ptypes); } @@ -561,6 +630,7 @@ * @see ClassEmitter.Flag */ final void field(final EnumSet fieldFlags, final String fieldName, final Class fieldType, final Object value) { + fieldCount++; cw.visitField(Flag.getValue(fieldFlags), fieldName, typeDescriptor(fieldType), null, value).visitEnd(); } diff -r 3f49db18721f -r 698280da463a src/jdk/nashorn/internal/codegen/CodeGeneratorLexicalContext.java --- a/src/jdk/nashorn/internal/codegen/CodeGeneratorLexicalContext.java Fri Sep 05 16:28:17 2014 +0200 +++ b/src/jdk/nashorn/internal/codegen/CodeGeneratorLexicalContext.java Wed Sep 10 12:37:44 2014 +0200 @@ -31,6 +31,7 @@ import java.util.Deque; import java.util.HashMap; import java.util.Map; + import jdk.nashorn.internal.IntDeque; import jdk.nashorn.internal.codegen.types.Type; import jdk.nashorn.internal.ir.Block; @@ -158,7 +159,9 @@ CompileUnit popCompileUnit(final CompileUnit oldUnit) { assert compileUnits.peek() == oldUnit; - compileUnits.pop(); + final CompileUnit unit = compileUnits.pop(); + assert unit.hasCode() : "compile unit popped without code"; + unit.setUsed(); return compileUnits.isEmpty() ? null : compileUnits.peek(); } diff -r 3f49db18721f -r 698280da463a src/jdk/nashorn/internal/codegen/CompilationPhase.java --- a/src/jdk/nashorn/internal/codegen/CompilationPhase.java Fri Sep 05 16:28:17 2014 +0200 +++ b/src/jdk/nashorn/internal/codegen/CompilationPhase.java Wed Sep 10 12:37:44 2014 +0200 @@ -53,7 +53,6 @@ import jdk.nashorn.internal.codegen.Compiler.CompilationPhases; import jdk.nashorn.internal.ir.FunctionNode; import jdk.nashorn.internal.ir.FunctionNode.CompilationState; -import jdk.nashorn.internal.ir.CompileUnitHolder; import jdk.nashorn.internal.ir.LexicalContext; import jdk.nashorn.internal.ir.LiteralNode; import jdk.nashorn.internal.ir.LiteralNode.ArrayLiteralNode; @@ -302,70 +301,6 @@ } }, - /** - * Mark compile units used in the method tree. Used for non-rest compilation only - */ - MARK_USED_COMPILE_UNITS( - EnumSet.of( - INITIALIZED, - PARSED, - CONSTANT_FOLDED, - LOWERED, - BUILTINS_TRANSFORMED, - SPLIT, - SYMBOLS_ASSIGNED, - SCOPE_DEPTHS_COMPUTED, - OPTIMISTIC_TYPES_ASSIGNED, - LOCAL_VARIABLE_TYPES_CALCULATED)) { - - @Override - FunctionNode transform(final Compiler compiler, final CompilationPhases phases, final FunctionNode fn) { - final FunctionNode newFunctionNode = (FunctionNode)fn.accept(new NodeVisitor(new LexicalContext()) { - private void tagUsed(final CompileUnitHolder node) { - assert node.getCompileUnit() != null : "no compile unit in " + node; - node.getCompileUnit().setUsed(); - } - - @Override - public Node leaveFunctionNode(final FunctionNode node) { - tagUsed(node); - return node; - } - - @Override - public Node leaveSplitNode(final SplitNode node) { - tagUsed(node); - return node; - } - - @Override - public Node leaveLiteralNode(final LiteralNode node) { - if (node instanceof ArrayLiteralNode) { - final ArrayLiteralNode aln = (ArrayLiteralNode)node; - if (aln.getUnits() != null) { - for (final ArrayUnit au : aln.getUnits()) { - tagUsed(au); - } - } - return aln; - } - return node; - } - - @Override - public Node leaveDefault(final Node node) { - return node.ensureUniqueLabels(lc); - } - }); - - return newFunctionNode; - } - - @Override - public String toString() { - return "'Tag Compile Units Used'"; - } - }, /** * Reuse compile units, if they are already present. We are using the same compiler @@ -401,6 +336,8 @@ if (phases.isRestOfCompilation()) { sb.append("$restOf"); } + //it's ok to not copy the initCount, methodCount and clinitCount here, as codegen is what + //fills those out anyway. Thus no need for a copy constructor final CompileUnit newUnit = compiler.createCompileUnit(sb.toString(), oldUnit.getWeight()); log.fine("Creating new compile unit ", oldUnit, " => ", newUnit); map.put(oldUnit, newUnit); @@ -425,7 +362,6 @@ assert newUnit != null : "old unit has no mapping to new unit " + oldUnit; log.fine("Replacing compile unit: ", oldUnit, " => ", newUnit, " in ", quote(node.getName())); - newUnit.setUsed(); return node.setCompileUnit(lc, newUnit).setState(lc, CompilationState.COMPILE_UNITS_REUSED); } @@ -438,7 +374,6 @@ assert newUnit != null : "old unit has no mapping to new unit " + oldUnit; log.fine("Replacing compile unit: ", oldUnit, " => ", newUnit, " in ", quote(node.getName())); - newUnit.setUsed(); return node.setCompileUnit(lc, newUnit); } @@ -453,7 +388,6 @@ for (final ArrayUnit au : aln.getUnits()) { final CompileUnit newUnit = map.get(au.getCompileUnit()); assert newUnit != null; - newUnit.setUsed(); newArrayUnits.add(new ArrayUnit(newUnit, au.getLo(), au.getHi())); } return aln.setUnits(lc, newArrayUnits); @@ -500,8 +434,14 @@ FunctionNode newFunctionNode = fn; + //root class is special, as it is bootstrapped from createProgramFunction, thus it's skipped + //in CodeGeneration - the rest can be used as a working "is compile unit used" metric + fn.getCompileUnit().setUsed(); + compiler.getLogger().fine("Starting bytecode generation for ", quote(fn.getName()), " - restOf=", phases.isRestOfCompilation()); + final CodeGenerator codegen = new CodeGenerator(compiler, phases.isRestOfCompilation() ? compiler.getContinuationEntryPoints() : null); + try { // Explicitly set BYTECODE_GENERATED here; it can not be set in case of skipping codegen for :program // in the lazy + optimistic world. See CodeGenerator.skipFunction(). @@ -522,19 +462,21 @@ } for (final CompileUnit compileUnit : compiler.getCompileUnits()) { + final ClassEmitter classEmitter = compileUnit.getClassEmitter(); + classEmitter.end(); + if (!compileUnit.isUsed()) { compiler.getLogger().fine("Skipping unused compile unit ", compileUnit); continue; } - final ClassEmitter classEmitter = compileUnit.getClassEmitter(); - classEmitter.end(); - final byte[] bytecode = classEmitter.toByteArray(); assert bytecode != null; final String className = compileUnit.getUnitClassName(); - compiler.addClass(className, bytecode); + compiler.addClass(className, bytecode); //classes are only added to the bytecode map if compile unit is used + + CompileUnit.increaseEmitCount(); // should we verify the generated code? if (senv._verify_code) { diff -r 3f49db18721f -r 698280da463a src/jdk/nashorn/internal/codegen/CompileUnit.java --- a/src/jdk/nashorn/internal/codegen/CompileUnit.java Fri Sep 05 16:28:17 2014 +0200 +++ b/src/jdk/nashorn/internal/codegen/CompileUnit.java Wed Sep 10 12:37:44 2014 +0200 @@ -44,6 +44,8 @@ private boolean isUsed; + private static int emittedUnitCount; + CompileUnit(final String className, final ClassEmitter classEmitter, final long initialWeight) { this.className = className; this.weight = initialWeight; @@ -54,6 +56,14 @@ return new TreeSet<>(); } + static void increaseEmitCount() { + emittedUnitCount++; + } + + public static int getEmittedUnitCount() { + return emittedUnitCount; + } + /** * Check if this compile unit is used * @return true if tagged as in use - i.e active code that needs to be generated @@ -62,6 +72,10 @@ return isUsed; } + public boolean hasCode() { + return (classEmitter.getMethodCount() - classEmitter.getInitCount() - classEmitter.getClinitCount()) > 0; + } + /** * Tag this compile unit as used */ @@ -138,7 +152,8 @@ @Override public String toString() { - return "[CompileUnit className=" + shortName(className) + " weight=" + weight + '/' + Splitter.SPLIT_THRESHOLD + ']'; + final String methods = classEmitter != null ? classEmitter.getMethodNames().toString() : ""; + return "[CompileUnit className=" + shortName(className) + " weight=" + weight + '/' + Splitter.SPLIT_THRESHOLD + " hasCode=" + methods + ']'; } @Override diff -r 3f49db18721f -r 698280da463a src/jdk/nashorn/internal/codegen/Compiler.java --- a/src/jdk/nashorn/internal/codegen/Compiler.java Fri Sep 05 16:28:17 2014 +0200 +++ b/src/jdk/nashorn/internal/codegen/Compiler.java Wed Sep 10 12:37:44 2014 +0200 @@ -38,7 +38,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.Comparator; -import java.util.EnumSet; import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; @@ -51,7 +50,6 @@ import java.util.function.Consumer; import java.util.logging.Level; import jdk.internal.dynalink.support.NameCodec; -import jdk.nashorn.internal.codegen.ClassEmitter.Flag; import jdk.nashorn.internal.codegen.types.Type; import jdk.nashorn.internal.ir.Expression; import jdk.nashorn.internal.ir.FunctionNode; @@ -173,14 +171,13 @@ CompilationPhase.SCOPE_DEPTH_COMPUTATION_PHASE, CompilationPhase.OPTIMISTIC_TYPE_ASSIGNMENT_PHASE, CompilationPhase.LOCAL_VARIABLE_TYPE_CALCULATION_PHASE, - CompilationPhase.MARK_USED_COMPILE_UNITS, CompilationPhase.BYTECODE_GENERATION_PHASE, CompilationPhase.INSTALL_PHASE }); /** Compile all for a rest of method */ public final static CompilationPhases COMPILE_ALL_RESTOF = - COMPILE_ALL.setDescription("Compile all, rest of").replace(CompilationPhase.MARK_USED_COMPILE_UNITS, CompilationPhase.REUSE_COMPILE_UNITS_PHASE); + COMPILE_ALL.setDescription("Compile all, rest of").addAfter(CompilationPhase.LOCAL_VARIABLE_TYPE_CALCULATION_PHASE, CompilationPhase.REUSE_COMPILE_UNITS_PHASE); /** Singleton that describes a standard eager compilation, but no installation, for example used by --compile-only */ public final static CompilationPhases COMPILE_ALL_NO_INSTALL = @@ -251,6 +248,7 @@ return new CompilationPhases(desc, list.toArray(new CompilationPhase[list.size()])); } + @SuppressWarnings("unused") //TODO I'll use this soon private CompilationPhases replace(final CompilationPhase phase, final CompilationPhase newPhase) { final LinkedList list = new LinkedList<>(); for (final CompilationPhase p : phases) { @@ -259,7 +257,6 @@ return new CompilationPhases(desc, list.toArray(new CompilationPhase[list.size()])); } - @SuppressWarnings("unused") //TODO I'll use this soon private CompilationPhases addAfter(final CompilationPhase phase, final CompilationPhase newPhase) { final LinkedList list = new LinkedList<>(); for (final CompilationPhase p : phases) { @@ -700,16 +697,8 @@ CompileUnit createCompileUnit(final String unitClassName, final long initialWeight) { final ClassEmitter classEmitter = new ClassEmitter(context, sourceName, unitClassName, isStrict()); final CompileUnit compileUnit = new CompileUnit(unitClassName, classEmitter, initialWeight); - classEmitter.begin(); - final MethodEmitter initMethod = classEmitter.init(EnumSet.of(Flag.PRIVATE)); - initMethod.begin(); - initMethod.load(Type.OBJECT, 0); - initMethod.newInstance(jdk.nashorn.internal.scripts.JS.class); - initMethod.returnVoid(); - initMethod.end(); - return compileUnit; } diff -r 3f49db18721f -r 698280da463a src/jdk/nashorn/internal/runtime/Timing.java --- a/src/jdk/nashorn/internal/runtime/Timing.java Fri Sep 05 16:28:17 2014 +0200 +++ b/src/jdk/nashorn/internal/runtime/Timing.java Wed Sep 10 12:37:44 2014 +0200 @@ -33,6 +33,8 @@ import java.util.Map; import java.util.concurrent.TimeUnit; import java.util.function.Supplier; + +import jdk.nashorn.internal.codegen.CompileUnit; import jdk.nashorn.internal.runtime.logging.DebugLogger; import jdk.nashorn.internal.runtime.logging.Loggable; import jdk.nashorn.internal.runtime.logging.Logger; @@ -224,6 +226,9 @@ append((int)(knownTime * 100.0 / total)). append("%])"); + sb.append("\n\nEmitted compile units: "). + append(CompileUnit.getEmittedUnitCount()); + return sb.toString(); }