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.

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

Support incremental processing #256

Open
vlsi opened this issue Jan 4, 2024 · 9 comments
Open

Support incremental processing #256

vlsi opened this issue Jan 4, 2024 · 9 comments
Labels
enhancement New feature or request

Comments

@vlsi
Copy link

vlsi commented Jan 4, 2024

What problem are you trying to solve?

Currently, rewriteRun and rewriteDryRun can take noticeable time (e.g. 30 seconds for not-so-big pgjdbc/pgjdbc) even when running back-to-back.
It is inconvenient when using OpenRewrite as the main code formatter.

Describe the solution you'd like

I want to make OpenRewrite process files incrementally, so it uses changed files as entry points. Then OpenRewrite could skip processing the files that have already been processed.

Gradle provides @Incremental machinery (see https://docs.gradle.org/current/userguide/custom_tasks.html#sec:implementing_an_incremental_task) for task inputs, so it should be feasible to make the task incremental and build cacheable.

See sample implementation at https://github.com/autostyle/autostyle/blob/0ebc8855d62a20b347fd1d1655977ebf25e4bf9b/plugin-gradle/src/main/kotlin/com/github/autostyle/gradle/AutostyleTask.kt#L84-L105

@timtebeek
Copy link
Contributor

Interesting suggestion; would you happen to know which version of Gradle support @Incremental?
I'm only seeing the https://docs.gradle.org/current/userguide/incremental_build.html page for 8.x so far.

We support Gradle versions all the way back to 4.10, which restricts us quite a bit unfortunately.
See also this earlier and ongoing attempt to add support for configuration cache for instance:

@vlsi
Copy link
Author

vlsi commented Jan 4, 2024

InputChanges is @since 5.4.

If you want to support 4.10, you could ship two classes for the tasks: one that uses 4.10 API only, and another one that is Gradle 5+ or 6+ or whatever.

Then you could register the relevant class based on the runtime Gradle version. It would enable incremental processing for the ones who have Gradle 5.4+ (released Apr 16, 2019)

@sergeykad
Copy link

sergeykad commented Jan 7, 2024

Gradle 5.4 was released almost 5 years ago. Most Gradle plugins I am familiar with tend to focus on supporting the latest two major versions or even only the latest.
If somebody needs to use OpenRewrite with an older Gradle version he can simply use an older OpenRewrite plugin. However, if the project's Gradle version is more than 5 years old, it's worth considering whether the project might be inactive or less actively maintained. In these cases, upgrading to a newer Gradle version and using the latest OpenRewrite plugin might be the best option.

@knutwannheden
Copy link
Contributor

For non-scanning recipes which only need to visit the modified sources, this should indeed be doable.

@vlsi
Copy link
Author

vlsi commented Jan 11, 2024

It takes openrewrite ~15sec to scan the classpath when searching for the recipes, so I excluded scanClassLoader(...) call. That change alone significantly improves rewriteRun performance.

Unfortunately, the current RewriteRunTask does not support input and output tracking, so it always processes the files even in the case the project was not modified. So I went with wrapping OpenRewrite with a Gradle task that tracks inputs and outputs.


Then, to implement incremental processing, OpenRewrite should track "which files used which classes".
In other words, if SomeInterface.java changes, then DefaultImplementation.java would need to be re-parsed and re-processed even if DefaultImplementation.java does not change itself (assuming DefaultImplementation implements SomeInterface).


Here's what I get with per-project openrewrite processing in Apache JMeter:

rewriteRun: ~30sec (10 concurrent threads), https://ge.apache.org/s/6lzvtx46jpttk/timeline?details=iide2thhvm25a

:src:core:rewriteRun
parse time: 13008
run time: 4245

"add a space to a private Java method in :src:core": ~20sec (single-threaded mostly), https://ge.apache.org/s/iqhrnb3eby76c/timeline?details=iide2thhvm25a&page=2

"add a space to a private Kotlin method in :src:core": ~20sec (single-threaded mostly), https://ge.apache.org/s/qc3l4w3h6iy5s/timeline?details=iide2thhvm25a&page=2

@timtebeek timtebeek moved this to Backlog in OpenRewrite Jan 11, 2024
@sergeykad
Copy link

@vlsi Gradle Java plugin supports incremental compilation, so I assume it should be doable here as well.

@koppor
Copy link
Contributor

koppor commented Jan 14, 2024

Another approach is proposed at #77.

@vlsi
Copy link
Author

vlsi commented Jan 14, 2024

#77 is an approach to estimate "which files were changed", however, it does not solve the case when its dependency changes (see #256 (comment) regarding DefaultImplementation example)

@blipper
Copy link
Contributor

blipper commented Mar 9, 2024

We should distinguish between incremental builds (running OpenRewrite on an delta changed set of files) and up-to-date checking.

For up-to-date checking you just need to declare the inputs (which can be done via api). Outputs should already be declared actually. Though the value is a bit questionable given that it would be all or none.

https://docs.gradle.org/current/userguide/incremental_build.html#sec:task_input_output_runtime_api

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Backlog
Development

No branches or pull requests

6 participants