Skip to content

Commit

Permalink
Devirtualize duplicate methods
Browse files Browse the repository at this point in the history 
Closes #2041

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=134430055
  • Loading branch information
@blickly
Nick Santos authored and blickly committed Sep 28, 2016
1 parent 0a01f84 commit cc3cc90
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 57 deletions.
30 changes: 28 additions & 2 deletions src/com/google/javascript/jscomp/DevirtualizePrototypeMethods.java
View file
Expand Up @@ -22,7 +22,6 @@
import com.google.javascript.rhino.IR; import com.google.javascript.rhino.IR;
import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.TypeI; import com.google.javascript.rhino.TypeI;

import java.util.Collection; import java.util.Collection;


/** /**
Expand Down Expand Up @@ -87,6 +86,9 @@ public void process(
private static boolean isCall(UseSite site) { private static boolean isCall(UseSite site) {
Node node = site.node; Node node = site.node;
Node parent = node.getParent(); Node parent = node.getParent();
if (parent == null) {
return false;
}
return (parent.getFirstChild() == node) && parent.isCall(); return (parent.getFirstChild() == node) && parent.isCall();
} }


Expand Down Expand Up @@ -275,7 +277,7 @@ private boolean isEligibleDefinition(DefinitionUseSiteFinder defFinder,
// Multiple definitions prevent rewrite. // Multiple definitions prevent rewrite.
Collection<Definition> singleSiteDefinitions = Collection<Definition> singleSiteDefinitions =
defFinder.getDefinitionsReferencedAt(nameNode); defFinder.getDefinitionsReferencedAt(nameNode);
if (singleSiteDefinitions.size() > 1) { if (!allDefinitionsEquivalent(singleSiteDefinitions)) {
return false; return false;
} }
Preconditions.checkState(!singleSiteDefinitions.isEmpty()); Preconditions.checkState(!singleSiteDefinitions.isEmpty());
Expand All @@ -295,6 +297,30 @@ private boolean isEligibleDefinition(DefinitionUseSiteFinder defFinder,
return true; return true;
} }


/** Given a set of method definitions, verify they are the same. */
boolean allDefinitionsEquivalent(Collection<Definition> definitions) {
if (definitions.size() <= 1) {
return true;
}

Definition first = null;
for (Definition definition : definitions) {
if (definition.getRValue() == null) {
return false; // We can't tell if they're all the same.
}

if (first == null) {
first = definition;
continue;
}

if (!compiler.areNodesEqualForInlining(first.getRValue(), definition.getRValue())) {
return false;
}
}
return true;
}

/** /**
* Rewrites object method call sites as calls to global functions * Rewrites object method call sites as calls to global functions
* that take "this" as their first argument. * that take "this" as their first argument.
Expand Down
115 changes: 60 additions & 55 deletions test/com/google/javascript/jscomp/DevirtualizePrototypeMethodsTest.java
View file
Expand Up @@ -22,7 +22,6 @@
import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.Token; import com.google.javascript.rhino.Token;
import com.google.javascript.rhino.jstype.JSType; import com.google.javascript.rhino.jstype.JSType;

import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;


Expand Down Expand Up @@ -232,79 +231,85 @@ public void testNoRewriteNamespaceFunctions() throws Exception {
testSame(source); testSame(source);
} }


/** public void testRewriteIfDuplicates() throws Exception {
* Inputs for multiple definition tests. test(
*/ LINE_JOINER.join(
private static class NoRewriteMultipleDefinitionTestInput { "function A(){}; A.prototype.getFoo = function() { return 1; }; ",
static final String TEMPLATE = ".prototype.foo = function() {}"; "function B(){}; B.prototype.getFoo = function() { return 1; }; ",
static final String SOURCE_A = "a" + TEMPLATE; "var x = Math.random() ? new A() : new B();",
static final String SOURCE_B = "b" + TEMPLATE; "alert(x.getFoo());"),
static final String CALL = "o.foo()"; LINE_JOINER.join(

"function A(){}; ",
static final String SINGLE_DEFINITION_EXPECTED = "var JSCompiler_StaticMethods_getFoo=",
"var JSCompiler_StaticMethods_foo = " + "function(JSCompiler_StaticMethods_getFoo$self){return 1};",
" function(JSCompiler_StaticMethods_foo$self) {};" + "function B(){};",
"JSCompiler_StaticMethods_foo(o)"; "B.prototype.getFoo=function(){return 1};",

"var x = Math.random() ? new A() : new B();",
private NoRewriteMultipleDefinitionTestInput() {} "alert(JSCompiler_StaticMethods_getFoo(x));"));
}

public void testRewriteSingleDefinition1() throws Exception {
test(semicolonJoin(NoRewriteMultipleDefinitionTestInput.SOURCE_A,
NoRewriteMultipleDefinitionTestInput.CALL),
NoRewriteMultipleDefinitionTestInput.SINGLE_DEFINITION_EXPECTED);
}

