Skip to content

Commit

Permalink
Optimization of Array.concat
Browse files Browse the repository at this point in the history 
-Replace empty array literal from the front of concatenation
by the first argument of concat function call.
-Add folding of chained concat functions.

Cleanup Array concat optimizations.

Closes #3386.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=257672795
  • Loading branch information
@qqingvarqq @tjgq
qqingvarqq authored and tjgq committed Jul 13, 2019
1 parent 4f59320 commit 40fe503
Show file tree
Hide file tree
Showing 2 changed files with 212 additions and 13 deletions.
146 changes: 141 additions & 5 deletions src/com/google/javascript/jscomp/PeepholeReplaceKnownMethods.java
View file
Expand Up @@ -24,9 +24,11 @@
import com.google.common.collect.ImmutableList;
import com.google.javascript.rhino.IR;
import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.jstype.JSType;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import javax.annotation.Nullable;

/**
* Just to fold known methods when they are called with constants.
Expand Down Expand Up @@ -59,15 +61,16 @@ Node optimizeSubtree(Node subtree) {
private Node tryFoldKnownMethods(Node subtree) {
// For now we only support string methods .join(),
// .indexOf(), .substring() and .substr()
// array method concat()
// and numeric methods parseInt() and parseFloat().

checkArgument(subtree.isCall(), subtree);
subtree = tryFoldArrayJoin(subtree);

// tryFoldArrayJoin may return a string literal instead of a CALL node
if (subtree.isCall()) {
Node callTarget = subtree.getFirstChild();
if (callTarget == null) {
return subtree;
}
subtree = tryToFoldArrayConcat(subtree);
checkState(subtree.isCall(), subtree);
Node callTarget = checkNotNull(subtree.getFirstChild());

if (NodeUtil.isGet(callTarget)) {
if (isASTNormalized() && callTarget.getFirstChild().isQualifiedName()) {
Expand Down Expand Up @@ -956,4 +959,137 @@ private Node tryFoldStringSplit(Node n, Node stringNode, Node arg1) {
reportChangeToEnclosingScope(parent);
return arrayOfStrings;
}

private Node tryToFoldArrayConcat(Node n) {
checkArgument(n.isCall(), n);

if (!isASTNormalized() || !useTypes) {
return n;
}
ConcatFunctionCall concatFunctionCall = createConcatFunctionCallForNode(n);
if (concatFunctionCall == null) {
return n;
}
concatFunctionCall = tryToRemoveArrayLiteralFromFrontOfConcat(concatFunctionCall);
checkNotNull(concatFunctionCall);
return tryToFoldConcatChaining(concatFunctionCall);
}

/**
* Check if we have this code pattern `[].concat(exactlyArrayArgument,...*)` and if yes replace
* empty array literal from the front of concatenation by the first argument of concat function
* call `[].concat(arr,1)` -> `arr.concat(1)`.
*/
private ConcatFunctionCall tryToRemoveArrayLiteralFromFrontOfConcat(
ConcatFunctionCall concatFunctionCall) {
checkNotNull(concatFunctionCall);

Node callNode = concatFunctionCall.callNode;
Node arrayLiteralToRemove = concatFunctionCall.calleeNode;
if (!arrayLiteralToRemove.isArrayLit() || arrayLiteralToRemove.hasChildren()) {
return concatFunctionCall;
}
Node firstArg = concatFunctionCall.firstArgumentNode;
if (!containsExactlyArray(firstArg)) {
return concatFunctionCall;
}

callNode.removeChild(firstArg);
Node currentTarget = callNode.getFirstChild();
currentTarget.replaceChild(arrayLiteralToRemove, firstArg);

reportChangeToEnclosingScope(callNode);
return createConcatFunctionCallForNode(callNode);
}

/**
* Check if we have this code pattern `array.concat(...*).concat(sideEffectFreeArguments)` and if
* yes fold chained concat functions, so `arr.concat(a).concat(b)` will be fold into
* `arr.concat(a,b)`.
*/
private Node tryToFoldConcatChaining(ConcatFunctionCall concatFunctionCall) {
checkNotNull(concatFunctionCall);

Node concatCallNode = concatFunctionCall.callNode;

Node maybeFunctionCall = concatFunctionCall.calleeNode;
if (!maybeFunctionCall.isCall()) {
return concatCallNode;
}
ConcatFunctionCall previousConcatFunctionCall =
createConcatFunctionCallForNode(maybeFunctionCall);
if (previousConcatFunctionCall == null) {
return concatCallNode;
}
// make sure that arguments in second concat function call can't change the array
// so we can fold chained concat functions
// to clarify, consider this code
// here we can't fold concatenation
// var a = [];
// a.concat(1).concat(a.push(1)); -> [1,1]
// a.concat(1,a.push(1)); -> [1,1,1]
for (Node arg = concatFunctionCall.firstArgumentNode; arg != null; arg = arg.getNext()) {
if (mayHaveSideEffects(arg)) {
return concatCallNode;
}
}

// perform folding
Node previousConcatCallNode = previousConcatFunctionCall.callNode;
Node arg = concatFunctionCall.firstArgumentNode;
while (arg != null) {
Node currentArg = arg;
arg = arg.getNext();
previousConcatCallNode.addChildToBack(currentArg.detach());
}
concatCallNode.replaceWith(previousConcatCallNode.detach());
reportChangeToEnclosingScope(previousConcatCallNode);
return previousConcatCallNode;
}

private abstract static class ConcatFunctionCall {
private final Node callNode;
private final Node calleeNode;
@Nullable private final Node firstArgumentNode;

ConcatFunctionCall(Node callNode, Node calleeNode, Node firstArgumentNode) {
this.callNode = checkNotNull(callNode);
this.calleeNode = checkNotNull(calleeNode);
this.firstArgumentNode = firstArgumentNode;
}
}

