Skip to content

Commit

Permalink
Some minor cleanup/performance fixes
Browse files Browse the repository at this point in the history
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=163748601
  • Loading branch information
tbreisacher authored and dimvar committed Jul 31, 2017
1 parent dda778d commit 87084b2
Show file tree
Hide file tree
Showing 9 changed files with 17 additions and 20 deletions.
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/AbstractCompiler.java
Expand Up @@ -224,7 +224,7 @@ static enum MostRecentTypechecker {
/** /**
* Logs a message under a central logger. * Logs a message under a central logger.
*/ */
abstract void addToDebugLog(String message); abstract void addToDebugLog(String... message);


/** /**
* Sets the CssRenamingMap. * Sets the CssRenamingMap.
Expand Down
7 changes: 4 additions & 3 deletions src/com/google/javascript/jscomp/Compiler.java
Expand Up @@ -2809,11 +2809,12 @@ public boolean hasErrors() {


/** Called from the compiler passes, adds debug info */ /** Called from the compiler passes, adds debug info */
@Override @Override
void addToDebugLog(String str) { void addToDebugLog(String... strings) {
if (options.useDebugLog) { if (options.useDebugLog) {
debugLog.append(str); String log = Joiner.on("").join(strings);
debugLog.append(log);
debugLog.append('\n'); debugLog.append('\n');
logger.fine(str); logger.fine(log);
} }
} }


Expand Down
13 changes: 5 additions & 8 deletions src/com/google/javascript/jscomp/FunctionInjector.java
Expand Up @@ -802,8 +802,7 @@ boolean inliningLowersCost(


// Check if any of the references cross the module boundaries. // Check if any of the references cross the module boundaries.
if (checkModules && ref.module != null) { if (checkModules && ref.module != null) {
if (ref.module != fnModule && if (ref.module != fnModule && !moduleGraph.dependsOn(ref.module, fnModule)) {
!moduleGraph.dependsOn(ref.module, fnModule)) {
// Calculate the cost as if the function were non-removable, // Calculate the cost as if the function were non-removable,
// if it still lowers the cost inline it. // if it still lowers the cost inline it.
isRemovable = false; isRemovable = false;
Expand All @@ -812,17 +811,15 @@ boolean inliningLowersCost(
} }
} }


int referencesUsingDirectInlining = referenceCount - int referencesUsingDirectInlining = referenceCount - referencesUsingBlockInlining;
referencesUsingBlockInlining;


// Don't bother calculating the cost of function for simple functions where // Don't bother calculating the cost of function for simple functions where
// possible. // possible.
// However, when inlining a complex function, even a single reference may be // However, when inlining a complex function, even a single reference may be
// larger than the original function if there are many returns (resulting // larger than the original function if there are many returns (resulting
// in additional assignments) or many parameters that need to be aliased // in additional assignments) or many parameters that need to be aliased
// so use the cost estimating. // so use the cost estimating.
if (referenceCount == 1 && isRemovable && if (referenceCount == 1 && isRemovable && referencesUsingDirectInlining == 1) {
referencesUsingDirectInlining == 1) {
return true; return true;
} }


Expand Down Expand Up @@ -904,8 +901,8 @@ private static int inlineCostDelta(
// "function xx(xx,xx){}" (15 + (param count * 3) -1; // "function xx(xx,xx){}" (15 + (param count * 3) -1;
int paramCount = NodeUtil.getFunctionParameters(fnNode).getChildCount(); int paramCount = NodeUtil.getFunctionParameters(fnNode).getChildCount();
int commaCount = (paramCount > 1) ? paramCount - 1 : 0; int commaCount = (paramCount > 1) ? paramCount - 1 : 0;
int costDeltaFunctionOverhead = 15 + commaCount + int costDeltaFunctionOverhead =
(paramCount * InlineCostEstimator.ESTIMATED_IDENTIFIER_COST); 15 + commaCount + (paramCount * InlineCostEstimator.ESTIMATED_IDENTIFIER_COST);


Node block = fnNode.getLastChild(); Node block = fnNode.getLastChild();
if (!block.hasChildren()) { if (!block.hasChildren()) {
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/InlineFunctions.java
Expand Up @@ -630,7 +630,7 @@ private void inlineFunction(NodeTraversal t, Reference ref, FunctionState functi
if (!newExpr.isEquivalentTo(ref.callNode)) { if (!newExpr.isEquivalentTo(ref.callNode)) {
t.getCompiler().reportChangeToEnclosingScope(newExpr); t.getCompiler().reportChangeToEnclosingScope(newExpr);
} }
t.getCompiler().addToDebugLog("Inlined function: " + fn.getName()); t.getCompiler().addToDebugLog("Inlined function: ", fn.getName());
} }
} }


Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/Normalize.java
Expand Up @@ -209,7 +209,7 @@ public void process(Node externs, Node root) {
@Override @Override
public void visit(NodeTraversal t, Node n, Node parent) { public void visit(NodeTraversal t, Node n, Node parent) {
// Note: Constant properties annotations are not propagated. // Note: Constant properties annotations are not propagated.
if ((n.isName() || n.isStringKey())) { if (n.isName() || n.isStringKey()) {
if (n.getString().isEmpty()) { if (n.getString().isEmpty()) {
return; return;
} }
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/ProcessDefines.java
Expand Up @@ -129,7 +129,7 @@ private void overrideDefines(Map<String, DefineInfo> allDefines) {
Node finalValue = inputValue != null ? Node finalValue = inputValue != null ?
inputValue : info.getLastValue(); inputValue : info.getLastValue();
if (finalValue != info.initialValue) { if (finalValue != info.initialValue) {
compiler.addToDebugLog("Overriding @define variable " + defineName); compiler.addToDebugLog("Overriding @define variable ", defineName);
boolean changed = boolean changed =
finalValue.getToken() != info.initialValue.getToken() finalValue.getToken() != info.initialValue.getToken()
|| !finalValue.isEquivalentTo(info.initialValue); || !finalValue.isEquivalentTo(info.initialValue);
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/RemoveUnusedVars.java
Expand Up @@ -889,7 +889,7 @@ private void removeUnreferencedVars() {
// to other unreferenced variables. // to other unreferenced variables.
removeAllAssigns(var); removeAllAssigns(var);


compiler.addToDebugLog("Unreferenced var: " + var.name); compiler.addToDebugLog("Unreferenced var: ", var.name);
Node nameNode = var.nameNode; Node nameNode = var.nameNode;
Node toRemove = nameNode.getParent(); Node toRemove = nameNode.getParent();
Node parent = toRemove.getParent(); Node parent = toRemove.getParent();
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/RenameLabels.java
Expand Up @@ -169,7 +169,7 @@ public boolean shouldTraverse(NodeTraversal nodeTraversal, Node node,
} }


String newName = getNameForId(currentDepth); String newName = getNameForId(currentDepth);
compiler.addToDebugLog("label renamed: " + name + " => " + newName); compiler.addToDebugLog("label renamed: ", name, " => ", newName);
} }


return true; return true;
Expand Down
5 changes: 2 additions & 3 deletions test/com/google/javascript/jscomp/IntegrationTest.java
Expand Up @@ -3185,8 +3185,7 @@ public void testIssue1198() throws Exception {
// http://blickly.github.io/closure-compiler-issues/#1131 // http://blickly.github.io/closure-compiler-issues/#1131
public void testIssue1131() { public void testIssue1131() {
CompilerOptions options = createCompilerOptions(); CompilerOptions options = createCompilerOptions();
CompilationLevel.ADVANCED_OPTIMIZATIONS CompilationLevel.ADVANCED_OPTIMIZATIONS.setOptionsForCompilationLevel(options);
.setOptionsForCompilationLevel(options);
test(options, test(options,
"function f(k) { return k(k); } alert(f(f));", "function f(k) { return k(k); } alert(f(f));",
"function a(b) { return b(b); } alert(a(a));"); "function a(b) { return b(b); } alert(a(a));");
Expand Down Expand Up @@ -3311,7 +3310,7 @@ public void process(Node externs, Node root) {
"function f() { return x + z; }"); "function f() { return x + z; }");
fail("Expected run-time exception"); fail("Expected run-time exception");
} catch (RuntimeException e) { } catch (RuntimeException e) {
assertThat(e.getMessage()).contains("Unexpected variable x"); assertThat(e).hasMessageThat().contains("Unexpected variable x");
} }
} }


Expand Down

0 comments on commit 87084b2

Please sign in to comment.