public void testRewriteSingleDefinition2() throws Exception {
test(semicolonJoin(NoRewriteMultipleDefinitionTestInput.SOURCE_B,
NoRewriteMultipleDefinitionTestInput.CALL),
NoRewriteMultipleDefinitionTestInput.SINGLE_DEFINITION_EXPECTED);
}

public void testNoRewriteMultipleDefinition1() throws Exception {
testSame(semicolonJoin(NoRewriteMultipleDefinitionTestInput.SOURCE_A,
NoRewriteMultipleDefinitionTestInput.SOURCE_A,
NoRewriteMultipleDefinitionTestInput.CALL));
} }


public void testNoRewriteMultipleDefinition2() throws Exception { public void testRewriteIfDuplicatesWithThis() throws Exception {
testSame(semicolonJoin(NoRewriteMultipleDefinitionTestInput.SOURCE_B, test(
NoRewriteMultipleDefinitionTestInput.SOURCE_B, LINE_JOINER.join(
NoRewriteMultipleDefinitionTestInput.CALL)); "function A(){}; A.prototype.getFoo = ",
"function() { return this._foo + 1; }; ",
"function B(){}; B.prototype.getFoo = ",
"function() { return this._foo + 1; }; ",
"var x = Math.random() ? new A() : new B();",
"alert(x.getFoo());"),
LINE_JOINER.join(
"function A(){}; ",
"var JSCompiler_StaticMethods_getFoo=",
"function(JSCompiler_StaticMethods_getFoo$self){",
" return JSCompiler_StaticMethods_getFoo$self._foo + 1",
"};",
"function B(){};",
"B.prototype.getFoo=function(){return this._foo + 1};",
"var x = Math.random() ? new A() : new B();",
"alert(JSCompiler_StaticMethods_getFoo(x));"));
} }


public void testNoRewriteMultipleDefinition3() throws Exception { public void testNoRewriteIfDuplicates() throws Exception {
testSame(semicolonJoin(NoRewriteMultipleDefinitionTestInput.SOURCE_A, testSame(
NoRewriteMultipleDefinitionTestInput.SOURCE_B, LINE_JOINER.join(
NoRewriteMultipleDefinitionTestInput.CALL)); "function A(){}; A.prototype.getFoo = function() { return 1; }; ",
"function B(){}; B.prototype.getFoo = function() { return 2; }; ",
"var x = Math.random() ? new A() : new B();",
"alert(x.getFoo());"));
} }


/** /**
* Inputs for object literal tests. * Inputs for object literal tests.
*/ */
private static class NoRewritePrototypeObjectLiteralsTestInput { private static class NoRewritePrototypeObjectLiteralsTestInput {
static final String REGULAR = "b.prototype.foo = function() {}"; static final String REGULAR = "b.prototype.foo = function() { return 1; }";
static final String OBJ_LIT = "a.prototype = {foo : function() {}}"; static final String OBJ_LIT = "a.prototype = {foo : function() { return 2; }}";
static final String CALL = "o.foo()"; static final String CALL = "o.foo()";


private NoRewritePrototypeObjectLiteralsTestInput() {} private NoRewritePrototypeObjectLiteralsTestInput() {}
} }


public void testRewritePrototypeNoObjectLiterals() throws Exception { public void testRewritePrototypeNoObjectLiterals() throws Exception {
test(semicolonJoin(NoRewritePrototypeObjectLiteralsTestInput.REGULAR, test(
NoRewritePrototypeObjectLiteralsTestInput.CALL), semicolonJoin(
"var JSCompiler_StaticMethods_foo = " + NoRewritePrototypeObjectLiteralsTestInput.REGULAR,
"function(JSCompiler_StaticMethods_foo$self) {};" + NoRewritePrototypeObjectLiteralsTestInput.CALL),
"JSCompiler_StaticMethods_foo(o)"); LINE_JOINER.join(
"var JSCompiler_StaticMethods_foo = ",
"function(JSCompiler_StaticMethods_foo$self) { return 1; };",
"JSCompiler_StaticMethods_foo(o)"));
} }


public void testRewritePrototypeObjectLiterals1() throws Exception { public void testRewritePrototypeObjectLiterals1() throws Exception {
test(semicolonJoin(NoRewritePrototypeObjectLiteralsTestInput.OBJ_LIT, test(
NoRewritePrototypeObjectLiteralsTestInput.CALL), semicolonJoin(
"a.prototype={};" + NoRewritePrototypeObjectLiteralsTestInput.OBJ_LIT,
"var JSCompiler_StaticMethods_foo=" + NoRewritePrototypeObjectLiteralsTestInput.CALL),
"function(JSCompiler_StaticMethods_foo$self){};" + LINE_JOINER.join(
"JSCompiler_StaticMethods_foo(o)"); "a.prototype={};",
"var JSCompiler_StaticMethods_foo=",
"function(JSCompiler_StaticMethods_foo$self){ return 2; };",
"JSCompiler_StaticMethods_foo(o)"));
} }


public void testNoRewritePrototypeObjectLiterals2() throws Exception { public void testNoRewritePrototypeObjectLiterals2() throws Exception {
Expand Down

0 comments on commit cc3cc90

Please sign in to comment.