Skip to content

Add FindVirtualThreadOpportunities #2684

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

Closed
Pankraz76 wants to merge 1 commit into from
Closed

Add FindVirtualThreadOpportunities #2684

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions lib/src/main/java/com/diffplug/spotless/ProcessRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@
* calls to an external process.
*/
public class ProcessRunner implements AutoCloseable {
private final ExecutorService threadStdOut = Executors.newSingleThreadExecutor();
private final ExecutorService threadStdErr = Executors.newSingleThreadExecutor();
private final ExecutorService threadStdOut = /*~~>*/Executors.newSingleThreadExecutor();
private final ExecutorService threadStdErr = /*~~>*/Executors.newSingleThreadExecutor();
private final ByteArrayOutputStream bufStdOut;
private final ByteArrayOutputStream bufStdErr;

Expand Down
3 changes: 3 additions & 0 deletions rewrite.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ recipeList:
- org.openrewrite.java.format.NormalizeLineBreaks
- org.openrewrite.java.format.RemoveTrailingWhitespace
- org.openrewrite.java.migrate.UpgradeToJava17
- org.openrewrite.java.migrate.lang.FindVirtualThreadOpportunities
- org.openrewrite.java.migrate.lang.StringRulesRecipes
- org.openrewrite.java.migrate.util.JavaLangAPIs
- org.openrewrite.java.migrate.util.JavaUtilAPIs
- org.openrewrite.java.migrate.util.MigrateInflaterDeflaterToClose
- org.openrewrite.java.migrate.util.ReplaceStreamCollectWithToList
Expand Down
2 changes: 1 addition & 1 deletion testlib/src/test/java/com/diffplug/spotless/GitPrePushHookInstallerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ private String mavenHookContent(String resourcePath, ExecutorType executorType)

private void parallelRun(ThrowableRun runnable) {
IntStream.range(0, 5)
.mapToObj(i -> new Thread(() -> {
.mapToObj(i -> /*~~>*/new Thread(() -> {
Copy link
Copy Markdown
Author

@Pankraz76 Pankraz76 Oct 15, 2025

Choose a reason for hiding this comment

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

does this really make any difference?

Copy link
Copy Markdown
Member

@jbduncan jbduncan Oct 15, 2025

Choose a reason for hiding this comment

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

No, I don't think so.

I wonder if all that FindVirtualThreadOpportunities does is literally point out potential opportunities to adopt virtual threads, not actually migrate them.

Furthermore, I wonder if we still need to support versions of Java that don't have virtual threads.

Therefore, I'm inclined to remove this rule.

Copy link
Copy Markdown
Member

@nedtwigg nedtwigg Oct 15, 2025

Choose a reason for hiding this comment

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

Yup, we just upgraded our baseline to 17, it will be a while before our baseline is 21+

try {
runnable.run();
} catch (Exception e) {
Expand Down
Loading