Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speed up Refaster bug checker #261

Draft
wants to merge 55 commits into
base: master
Choose a base branch
from
Draft

Speed up Refaster bug checker #261

Show file tree
Hide file tree
Changes from 53 commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
69386f0
Emit website link along with Refaster refactor suggestions
Stephan202 Sep 21, 2022
84061fc
More extensible approach
Stephan202 Sep 22, 2022
827880b
WIP: Some plumbing for annotation support
Stephan202 Sep 22, 2022
0f44844
Another round
Stephan202 Sep 22, 2022
0e46f9c
Tweaks
Stephan202 Sep 23, 2022
914d30a
Better annotation support, simplify setup
Stephan202 Sep 23, 2022
458fb99
Flag why build currently doesn't fail, while it should.
Stephan202 Sep 23, 2022
db8cf3c
Improve logic and test coverage
Stephan202 Sep 24, 2022
abb6cea
Properly document URL placeholder usage
Stephan202 Sep 24, 2022
cf772c4
Expand test coverage
Stephan202 Sep 24, 2022
d26bc18
Flag classpath issue
Stephan202 Sep 25, 2022
01b7e5b
Improve text and minor improvements
rickie Sep 27, 2022
3482641
Use `@AutoValue` for the `AnnotatedCompositeCodeTransformer`
rickie Sep 29, 2022
12d09ad
Support `AllSuggestionsAsWarnings` and add a suggestion
rickie Sep 29, 2022
74d6c9a
Tweak
rickie Sep 29, 2022
c4b6a5f
Suggestions, introduce `ErrorProneFork`
Stephan202 Sep 29, 2022
d899a8a
Also this
Stephan202 Sep 29, 2022
20d194b
Clarify how the default Refaster hit severity comes to be
Stephan202 Sep 30, 2022
ad1c98d
Align documentation of reported description names by the Refaster check
Badbond Sep 30, 2022
50443d1
Suggestion
Badbond Sep 30, 2022
7e49b08
Pass null for urlLink to Description.Builder
Badbond Sep 30, 2022
594f51c
Tweaks
Stephan202 Sep 30, 2022
e6caceb
Move `AnnotatedCompositeCodeTransformer` and `ErrorProneFork` to `ref…
rickie Oct 4, 2022
b2ef631
`s/information/content/`
rickie Oct 4, 2022
8bd88bb
Improve Javadoc `AnnotatedCompositeCodeTransformer`
rickie Oct 4, 2022
302e20b
Revert changes in `OptionalTemplates`
rickie Oct 4, 2022
32300ff
Tweak `AnnotatedCompositeCodeTransformer` Javadoc
Badbond Oct 4, 2022
d21ac59
Apply `StringJoin` suggestion
Stephan202 Oct 7, 2022
0484762
Suggestions
Stephan202 Oct 8, 2022
1624ebf
WIP: Speed up `Refaster` check
Stephan202 May 1, 2022
ee74279
Compatibility with stock Error Prone
Stephan202 Sep 12, 2022
344f4e4
Initial copy over of the improved algorithm
rickie Sep 23, 2022
6739cb4
Improve code and algorithm for Refaster
rickie Sep 23, 2022
72ff8ae
Drop unnecessary `@AutoService` annotation
rickie Sep 23, 2022
39dc9aa
Add XXXs
rickie Sep 23, 2022
2a93011
Merge `RefasterRuleSelector` type hierarchy
Stephan202 Sep 25, 2022
1c5077f
Create selector only once per `Refaster` instantiation
Stephan202 Sep 25, 2022
2c137c0
Move all `RefasterRuleSelector` construction logic into the relevant …
Stephan202 Sep 25, 2022
c3a5106
Push sorting requirements into `Node`, minimize tree, add tests
Stephan202 Sep 25, 2022
9151287
Suggestion
rickie Sep 27, 2022
a27d635
Extract the `TreeScanner`s to their own classes
rickie Sep 27, 2022
2003bd2
Introduce `getClass` method to deduplicate
rickie Sep 27, 2022
7889148
Reorder methods in `RefasterIntrospection`
rickie Sep 27, 2022
afdcf52
Apply some suggestions
Stephan202 Sep 30, 2022
ab03739
Optimize code, introduce benchmark, simplify test
Stephan202 Oct 1, 2022
457a8e2
Comment style, explain both performance-only pieces of code
Stephan202 Oct 2, 2022
63ad14e
Fix typo
rickie Oct 4, 2022
0cf891c
Suggestions
ibabiankou Oct 7, 2022
2cb4bd0
Suggestions
Stephan202 Oct 8, 2022
c34fa4d
Fix typo
rickie Oct 9, 2022
d59b626
Merge remote-tracking branch 'origin/sschroevers/refaster-custom-urls…
Stephan202 Oct 9, 2022
5ba7075
Introduce `AnnotatedCompositeCodeTransformerTest`
Stephan202 Oct 9, 2022
0157692
Merge remote-tracking branch 'origin/sschroevers/refaster-custom-urls…
Stephan202 Oct 9, 2022
72f8881
Suggestions
Stephan202 Oct 9, 2022
77bc107
Merge branch 'master' into sschroevers/refaster-speed-up
Stephan202 Dec 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 132 additions & 7 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,13 @@
<version.error-prone-slf4j>0.1.15</version.error-prone-slf4j>
<version.guava-beta-checker>1.0</version.guava-beta-checker>
<version.jdk>11</version.jdk>
<version.jmh>1.35</version.jmh>
<version.maven>3.8.6</version.maven>
<version.mockito>4.8.0</version.mockito>
<version.nopen-checker>1.0.1</version.nopen-checker>
<!-- XXX: Consider providing our own implementation with similar
functionality, and designing it such that JMH classes would not need to
be annotated `@Open`. -->
<version.nopen>1.0.1</version.nopen>
<version.nullaway>0.10.2</version.nullaway>
<!-- XXX: Two other dependencies are potentially of interest:
`com.palantir.assertj-automation:assertj-refaster-rules` and
Expand Down Expand Up @@ -272,12 +276,10 @@
<artifactId>truth</artifactId>
<version>1.1.3</version>
</dependency>
<!-- Specified as a workaround for
https://github.com/mojohaus/versions-maven-plugin/issues/244. -->
<dependency>
<groupId>com.jakewharton.nopen</groupId>
<artifactId>nopen-checker</artifactId>
<version>${version.nopen-checker}</version>
<artifactId>nopen-annotations</artifactId>
<version>${version.nopen}</version>
</dependency>
<dependency>
<groupId>com.newrelic.agent.java</groupId>
Expand Down Expand Up @@ -401,6 +403,11 @@
<type>pom</type>
<scope>import</scope>
</dependency>
<dependency>
<groupId>org.openjdk.jmh</groupId>
<artifactId>jmh-core</artifactId>
<version>${version.jmh}</version>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
Expand Down Expand Up @@ -475,7 +482,6 @@
<configuration>
<bundledSignatures>
<bundledSignature>jdk-internal</bundledSignature>
<bundledSignature>jdk-reflection</bundledSignature>
<bundledSignature>jdk-system-out</bundledSignature>
<!-- Other bundles are available but currently not
enabled:
Expand All @@ -485,6 +491,10 @@
- jdk-deprecated: we compile with `-Xlint:all`,
which causes the build to fail when _any_
deprecated method is called.
- jdk-reflection: this bundle should probably be
enabled, but currently
`java.lang.reflect.AccessibleObject#setAccessible`
is still called in various places.
- jdk-non-portable: the Error Prone integration
crucially relies on some of these APIs.
- jdk-unsafe: see
Expand Down Expand Up @@ -622,6 +632,10 @@
</module>
<module name="IllegalIdentifierName" />
<module name="IllegalImport">
<property name="illegalClasses" value="com.google.auto.common.MoreStreams">
<!-- Instead, please use Guava's
`java.util.stream.Collector`s. -->
</property>
<property name="illegalClasses" value="com.mongodb.lang.Nullable">
<!-- Instead, please use
`javax.annotation.Nullable`. -->
Expand Down Expand Up @@ -821,6 +835,11 @@
<artifactId>error_prone_core</artifactId>
<version>${version.error-prone}</version>
</path>
<path>
<groupId>com.google.auto.value</groupId>
<artifactId>auto-value</artifactId>
<version>${version.auto-value}</version>
</path>
<path>
<groupId>com.google.auto.service</groupId>
<artifactId>auto-service</artifactId>
Expand All @@ -834,7 +853,7 @@
<path>
<groupId>com.jakewharton.nopen</groupId>
<artifactId>nopen-checker</artifactId>
<version>${version.nopen-checker}</version>
<version>${version.nopen}</version>
</path>
<!-- XXX: Before enabling these plugins we'll
need to resolve some violations. Some of the
Expand Down Expand Up @@ -865,6 +884,11 @@
<artifactId>mockito-errorprone</artifactId>
<version>${version.mockito}</version>
</path>
<path>
<groupId>org.openjdk.jmh</groupId>
<artifactId>jmh-generator-annprocess</artifactId>
<version>${version.jmh}</version>
</path>
</annotationProcessorPaths>
<compilerArgs>
<arg>--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED</arg>
Expand Down Expand Up @@ -1107,6 +1131,11 @@
<artifactId>build-helper-maven-plugin</artifactId>
<version>3.3.0</version>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>exec-maven-plugin</artifactId>
<version>3.1.0</version>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>license-maven-plugin</artifactId>
Expand Down Expand Up @@ -1181,6 +1210,7 @@
<!-- -->
GPL-2.0-with-classpath-exception
| CDDL/GPLv2+CE
| GNU General Public License (GPL), version 2, with the Classpath exception
| GNU General Public License, version 2 (GPL2), with the classpath exception
| GNU General Public License, version 2, with the Classpath Exception
| GPL2 w/ CPE
Expand Down Expand Up @@ -1257,12 +1287,18 @@
<artifactId>pitest-maven</artifactId>
<version>1.9.7</version>
<configuration>
<excludedClasses>
<excludedClass>*.AutoValue_*</excludedClass>
</excludedClasses>
<!-- Use multiple threads to speed things up. Extend
timeouts to prevent false positives as a result of
contention. -->
<threads>4</threads>
<timeoutFactor>4</timeoutFactor>
<timestampedReports>false</timestampedReports>
<excludedClasses>
<excludedClass>*.AutoValue_*</excludedClass>
</excludedClasses>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will port this to #255 and move the excludedClasses tag before the others.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to drop this now.

