deduplicating lambda methods
B. Blaser
bsrbnd at gmail.com
Wed Mar 21 19:00:15 UTC 2018
On 21 March 2018 at 13:48, Maurizio Cimadamore
<maurizio.cimadamore at oracle.com> wrote:
>
>
> On 21/03/18 12:24, B. Blaser wrote:
>>
>> On 19 March 2018 at 22:31, Maurizio Cimadamore
>> <maurizio.cimadamore at oracle.com> wrote:
>>>
>>> [...]
>>>
>>> Also, a side note: we have learned during condy-folding that rewriting
>>> trees
>>> is not always the right approach - if you are compiling with -g, you
>>> might
>>> need the trees to remain in place (as you want the compiler to perform
>>> less
>>> aggressive folding) - so I'm not sure the compiler will always be able to
>>> rely on constants to be rewritten in the constant folding phase.
>>>
>>> Maurizio
>>
>> This makes me think we could disable dedup with '-g' to preserve full
>> debug-ability (as next), is this partially what you meant?
>
> Yes and no, but I agree that it's better to conservatively disable
> deduplication with -g.
>
> Maurizio
I note also that we may want to skip empty blocs '{}' and statements ';'.
The following examples would then produce only 2 lambda methods
instead of currently 8:
Runnable r = () -> {};
r = () -> { ; };
r = () -> { {} };
r = () -> { { ; } };
r = () -> { int i = 0; if (i==0) ; };
r = () -> { int i = 0; if (i==0) {} };
r = () -> { int i = 0; if (i==0) { ; } };
r = () -> { int i = 0; if (i==0) { {} } };
I had to write a 'NoOpFinder' class (does something like that already
exist?) and I added a default action in 'TreeScanner' for that (see
below).
Does this look reasonable (I did only some quick tests)?
Bernard
$ diff -u src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TreeDiffer.java.r03
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TreeDiffer.java
--- src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TreeDiffer.java.r03
2018-03-19 14:59:49.351288750 +0100
+++ src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TreeDiffer.java
2018-03-21 19:04:46.282040040 +0100
@@ -94,6 +94,8 @@
import java.util.Iterator;
import java.util.Objects;
+import static com.sun.tools.javac.comp.NoOpFinder.isNoOp;
+
/** A visitor that compares two lambda bodies for structural equality. */
public class TreeDiffer extends TreeScanner {
@@ -109,8 +111,9 @@
private boolean result;
public boolean scan(JCTree tree, JCTree parameter) {
- if (tree == null || parameter == null) {
- return tree == null && parameter == null;
+ boolean nop1 = isNoOp(tree), nop2 = isNoOp(parameter);
+ if (nop1 || nop2) {
+ return nop1 && nop2;
}
tree = TreeInfo.skipParens(tree);
parameter = TreeInfo.skipParens(parameter);
@@ -172,6 +175,10 @@
result = index == otherIndex;
return;
}
+ else {
+ result = symbol.name == otherSymbol.name;
+ return;
+ }
}
result = tree.sym == that.sym;
}
$ diff -u src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TreeHasher.java.r03
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TreeHasher.java
--- src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TreeHasher.java.r03
2018-03-19 15:00:02.743120393 +0100
+++ src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TreeHasher.java
2018-03-21 19:04:25.454301881 +0100
@@ -61,7 +61,7 @@
@Override
public void scan(JCTree tree) {
- if (tree == null) {
+ if (tree == null || NoOpFinder.isNoOp(tree)) {
return;
}
tree = TreeInfo.skipParens(tree);
@@ -84,6 +84,10 @@
hash(idx);
return;
}
+ else {
+ hash(tree.sym.name);
+ return;
+ }
}
hash(sym);
}
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/NoOpFinder.java
b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/NoOpFinder.java
new file mode 100644
--- /dev/null
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/NoOpFinder.java
@@ -0,0 +1,29 @@
+package com.sun.tools.javac.comp;
+
+import com.sun.tools.javac.tree.JCTree;
+import com.sun.tools.javac.tree.TreeScanner;
+
+public class NoOpFinder extends TreeScanner {
+
+ public static boolean isNoOp(JCTree tree) {
+ NoOpFinder nop = new NoOpFinder();
+ if (tree != null)
+ nop.scan(tree);
+ return nop.result;
+ }
+
+ private boolean result = true;
+
+ private NoOpFinder() {
+ defaultAction = () -> { result = false; };
+ }
+
+ @Override
+ public void visitSkip(JCTree.JCSkip tree) {
+ }
+
+ @Override
+ public void visitBlock(JCTree.JCBlock tree) {
+ scan(tree.stats);
+ }
+}
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeScanner.java
b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeScanner.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeScanner.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeScanner.java
@@ -43,6 +43,8 @@
*/
public class TreeScanner extends Visitor {
+ protected Runnable defaultAction = () -> {};
+
/** Visitor method: Scan a single node.
*/
public void scan(JCTree tree) {
@@ -63,16 +65,19 @@
****************************************************************************/
public void visitTopLevel(JCCompilationUnit tree) {
+ defaultAction.run();
scan(tree.defs);
}
public void visitPackageDef(JCPackageDecl tree) {
+ defaultAction.run();
scan(tree.annotations);
scan(tree.pid);
}
@Override
public void visitModuleDef(JCModuleDecl tree) {
+ defaultAction.run();
scan(tree.mods);
scan(tree.qualId);
scan(tree.directives);
@@ -80,37 +85,44 @@
@Override
public void visitExports(JCExports tree) {
+ defaultAction.run();
scan(tree.qualid);
scan(tree.moduleNames);
}
@Override
public void visitOpens(JCOpens tree) {
+ defaultAction.run();
scan(tree.qualid);
scan(tree.moduleNames);
}
@Override
public void visitProvides(JCProvides tree) {
+ defaultAction.run();
scan(tree.serviceName);
scan(tree.implNames);
}
@Override
public void visitRequires(JCRequires tree) {
+ defaultAction.run();
scan(tree.moduleName);
}
@Override
public void visitUses(JCUses tree) {
+ defaultAction.run();
scan(tree.qualid);
}
public void visitImport(JCImport tree) {
+ defaultAction.run();
scan(tree.qualid);
}
public void visitClassDef(JCClassDecl tree) {
+ defaultAction.run();
scan(tree.mods);
scan(tree.typarams);
scan(tree.extending);
@@ -119,6 +131,7 @@
}
public void visitMethodDef(JCMethodDecl tree) {
+ defaultAction.run();
scan(tree.mods);
scan(tree.restype);
scan(tree.typarams);
@@ -130,6 +143,7 @@
}
public void visitVarDef(JCVariableDecl tree) {
+ defaultAction.run();
scan(tree.mods);
scan(tree.vartype);
scan(tree.nameexpr);
@@ -137,23 +151,28 @@
}
public void visitSkip(JCSkip tree) {
+ defaultAction.run();
}
public void visitBlock(JCBlock tree) {
+ defaultAction.run();
scan(tree.stats);
}
public void visitDoLoop(JCDoWhileLoop tree) {
+ defaultAction.run();
scan(tree.body);
scan(tree.cond);
}
public void visitWhileLoop(JCWhileLoop tree) {
+ defaultAction.run();
scan(tree.cond);
scan(tree.body);
}
public void visitForLoop(JCForLoop tree) {
+ defaultAction.run();
scan(tree.init);
scan(tree.cond);
scan(tree.step);
@@ -161,31 +180,37 @@
}
public void visitForeachLoop(JCEnhancedForLoop tree) {
+ defaultAction.run();
scan(tree.var);
scan(tree.expr);
scan(tree.body);
}
public void visitLabelled(JCLabeledStatement tree) {
+ defaultAction.run();
scan(tree.body);
}
public void visitSwitch(JCSwitch tree) {
+ defaultAction.run();
scan(tree.selector);
scan(tree.cases);
}
public void visitCase(JCCase tree) {
+ defaultAction.run();
scan(tree.pat);
scan(tree.stats);
}
public void visitSynchronized(JCSynchronized tree) {
+ defaultAction.run();
scan(tree.lock);
scan(tree.body);
}
public void visitTry(JCTry tree) {
+ defaultAction.run();
scan(tree.resources);
scan(tree.body);
scan(tree.catchers);
@@ -193,52 +218,63 @@
}
public void visitCatch(JCCatch tree) {
+ defaultAction.run();
scan(tree.param);
scan(tree.body);
}
public void visitConditional(JCConditional tree) {
+ defaultAction.run();
scan(tree.cond);
scan(tree.truepart);
scan(tree.falsepart);
}
public void visitIf(JCIf tree) {
+ defaultAction.run();
scan(tree.cond);
scan(tree.thenpart);
scan(tree.elsepart);
}
public void visitExec(JCExpressionStatement tree) {
+ defaultAction.run();
scan(tree.expr);
}
public void visitBreak(JCBreak tree) {
+ defaultAction.run();
}
public void visitContinue(JCContinue tree) {
+ defaultAction.run();
}
public void visitReturn(JCReturn tree) {
+ defaultAction.run();
scan(tree.expr);
}
public void visitThrow(JCThrow tree) {
+ defaultAction.run();
scan(tree.expr);
}
public void visitAssert(JCAssert tree) {
+ defaultAction.run();
scan(tree.cond);
scan(tree.detail);
}
public void visitApply(JCMethodInvocation tree) {
+ defaultAction.run();
scan(tree.typeargs);
scan(tree.meth);
scan(tree.args);
}
public void visitNewClass(JCNewClass tree) {
+ defaultAction.run();
scan(tree.encl);
scan(tree.typeargs);
scan(tree.clazz);
@@ -247,6 +283,7 @@
}
public void visitNewArray(JCNewArray tree) {
+ defaultAction.run();
scan(tree.annotations);
scan(tree.elemtype);
scan(tree.dims);
@@ -256,90 +293,110 @@
}
public void visitLambda(JCLambda tree) {
+ defaultAction.run();
scan(tree.body);
scan(tree.params);
}
public void visitParens(JCParens tree) {
+ defaultAction.run();
scan(tree.expr);
}
public void visitAssign(JCAssign tree) {
+ defaultAction.run();
scan(tree.lhs);
scan(tree.rhs);
}
public void visitAssignop(JCAssignOp tree) {
+ defaultAction.run();
scan(tree.lhs);
scan(tree.rhs);
}
public void visitUnary(JCUnary tree) {
+ defaultAction.run();
scan(tree.arg);
}
public void visitBinary(JCBinary tree) {
+ defaultAction.run();
scan(tree.lhs);
scan(tree.rhs);
}
public void visitTypeCast(JCTypeCast tree) {
+ defaultAction.run();
scan(tree.clazz);
scan(tree.expr);
}
public void visitTypeTest(JCInstanceOf tree) {
+ defaultAction.run();
scan(tree.expr);
scan(tree.clazz);
}
public void visitIndexed(JCArrayAccess tree) {
+ defaultAction.run();
scan(tree.indexed);
scan(tree.index);
}
public void visitSelect(JCFieldAccess tree) {
+ defaultAction.run();
scan(tree.selected);
}
public void visitReference(JCMemberReference tree) {
+ defaultAction.run();
scan(tree.expr);
scan(tree.typeargs);
}
public void visitIdent(JCIdent tree) {
+ defaultAction.run();
}
public void visitLiteral(JCLiteral tree) {
+ defaultAction.run();
}
public void visitTypeIdent(JCPrimitiveTypeTree tree) {
+ defaultAction.run();
}
public void visitTypeArray(JCArrayTypeTree tree) {
+ defaultAction.run();
scan(tree.elemtype);
}
public void visitTypeApply(JCTypeApply tree) {
+ defaultAction.run();
scan(tree.clazz);
scan(tree.arguments);
}
public void visitTypeUnion(JCTypeUnion tree) {
+ defaultAction.run();
scan(tree.alternatives);
}
public void visitTypeIntersection(JCTypeIntersection tree) {
+ defaultAction.run();
scan(tree.bounds);
}
public void visitTypeParameter(JCTypeParameter tree) {
+ defaultAction.run();
scan(tree.annotations);
scan(tree.bounds);
}
@Override
public void visitWildcard(JCWildcard tree) {
+ defaultAction.run();
scan(tree.kind);
if (tree.inner != null)
scan(tree.inner);
@@ -347,26 +404,32 @@
@Override
public void visitTypeBoundKind(TypeBoundKind that) {
+ defaultAction.run();
}
public void visitModifiers(JCModifiers tree) {
+ defaultAction.run();
scan(tree.annotations);
}
public void visitAnnotation(JCAnnotation tree) {
+ defaultAction.run();
scan(tree.annotationType);
scan(tree.args);
}
public void visitAnnotatedType(JCAnnotatedType tree) {
+ defaultAction.run();
scan(tree.annotations);
scan(tree.underlyingType);
}
public void visitErroneous(JCErroneous tree) {
+ defaultAction.run();
}
public void visitLetExpr(LetExpr tree) {
+ defaultAction.run();
scan(tree.defs);
scan(tree.expr);
}
>>
>> Bernard
>>
>>
>> $ diff -u
>> ./src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java.r03
>>
>> ./src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
>> ---
>> ./src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java.r03
>> 2018-03-21 12:51:14.338021489 +0100
>> +++
>> ./src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
>> 2018-03-21 12:51:56.209495093 +0100
>> @@ -113,6 +113,8 @@
>> /** force serializable representation, for stress testing **/
>> private final boolean forceSerializable;
>>
>> + private final boolean debug;
>> +
>> /** Flag for alternate metafactories indicating the lambda object
>> is intended to be serializable */
>> public static final int FLAG_SERIALIZABLE = 1 << 0;
>>
>> @@ -149,6 +151,7 @@
>> dumpLambdaToMethodStats =
>> options.isSet("debug.dumpLambdaToMethodStats");
>> attr = Attr.instance(context);
>> forceSerializable = options.isSet("forceSerializable");
>> + debug = options.isSet(Option.G) ||
>> options.isSet(Option.G_CUSTOM, "source");
>> }
>> // </editor-fold>
>>
>> @@ -365,7 +368,7 @@
>> lambdaDecl.body = translate(makeLambdaBody(tree, lambdaDecl));
>>
>> boolean dedupe = false;
>> - if (!localContext.isSerializable()) {
>> + if (!localContext.isSerializable() && !debug) {
>> DedupedLambda dedupedLambda = new
>> DedupedLambda(lambdaDecl.sym, lambdaDecl.body);
>> DedupedLambda existing =
>> kInfo.dedupedLambdas.putIfAbsent(dedupedLambda, dedupedLambda);
>> if (existing != null) {
>
>
More information about the amber-dev
mailing list