8057703: More empty classes generated by Nashorn

Reviewed-by: attila, sundar

6 files changed, 112 insertions, 88 deletions

diff --git a/src/jdk/nashorn/internal/codegen/ClassEmitter.java b/src/jdk/nashorn/internal/codegen/ClassEmitter.java
index 386effdc..9f7fe79d 100644
--- a/src/jdk/nashorn/internal/codegen/ClassEmitter.java
+++ b/src/jdk/nashorn/internal/codegen/ClassEmitter.java
@@ -59,6 +59,7 @@ import java.security.PrivilegedAction;
 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 @@ public class ClassEmitter implements Emitter {
     /** Set of constants access methods required. */
     private Set<Class<?>> constantMethodNeeded;
 
+    private int methodCount;
+
+    private int initCount;
+
+    private int clinitCount;
+
+    private int fieldCount;
+
+    private final Set<String> methodNames;
+
     /**
      * Constructor - only used internally in this class as it breaks
      * abstraction towards ASM or other code generator below
@@ -146,6 +157,11 @@ public class ClassEmitter implements Emitter {
         this.context        = context;
         this.cw             = cw;
         this.methodsStarted = new HashSet<>();
+        this.methodNames    = new HashSet<>();
+    }
+
+    public Set<String> getMethodNames() {
+        return methodNames;
     }
 
     /**
@@ -209,6 +225,38 @@ public class ClassEmitter implements Emitter {
     }
 
     /**
+     * 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 @@ public class ClassEmitter implements Emitter {
      */
     @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 @@ public class ClassEmitter implements Emitter {
     }
 
     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 @@ public class ClassEmitter implements Emitter {
      * @return method emitter to use for weaving this method
      */
     MethodEmitter method(final EnumSet<Flag> 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 @@ public class ClassEmitter implements Emitter {
      * @return method emitter to use for weaving this method
      */
     MethodEmitter method(final EnumSet<Flag> 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 @@ public class ClassEmitter implements Emitter {
      * @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 @@ public class ClassEmitter implements Emitter {
      * @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 @@ public class ClassEmitter implements Emitter {
      * @return method emitter to use for weaving <clinit>
      */
     MethodEmitter clinit() {
+        clinitCount++;
         return method(EnumSet.of(Flag.STATIC), CLINIT.symbolName(), void.class);
     }
 
@@ -525,6 +591,7 @@ public class ClassEmitter implements Emitter {
      * @return method emitter to use for weaving <init>()V
      */
     MethodEmitter init() {
+        initCount++;
         return method(INIT.symbolName(), void.class);
     }
 
@@ -535,6 +602,7 @@ public class ClassEmitter implements Emitter {
      * @return method emitter to use for weaving <init>()V
      */
     MethodEmitter init(final Class<?>... ptypes) {
+        initCount++;
         return method(INIT.symbolName(), void.class, ptypes);
     }
 
@@ -547,6 +615,7 @@ public class ClassEmitter implements Emitter {
      * @return method emitter to use for weaving <init>(...)V
      */
     MethodEmitter init(final EnumSet<Flag> flags, final Class<?>... ptypes) {
+        initCount++;
         return method(flags, INIT.symbolName(), void.class, ptypes);
     }
 
@@ -561,6 +630,7 @@ public class ClassEmitter implements Emitter {
      * @see ClassEmitter.Flag
      */
     final void field(final EnumSet<Flag> fieldFlags, final String fieldName, final Class<?> fieldType, final Object value) {
+        fieldCount++;
         cw.visitField(Flag.getValue(fieldFlags), fieldName, typeDescriptor(fieldType), null, value).visitEnd();
     }
 
diff --git a/src/jdk/nashorn/internal/codegen/CodeGeneratorLexicalContext.java b/src/jdk/nashorn/internal/codegen/CodeGeneratorLexicalContext.java
index 1176dcc7..08c4b65c 100644
--- a/src/jdk/nashorn/internal/codegen/CodeGeneratorLexicalContext.java
+++ b/src/jdk/nashorn/internal/codegen/CodeGeneratorLexicalContext.java
@@ -31,6 +31,7 @@ import java.util.Collections;
 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 @@ final class CodeGeneratorLexicalContext extends LexicalContext {
 
     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 --git a/src/jdk/nashorn/internal/codegen/CompilationPhase.java b/src/jdk/nashorn/internal/codegen/CompilationPhase.java
index 1137c6ac..603157d6 100644
--- a/src/jdk/nashorn/internal/codegen/CompilationPhase.java
+++ b/src/jdk/nashorn/internal/codegen/CompilationPhase.java
@@ -53,7 +53,6 @@ import jdk.nashorn.internal.AssertsEnabled;
 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 @@ enum CompilationPhase {
         }
     },
 
-    /**
-     * 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<LexicalContext>(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 @@ enum CompilationPhase {
                 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 @@ enum CompilationPhase {
                     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 @@ enum CompilationPhase {
                     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 @@ enum CompilationPhase {
                         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 @@ enum CompilationPhase {
 
             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 @@ enum CompilationPhase {
             }
 
             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 --git a/src/jdk/nashorn/internal/codegen/CompileUnit.java b/src/jdk/nashorn/internal/codegen/CompileUnit.java
index 4b8e308a..d45a58f1 100644
--- a/src/jdk/nashorn/internal/codegen/CompileUnit.java
+++ b/src/jdk/nashorn/internal/codegen/CompileUnit.java
@@ -44,6 +44,8 @@ public final class CompileUnit implements Comparable<CompileUnit> {
 
     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 @@ public final class CompileUnit implements Comparable<CompileUnit> {
         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 @@ public final class CompileUnit implements Comparable<CompileUnit> {
         return isUsed;
     }
 
+    public boolean hasCode() {
+        return (classEmitter.getMethodCount() - classEmitter.getInitCount() - classEmitter.getClinitCount()) > 0;
+    }
+
     /**
      * Tag this compile unit as used
      */
@@ -138,7 +152,8 @@ public final class CompileUnit implements Comparable<CompileUnit> {
 
     @Override
     public String toString() {
-        return "[CompileUnit className=" + shortName(className) + " weight=" + weight + '/' + Splitter.SPLIT_THRESHOLD + ']';
+        final String methods = classEmitter != null ? classEmitter.getMethodNames().toString() : "<anon>";
+        return "[CompileUnit className=" + shortName(className) + " weight=" + weight + '/' + Splitter.SPLIT_THRESHOLD + " hasCode=" + methods + ']';
     }
 
     @Override
diff --git a/src/jdk/nashorn/internal/codegen/Compiler.java b/src/jdk/nashorn/internal/codegen/Compiler.java
index 71f1532a..a7347f71 100644
--- a/src/jdk/nashorn/internal/codegen/Compiler.java
+++ b/src/jdk/nashorn/internal/codegen/Compiler.java
@@ -38,7 +38,6 @@ import java.lang.invoke.MethodType;
 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.concurrent.atomic.AtomicInteger;
 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 @@ public final class Compiler implements Loggable {
                         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 @@ public final class Compiler implements Loggable {
             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<CompilationPhase> list = new LinkedList<>();
             for (final CompilationPhase p : phases) {
@@ -259,7 +257,6 @@ public final class Compiler implements Loggable {
             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<CompilationPhase> list = new LinkedList<>();
             for (final CompilationPhase p : phases) {
@@ -700,16 +697,8 @@ public final class Compiler implements Loggable {
     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 --git a/src/jdk/nashorn/internal/runtime/Timing.java b/src/jdk/nashorn/internal/runtime/Timing.java
index 70eaa353..e1d464d9 100644
--- a/src/jdk/nashorn/internal/runtime/Timing.java
+++ b/src/jdk/nashorn/internal/runtime/Timing.java
@@ -33,6 +33,8 @@ import java.util.List;
 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 @@ public final class Timing implements Loggable {
                 append((int)(knownTime * 100.0 / total)).
                 append("%])");
 
+            sb.append("\n\nEmitted compile units: ").
+                append(CompileUnit.getEmittedUnitCount());
+
             return sb.toString();
         }