/**
* If the argument node is a call to `Array.prototype.concat`, then return a `ConcatFunctionCall`
* object for it, otherwise return `null`.
*/
@Nullable
private static ConcatFunctionCall createConcatFunctionCallForNode(Node n) {
checkArgument(n.isCall(), n);
Node callTarget = checkNotNull(n.getFirstChild());
if (!callTarget.isGetProp()) {
return null;
}
Node functionName = callTarget.getSecondChild();
if (functionName == null || !functionName.getString().equals("concat")) {
return null;
}
Node calleNode = callTarget.getFirstChild();
if (!containsExactlyArray(calleNode)) {
return null;
}
Node firstArgumentNode = n.getSecondChild();
return new ConcatFunctionCall(n, calleNode, firstArgumentNode) {};
}

/** Check if a node contains an array type or function call that returns only an array. */
private static boolean containsExactlyArray(Node n) {
if (n == null || n.getJSType() == null) {
return false;
}
JSType nodeType = n.getJSType();
return (nodeType.isArrayType()
|| (nodeType.isTemplatizedType()
&& nodeType.toMaybeTemplatizedType().getReferencedType().isArrayType()));
}
}
79 changes: 71 additions & 8 deletions test/com/google/javascript/jscomp/PeepholeReplaceKnownMethodsTest.java
View file
Expand Up @@ -33,14 +33,26 @@ public final class PeepholeReplaceKnownMethodsTest extends CompilerTestCase {
private boolean useTypes = true;

public PeepholeReplaceKnownMethodsTest() {
super(MINIMAL_EXTERNS + lines(
// NOTE: these are defined as variadic to avoid wrong-argument-count warnings in NTI,
// which enables testing that the pass does not touch calls with wrong argument count.
"/** @type {function(this: string, ...*): string} */ String.prototype.substring;",
"/** @type {function(this: string, ...*): string} */ String.prototype.substr;",
"/** @type {function(this: string, ...*): string} */ String.prototype.slice;",
"/** @type {function(this: string, ...*): string} */ String.prototype.charAt;",
"/** @type {function(this: Array, ...*): !Array} */ Array.prototype.slice;"));
super(
MINIMAL_EXTERNS
+ lines(
// NOTE: these are defined as variadic to avoid wrong-argument-count warnings in
// NTI,
// which enables testing that the pass does not touch calls with wrong argument
// count.
"/** @type {function(this: string, ...*): string} */ String.prototype.substring;",
"/** @type {function(this: string, ...*): string} */ String.prototype.substr;",
"/** @type {function(this: string, ...*): string} */ String.prototype.slice;",
"/** @type {function(this: string, ...*): string} */ String.prototype.charAt;",
"/** @type {function(this: Array, ...*): !Array} */ Array.prototype.slice;",
"/** @type {function(this: Array, ...*): !Array<?>} */ Array.prototype.concat;",
"/** @type {function(this: Array, ...*): !Array<?>} */ function returnArrayType()"
+ " {}",
"/** @type {function(this: Array, ...*): !Array<?>|string} */ function"
+ " returnUnionType(){}",
"/** @constructor */ function Foo(){}",
"/** @type {function(this: Foo, ...*): !Foo} */ Foo.prototype.concat",
"var obj = new Foo();"));
}

@Override
Expand Down Expand Up @@ -609,6 +621,57 @@ public void testReplaceWithCharAt() {
foldSameStringTyped("''.substring(i, i + 1)");
}

@Test
public void testFoldConcatChaining() {
enableNormalize();
enableTypeCheck();

fold("[1,2].concat(1).concat(2,['abc']).concat('abc')", "[1,2].concat(1,2,['abc'],'abc')");
fold("[].concat(['abc']).concat(1).concat([2,3])", "['abc'].concat(1,[2,3])");

// because function returnArrayType() or returnUnionType()
// possibly can produce a side effects
// we can't fold all concatenation chaining
fold(
"returnArrayType().concat(returnArrayType()).concat(1).concat(2)",
"returnArrayType().concat(returnArrayType(),1,2)");
fold(
"returnArrayType().concat(returnUnionType()).concat(1).concat(2)",
"returnArrayType().concat(returnUnionType(),1,2)");
fold(
"[1,2,1].concat(1).concat(returnArrayType()).concat(2)",
"[1,2,1].concat(1).concat(returnArrayType(),2)");
fold(
"[1].concat(1).concat(2).concat(returnArrayType())",
"[1].concat(1,2).concat(returnArrayType())");
foldSame("[].concat(1).concat(returnArrayType())");
foldSame("obj.concat([1,2]).concat(1)");
}

@Test
public void testRemoveArrayLiteralFromFrontOfConcat() {
enableNormalize();
enableTypeCheck();

fold("[].concat([1,2,3],1)", "[1,2,3].concat(1)");
fold("[].concat(returnArrayType(),1)", "returnArrayType().concat(1)");

foldSame("[1,2,3].concat(returnArrayType())");
foldSame("returnArrayType().concat([1,2,3])");
// Call method with the same name as Array.prototype.concat
foldSame("obj.concat([1,2,3])");

foldSame("[].concat(1,[1,2,3])");
foldSame("[].concat(returnUnionType())");
foldSame("[].concat(1)");
fold("[].concat([1])", "[1].concat()");
fold("[].concat(returnArrayType())", "returnArrayType().concat()");

// Chained folding of empty array lit
fold("[].concat([], [1,2,3], [4])", "[1,2,3].concat([4])");
fold("[].concat([]).concat([1]).concat([2,3])", "[1].concat([2,3])");
}

private void foldSame(String js) {
testSame(js);
}
Expand Down

0 comments on commit 40fe503

Please sign in to comment.