</configuration>
<dependencies>
<dependency>
Expand Down Expand Up @@ -1319,6 +1355,23 @@
</dependency>
</dependencies>
</dependencyManagement>
<build>
<pluginManagement>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<systemPropertyVariables>
<!-- Property used by `ErrorProneForkTest`
to verify fork identification. -->
<error-prone-fork-in-use>true</error-prone-fork-in-use>
</systemPropertyVariables>
</configuration>
</plugin>
</plugins>
</pluginManagement>
</build>
</profile>
<profile>
<!-- Error Prone checks that are not available from Maven Central;
Expand Down Expand Up @@ -1534,10 +1587,21 @@
failing the build on the first error
encountered. -->
-XepAllErrorsAsWarnings
<!-- Some generated classes violate Error
Prone bug patterns. We cannot in all cases
avoid that, so we simply tell Error Prone
not to warn about generated code. -->
-XepDisableWarningsInGeneratedCode
<!-- We want to enable almost all Error
Prone bug pattern checkers, so we enable
all and then selectively deactivate some. -->
-XepAllDisabledChecksAsWarnings
<!-- Some generated classes violate Error
Prone bug patterns. We cannot in all cases
avoid that, so we simply tell Error Prone
not to warn about generated code. -->
-XepDisableWarningsInGeneratedCode
-XepExcludedPaths:\Q${project.build.directory}${file.separator}\E.*
<!-- We don't target Android. -->
-Xep:AndroidJdkLibsChecker:OFF
<!-- XXX: Enable this once we open-source
Expand Down Expand Up @@ -1751,5 +1815,66 @@
</plugins>
</build>
</profile>
<profile>
<!-- Enables execution of a JMH benchmark. Given a benchmark class
`tech.picnic.MyBenchmark`, the following command (executed against
the (sub)module in which the benchmark resides) will compile and
execute said benchmark:

