Skip to content

Commit

Permalink
Optimize more J2CL clinit patterns.
Browse files Browse the repository at this point in the history 
When clinit X just exists to call another clinit Y, we can replace all clinit X reference with clinit Y which will result in clinit X to be unused and get pruned. And that is exactly what this CL does.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=169185339
  • Loading branch information
@gkdn @blickly
gkdn authored and blickly committed Sep 19, 2017
1 parent ad1dd6f commit 57fd8fa
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 19 deletions.
81 changes: 62 additions & 19 deletions src/com/google/javascript/jscomp/J2clClinitPrunerPass.java
View file
Expand Up @@ -29,14 +29,17 @@
import java.util.Collection; import java.util.Collection;
import java.util.Deque; import java.util.Deque;
import java.util.HashSet; import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List; import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set; import java.util.Set;
import javax.annotation.Nullable; import javax.annotation.Nullable;


/** An optimization pass to prune J2CL clinits. */ /** An optimization pass to prune J2CL clinits. */
public class J2clClinitPrunerPass implements CompilerPass { public class J2clClinitPrunerPass implements CompilerPass {


private final Set<String> emptiedClinitMethods = new HashSet<>(); private final Map<String, Node> emptiedClinitMethods = new LinkedHashMap<>();
private final AbstractCompiler compiler; private final AbstractCompiler compiler;
private final List<Node> changedScopeNodes; private final List<Node> changedScopeNodes;


Expand Down Expand Up @@ -93,6 +96,21 @@ private void pruneEmptyClinits(Node root, List<Node> changedScopes) {
emptiedClinitMethods.clear(); emptiedClinitMethods.clear();
NodeTraversal.traverseEs6ScopeRoots( NodeTraversal.traverseEs6ScopeRoots(
compiler, root, changedScopes, new EmptyClinitPruner(), false); compiler, root, changedScopes, new EmptyClinitPruner(), false);

// Make sure replacements are to final destination instead of pointing intermediate ones.
for (Entry<String, Node> clinitReplacementEntry : emptiedClinitMethods.entrySet()) {
clinitReplacementEntry.setValue(resolveReplacement(clinitReplacementEntry.getValue()));
}
}

private Node resolveReplacement(Node node) {
if (node == null) {
return null;
}
String clinitName = getClinitMethodName(node);
return emptiedClinitMethods.containsKey(clinitName)
? resolveReplacement(emptiedClinitMethods.get(clinitName))
: node;
} }


private Multimap<String, Node> collectClinitReferences(Node root) { private Multimap<String, Node> collectClinitReferences(Node root) {
Expand All @@ -103,7 +121,7 @@ private Multimap<String, Node> collectClinitReferences(Node root) {
new AbstractPostOrderCallback() { new AbstractPostOrderCallback() {
@Override @Override
public void visit(NodeTraversal t, Node node, Node parent) { public void visit(NodeTraversal t, Node node, Node parent) {
String clinitName = node.isCall() ? getClinitMethodName(node.getFirstChild()) : null; String clinitName = getClinitMethodName(node);
if (clinitName != null) { if (clinitName != null) {
clinitReferences.put(clinitName, node); clinitReferences.put(clinitName, node);
} }
Expand All @@ -114,11 +132,23 @@ public void visit(NodeTraversal t, Node node, Node parent) {


private List<Node> cleanEmptyClinitReferences(Multimap<String, Node> clinitReferences) { private List<Node> cleanEmptyClinitReferences(Multimap<String, Node> clinitReferences) {
final List<Node> newChangedScopes = new ArrayList<>(); final List<Node> newChangedScopes = new ArrayList<>();
for (String clinitName : emptiedClinitMethods) { // resolveReplacement step above should not require any particular iteration order of the
// emptiedClinitMethods but we are using LinkedHashMap to be extra safe.
for (Entry<String, Node> clinitReplacementEntry : emptiedClinitMethods.entrySet()) {
String clinitName = clinitReplacementEntry.getKey();
Node replacement = clinitReplacementEntry.getValue();

Collection<Node> references = clinitReferences.removeAll(clinitName); Collection<Node> references = clinitReferences.removeAll(clinitName);
for (Node reference : references) { for (Node reference : references) {
Node changedScope = NodeUtil.getEnclosingChangeScopeRoot(reference.getParent()); Node changedScope = NodeUtil.getEnclosingChangeScopeRoot(reference.getParent());
NodeUtil.deleteFunctionCall(reference, compiler); if (replacement == null) {
NodeUtil.deleteFunctionCall(reference, compiler);
} else {
replacement = replacement.cloneTree();
reference.replaceWith(replacement);
compiler.reportChangeToChangeScope(changedScope);
clinitReferences.put(getClinitMethodName(replacement), replacement);
}
newChangedScopes.add(changedScope); newChangedScopes.add(changedScope);
} }
} }
Expand Down Expand Up @@ -181,7 +211,7 @@ public void visit(NodeTraversal t, Node node, Node parent) {
} }


private void tryRemovingClinit(Node node) { private void tryRemovingClinit(Node node) {
String clinitName = node.isCall() ? getClinitMethodName(node.getFirstChild()) : null; String clinitName = getClinitMethodName(node);
if (clinitName == null) { if (clinitName == null) {
return; return;
} }
Expand Down Expand Up @@ -219,8 +249,7 @@ public void visit(NodeTraversal t, Node node, Node parent) {
} }


// Find clinit calls. // Find clinit calls.
String clinitName = String clinitName = getClinitMethodName(node.getFirstChild());
node.getFirstChild().isCall() ? getClinitMethodName(node.getFirstFirstChild()) : null;
if (clinitName == null) { if (clinitName == null) {
return; return;
} }
Expand Down Expand Up @@ -310,8 +339,7 @@ private boolean callsClinit(Node fnNode, String clinitName) {
Node child = fnNode.getLastChild().getFirstChild(); Node child = fnNode.getLastChild().getFirstChild();
return child != null return child != null
&& child.isExprResult() && child.isExprResult()
&& child.getFirstChild().isCall() && clinitName.equals(getClinitMethodName(child.getFirstChild()));
&& clinitName.equals(getClinitMethodName(child.getFirstFirstChild()));
} }
} }


Expand Down Expand Up @@ -341,18 +369,30 @@ private void trySubstituteEmptyFunction(Node fnNode) {
return; return;
} }


// Ensure that the first expression in the body is setting itself to the empty function and // Ensure that the first expression in the body is setting itself to the empty function
// there are no other expressions.
Node firstExpr = body.getFirstChild(); Node firstExpr = body.getFirstChild();
if (!isAssignToEmptyFn(firstExpr, fnQualifiedName) || firstExpr.getNext() != null) { if (!isAssignToEmptyFn(firstExpr, fnQualifiedName)) {
return; return;
} }


body.removeChild(firstExpr); Node secondExpr = firstExpr.getNext();
NodeUtil.markFunctionsDeleted(firstExpr, compiler); Node replaceReferencesWith;
compiler.reportChangeToEnclosingScope(body); if (secondExpr == null) {
// There are no expressions in clinit so it is noop; remove all references.
replaceReferencesWith = null;
} else if (secondExpr.getNext() == null
&& secondExpr.isExprResult()
&& getClinitMethodName(secondExpr.getFirstChild()) != null) {
// Only expression in clinit is a call to another clinit. We can safely replace all
// references with the other clinit.
replaceReferencesWith = secondExpr.getFirstChild();
} else {
// Clinit is not empty.
return;
}


emptiedClinitMethods.add(fnQualifiedName); emptiedClinitMethods.put(fnQualifiedName, replaceReferencesWith);
NodeUtil.deleteNode(firstExpr, compiler);
} }


private boolean isAssignToEmptyFn(Node node, String enclosingFnName) { private boolean isAssignToEmptyFn(Node node, String enclosingFnName) {
Expand All @@ -370,9 +410,12 @@ private static boolean isClinitMethod(Node node) {
return node.isFunction() && isClinitMethodName(NodeUtil.getName(node)); return node.isFunction() && isClinitMethodName(NodeUtil.getName(node));
} }


private static String getClinitMethodName(Node fnNode) { private static String getClinitMethodName(Node node) {
String fnName = fnNode.getQualifiedName(); if (node.isCall()) {
return isClinitMethodName(fnName) ? fnName : null; String fnName = node.getFirstChild().getQualifiedName();
return isClinitMethodName(fnName) ? fnName : null;
}
return null;
} }


private static boolean isClinitMethodName(String fnName) { private static boolean isClinitMethodName(String fnName) {
Expand Down
31 changes: 31 additions & 0 deletions test/com/google/javascript/jscomp/J2clClinitPrunerPassTest.java
View file
Expand Up @@ -422,6 +422,37 @@ public void testFoldClinit_classHierarchy() {
"someChildClass.someFunction = function() {};")); "someChildClass.someFunction = function() {};"));
} }


public void testFoldClinit_classHierarchyNonEmpty() {
test(
LINE_JOINER.join(
"var someClass = {};",
"someClass.$clinit = function() {",
" someClass.$clinit = function() {};",
" somefn();",
"};",
"var someChildClass = {};",
"someChildClass.$clinit = function() {",
" someChildClass.$clinit = function() {};",
" someClass.$clinit();",
"};",
"someChildClass.someFunction = function() {",
" someChildClass.$clinit();",
"};"),
LINE_JOINER.join(
"var someClass = {};",
"someClass.$clinit = function() {",
" someClass.$clinit = function() {};",
" somefn();",
"};",
"var someChildClass = {};",
"someChildClass.$clinit = function() {",
" someClass.$clinit();",
"};",
"someChildClass.someFunction = function() {",
" someClass.$clinit();",
"};"));
}

public void testFoldClinit_invalidCandidates() { public void testFoldClinit_invalidCandidates() {
testSame( testSame(
LINE_JOINER.join( LINE_JOINER.join(
Expand Down

0 comments on commit 57fd8fa

Please sign in to comment.