Skip to content

Commit

Permalink
Rollforward of "Improve the compile time cost for Optimize Calls"
Browse files Browse the repository at this point in the history
Has the following changes from the original:

 - fixed bad code, when a symbol definition had a HOOK or other nontrvial
   expressions (a mismatch between isCandidate and ReferenceMap::getFunctionNodes).
 - fixes regressions where constructor parameters were not being optimized if used in the target of a property set (f.prototype.method = ...).
 - fixes regressions where parameters were not optimized if the symbol was used in a typeof/instanceof expression
 - reverts improvement to constructor optimizations when caused breakage when function constructor properties were used to invoke constructors (until we can decide to ban them or similar).

Improve the compile time cost for Optimize Calls from #1 to google#17 (roughly 25% of its previous time).

Instead of using the DefinitionUseSiteFinder, as simpler data structure is introduced: OptimizeCalls.ReferenceMap.  The ReferenceMap provides...

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=173976261
  • Loading branch information
concavelenz authored and brad4d committed Oct 31, 2017
1 parent 78b30b6 commit 60d2c63
Show file tree
Hide file tree
Showing 15 changed files with 1,599 additions and 902 deletions.
9 changes: 0 additions & 9 deletions src/com/google/javascript/jscomp/AbstractCompiler.java
Original file line number Diff line number Diff line change
Expand Up @@ -281,15 +281,6 @@ static enum MostRecentTypechecker {
*/
abstract <T extends TypeIRegistry> T getGlobalTypeInfo();

/**
* Used by three passes that run in sequence (optimize-returns,
* optimize-parameters, remove-unused-variables), to avoid having them
* recompute it independently.
*/
abstract DefinitionUseSiteFinder getDefinitionFinder();

abstract void setDefinitionFinder(DefinitionUseSiteFinder defFinder);

abstract void setExternExports(String externExports);

/**
Expand Down
13 changes: 0 additions & 13 deletions src/com/google/javascript/jscomp/Compiler.java
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,6 @@ public SourceFile loadSource(String filename) {

public PerformanceTracker tracker;

// Used by optimize-returns, optimize-parameters and remove-unused-variables
private DefinitionUseSiteFinder defFinder = null;

// Types that have been forward declared
private Set<String> forwardDeclaredTypes = new HashSet<>();

Expand Down Expand Up @@ -1688,16 +1685,6 @@ GlobalTypeInfo getGlobalTypeInfo() {
return this.globalTypeInfo;
}

@Override
DefinitionUseSiteFinder getDefinitionFinder() {
return this.defFinder;
}

@Override
void setDefinitionFinder(DefinitionUseSiteFinder defFinder) {
this.defFinder = defFinder;
}

public void maybeSetTracker() {
if (options.getTracerMode().isOn()) {
PrintStream tracerOutput =
Expand Down
33 changes: 11 additions & 22 deletions src/com/google/javascript/jscomp/DefaultPassConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -970,10 +970,6 @@ private List<PassFactory> getMainOptimizationLoop() {
passes.add(inlineProperties);
}

boolean runOptimizeCalls = options.optimizeCalls
|| options.optimizeParameters
|| options.optimizeReturns;

if (options.removeUnusedVars || options.removeUnusedLocalVars) {
if (options.deadAssignmentElimination) {
passes.add(deadAssignmentsElimination);
Expand All @@ -986,19 +982,14 @@ private List<PassFactory> getMainOptimizationLoop() {
passes.add(deadPropertyAssignmentElimination);
}
}
if (!runOptimizeCalls) {
passes.add(getRemoveUnusedVars(PassNames.REMOVE_UNUSED_VARS, false));
}
}

if (runOptimizeCalls) {
if (options.optimizeCalls || options.optimizeParameters || options.optimizeReturns) {
passes.add(optimizeCalls);
// RemoveUnusedVars cleans up after optimizeCalls, so we run it here.
// It has a special name because otherwise PhaseOptimizer would change its
// position in the optimization loop.
if (options.optimizeCalls) {
passes.add(getRemoveUnusedVars("removeUnusedVars_afterOptimizeCalls", true));
}
}

if (options.removeUnusedVars || options.removeUnusedLocalVars) {
passes.add(getRemoveUnusedVars());
}

if (options.j2clPassMode.shouldAddJ2clPasses()) {
Expand Down Expand Up @@ -2749,18 +2740,17 @@ protected CompilerPass create(AbstractCompiler compiler) {
}
};

private PassFactory getRemoveUnusedVars(String name, final boolean modifyCallSites) {
return getRemoveUnusedVars(name, modifyCallSites, false /* isOneTimePass */);
private PassFactory getRemoveUnusedVars() {
return getRemoveUnusedVars(false /* isOneTimePass */);
}

private PassFactory lastRemoveUnusedVars() {
return getRemoveUnusedVars(PassNames.REMOVE_UNUSED_VARS, false, true /* isOneTimePass */);
return getRemoveUnusedVars(true /* isOneTimePass */);
}

private PassFactory getRemoveUnusedVars(
String name, final boolean modifyCallSites, boolean isOneTimePass) {
private PassFactory getRemoveUnusedVars(boolean isOneTimePass) {
/** Removes variables that are never used. */
return new PassFactory(name, isOneTimePass) {
return new PassFactory(PassNames.REMOVE_UNUSED_VARS, isOneTimePass) {
@Override
protected CompilerPass create(AbstractCompiler compiler) {
boolean removeOnlyLocals = options.removeUnusedLocalVars && !options.removeUnusedVars;
Expand All @@ -2769,8 +2759,7 @@ protected CompilerPass create(AbstractCompiler compiler) {
return new RemoveUnusedVars(
compiler,
!removeOnlyLocals,
preserveAnonymousFunctionNames,
modifyCallSites);
preserveAnonymousFunctionNames);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@
* </pre>
*
*/
class DevirtualizePrototypeMethods
implements OptimizeCalls.CallGraphCompilerPass, CompilerPass {
class DevirtualizePrototypeMethods implements CompilerPass {
private final AbstractCompiler compiler;

DevirtualizePrototypeMethods(AbstractCompiler compiler) {
Expand All @@ -73,7 +72,6 @@ public void process(Node externs, Node root) {
process(externs, root, defFinder);
}

@Override
public void process(
Node externs, Node root, DefinitionUseSiteFinder definitions) {
for (DefinitionSite defSite : definitions.getDefinitionSites()) {
Expand Down
Loading

0 comments on commit 60d2c63

Please sign in to comment.