mvn process-test-classes -Dverification.skip \
-Djmh.run-benchmark=tech.picnic.MyBenchmark
-->
<id>run-jmh-benchmark</id>
<activation>
<property>
<name>jmh.run-benchmark</name>
</property>
</activation>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
<executions>
<execution>
<id>build-jmh-runtime-classpath</id>
<goals>
<goal>build-classpath</goal>
</goals>
<configuration>
<outputProperty>testClasspath</outputProperty>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>exec-maven-plugin</artifactId>
<executions>
<execution>
<id>run-jmh-benchmark</id>
<goals>
<goal>java</goal>
</goals>
<phase>process-test-classes</phase>
<configuration>
<classpathScope>test</classpathScope>
<mainClass>${jmh.run-benchmark}</mainClass>
<systemProperties>
<!-- The runtime classpath is defined
in this way so that any JVMs forked by
JMH will have the desired classpath. -->
<systemProperty>
<key>java.class.path</key>
<value>${project.build.testOutputDirectory}${path.separator}${project.build.outputDirectory}${path.separator}${testClasspath}</value>
</systemProperty>
</systemProperties>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
</profiles>
</project>
4 changes: 4 additions & 0 deletions refaster-compiler/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@
<groupId>${groupId.error-prone}</groupId>
<artifactId>error_prone_core</artifactId>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>refaster-support</artifactId>
</dependency>
<dependency>
<groupId>com.google.auto.service</groupId>
<artifactId>auto-service-annotations</artifactId>
Expand Down
Loading