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

KSP1 breaks compilation avoidance #2347

Closed
ZacSweers opened this issue Feb 21, 2025 · 13 comments · Fixed by #2349
Closed

KSP1 breaks compilation avoidance #2347

ZacSweers opened this issue Feb 21, 2025 · 13 comments · Fixed by #2349
Milestone

Comments

@ZacSweers
Copy link
Contributor

We've been doing some benchmarking and I was surprised to find that KSP tasks are not compatible with Kotlin's compilation avoidance in incremental compilation because it appears to poke through useClasspathSnapshot to manually depend on packaged jars (ref), which will always be out of date even if the change is non-abi breaking.

The task was not up-to-date because of the following reasons:
Input property 'classpathSnapshotProperties.classpath' file services/navigation/navigation-key/build/intermediates/compile_library_classes_jar/release/bundleLibCompileToJarRelease/classes.jar has changed.

This significantly slows down builds that use KSP extensively because their tasks rerun unnecessarily

@ting-yuan ting-yuan added this to the 1.0.31 milestone Feb 21, 2025
@ZacSweers
Copy link
Contributor Author

To put an example number for this - one benchmark in our monorepo (~1k modules) that uses KSP extensively results in the critical path for a non-abi change for a module in the middle of the graph to go from ~12sec to ~58sec

@ZacSweers
Copy link
Contributor Author

Confirmed the same issue exists in KSP2

The task was not up-to-date because of the following reasons:
Input property 'kspConfig.libraries' file services/navigation/navigation-key/build/intermediates/compile_library_classes_jar/release/bundleLibCompileToJarRelease/classes.jar has changed.

@ting-yuan
Copy link
Collaborator

Some non-ABI changes can invalidate annotation processing results. For example, annotation processors may look into private members of a class in a library and generate outputs accordingly. This seems to be an inherited limitation of annotation processing.

Some other changes can be ignored though. A quick solution is to early return in KSP tasks when it doesn't see any meaningful changes.

The long term solution is to dependOn KGP's finer grained classpath snapshot artifacts instead of compile classpath. The performance would be similar to the above, but with KSP tasks reported as up to date in Gradle.

@ting-yuan
Copy link
Collaborator

BTW, there are some changes recently that aggregating processors always re-run in this case. Were you seeing performance changes recently or just discovered this 2-year old behavior?

@ZacSweers
Copy link
Contributor Author

I see what you mean. That does seem like it could at least be configurable though given that it's not common for processors to process external private symbols

@ZacSweers
Copy link
Contributor Author

In our case almost all of these projects are using isolating processors. You're right that it's not new behavior, but we also just weren't looking before I suppose.

@lzb6666
Copy link

lzb6666 commented Feb 24, 2025

BTW, there are some changes recently that aggregating processors always re-run in this case. Were you seeing performance changes recently or just discovered this 2-year old behavior?

I encountered the same issue when upgrading Kotlin from 1.7 to 2.0, which doubled the incremental compilation time for non-ABI changes in the lower-level modules. Is there any plan to solve this issue?

@Kimmon322

This comment has been minimized.

@ting-yuan ting-yuan linked a pull request Feb 25, 2025 that will close this issue
@ZacSweers
Copy link
Contributor Author

Excited to see a fix already! Looks like there's no toggle, so will this now work like avoidance in other kotlinc tasks? And if so - does that mean that processing of private symbols from other modules is no longer going to be IC-compatible? (Assuming yes and I think that's ok, just confirming!)

@ZacSweers
Copy link
Contributor Author

Just tested out the snapshot and worked like a charm 🚀

@ting-yuan
Copy link
Collaborator

ting-yuan commented Feb 26, 2025

Changes to signatures of private fields still trigger re-processing. Please file bugs or reopen if you found cases where it is not. The main difference, compared to previous implementation, is that it is no longer sensitive to changes of function body / text. It's basically the difference between @Classpath v.s. signatures of classes and all members.

@ZacSweers
Copy link
Contributor Author

got it. In that case I still think it may be worth considering an option to ignore private signature changes since they are not part of public ABIs and not common in processors? Otherwise it discourages more mindful API visibility. I'd also expect this to align with Kotlin's handling of internal symbols, but suspect gradle's @Classpath is purely the java API? I'm curious how that would work with KMP projects

@ting-yuan
Copy link
Collaborator

The implementation could be non-trivial so I'd like to revisit this after KSP2. Another related performance option is skip-processing-if-no-relavent-changes. Currently, aggregating processors always got re-triggered. Although it should be safe to skip, we do get bugs reports about missing side effects that is not in generated outputs, such as printed logs.

For KMP, compilation avoidance is not supported at the moment, so any changes in libraries will trigger re-processing. Proper support of that is an even bigger effort compared to the above and will only be done after KSP2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants