Skip to content

Commit

Permalink
Allocate fewer VisitorState objects
Browse files Browse the repository at this point in the history
Most of these can be shared among matchers for the AST node they're
processing - the only thing that (rarely) differs is the
SuppressionState.  Adding a check to decide whether we need to
allocate a new object reduces time spent allocating (including the new
time spent deciding) by 75%, from around 1.2% of javac time to 0.35% of
javac time.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=258845118
  • Loading branch information
amalloy authored and nick-someone committed Jul 24, 2019
1 parent 01515b0 commit 3e5af8e
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 6 deletions.
16 changes: 16 additions & 0 deletions check_api/src/main/java/com/google/errorprone/VisitorState.java
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ public VisitorState withPath(TreePath path) {
sharedState);
}

@Deprecated // TODO(amalloy): Delete after next error-prone release.
public VisitorState withPathAndSuppression(TreePath path, SuppressedState suppressedState) {
return new VisitorState(
context,
Expand All @@ -232,6 +233,21 @@ public VisitorState withPathAndSuppression(TreePath path, SuppressedState suppre
sharedState);
}

public VisitorState withSuppression(SuppressedState suppressedState) {
if (suppressedState == this.suppressedState) {
return this;
}
return new VisitorState(
context,
descriptionListener,
severityMap,
errorProneOptions,
statisticsCollector,
path,
suppressedState,
sharedState);
}

public TreePath getPath() {
return path;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -421,15 +421,16 @@ private interface TreeProcessor<M extends Suppressible, T extends Tree> {
private <M extends Suppressible, T extends Tree> VisitorState processMatchers(
Iterable<M> matchers, T tree, TreeProcessor<M, T> processingFunction, VisitorState oldState) {
ErrorProneOptions errorProneOptions = oldState.errorProneOptions();
// A VisitorState with our new path, but without mentioning the suppression of any matcher.
VisitorState newState = oldState.withPath(getCurrentPath());
for (M matcher : matchers) {
SuppressedState suppressed = isSuppressed(matcher, errorProneOptions);
// If the ErrorProneOptions say to visit suppressed code, we still visit it
if (suppressed == SuppressedState.UNSUPPRESSED
|| errorProneOptions.isIgnoreSuppressionAnnotations()) {
try (AutoCloseable unused = oldState.timingSpan(matcher)) {
// We create a new VisitorState with the suppression info specific to this matcher.
VisitorState stateWithSuppressionInformation =
oldState.withPathAndSuppression(getCurrentPath(), suppressed);
VisitorState stateWithSuppressionInformation = newState.withSuppression(suppressed);
reportMatch(
processingFunction.process(matcher, tree, stateWithSuppressionInformation),
stateWithSuppressionInformation);
Expand All @@ -438,10 +439,7 @@ private <M extends Suppressible, T extends Tree> VisitorState processMatchers(
}
}
}

// Return a VisitorState with our new path, but without mentioning the suppression of any
// matcher.
return oldState.withPath(getCurrentPath());
return newState;
}

@Override
Expand Down

0 comments on commit 3e5af8e

Please sign in to comment.