8000 Refactor IncrementalJavaFileManager and KotlinSymbolProcessing by bcorso · Pull Request #3257 · google/ksp · GitHub
[go: up one dir, main page]
Skip to content

Refactor IncrementalJavaFileManager and KotlinSymbolProcessing - #3257

Open
bcorso wants to merge 3 commits into
google:mainfrom
bcorso:bcorso/simplify-incremental-java-file-manager
Open

Refactor IncrementalJavaFileManager and KotlinSymbolProcessing #3257
bcorso wants to merge 3 commits into
google:mainfrom
bcorso:bcorso/simplify-incremental-java-file-manager

Conversation

@bcorso
@bcorso bcorso commented Oct 9, 2026
Copy link
Copy Markdown
Collaborator

This PR simplifies classpath/source root indexing and improves readability across IncrementalJavaFileManager and KotlinSymbolProcessing:

  1. Simplify root indexing and remove dead JDK root setup (IncrementalJavaFileManager)
    Previously, IncrementalJavaFileManager re-resolved binary roots and created a separate package part provider during initialization even though KotlinSymbolProcessing already computed both during session setup. It also set up a CliJavaModuleFinder with jdkHome = null whose JDK root lookup was a no-op because SDK roots are already included in libraryRoots. Passing the precomputed libraryRoots and createPackagePartProvider directly allows binary roots to be indexed upfront during construction and lets initial and incremental Java source files share a single add code path (skipping empty sub-index additions when no new source files are present).
  2. Deduplicate initial and incremental KSFile setup (KotlinSymbolProcessing)
    Extracts a shared prepareKSFiles helper for prepareAllKSFiles and prepareNewKSFiles to avoid duplicating Java PSI file creation and incremental Java source indexing between initial setup and subsequent processing rounds.
  3. Decompose KotlinSymbolProcessing.execute into focused helper methods (KotlinSymbolProcessing)
    Splits error tracking, code generator setup, incremental context initialization, processor instantiation, processing round execution, finalization, and cache clearing out of execute so the top-level processing flow is concise and easier to follow.

@jaschdoc jaschdoc added the performance KSP performance, e.g., reducing build times for users label Oct 15, 2026
jaschdoc
jaschdoc reviewed Oct 16, 2026
View reviewed changes
@jaschdoc jaschdoc left a comment •
edited
Loading
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the PR! Left some comments. I will hold off merging this until #3253 and #3255 are merged.

Comment thread kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/KotlinSymbolProcessing.kt Resolved
Comment thread kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/KotlinSymbolProcessing.kt Resolved
Comment thread kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/KotlinSymbolProcessing.kt Resolved
Comment thread kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/KotlinSymbolProcessing.kt Resolved
@jaschdoc jaschdoc mentioned this pull request Oct 16, 2026
Merged
@bcorso
bcorso force-pushed the bcorso/simplify-incremental-java-file-manager branch 4 times, most recently from 5bc47ba to db47fac Compare October 19, 2026 16:31
@jaschdoc
jaschdoc commented Oct 20, 2026
Copy link
Copy Markdown
Collaborator

Please rebase on main :)

@bcorso
bcorso force-pushed the bcorso/simplify-incremental-java-file-manager branch 6 times, most recently from 280ad43 to dea4de8 Compare October 26, 2026 05:33
@bcorso
bcorso commented Oct 26, 2026
Copy link
Copy Markdown
Collaborator Author

Please rebase on main :)

Done!

@bcorso
bcorso requested a review from jaschdoc October 30, 2026 04:02
jaschdoc
jaschdoc previously approved these changes Nov 2, 2026
View reviewed changes
@jaschdoc jaschdoc left a comment
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you!

@bcorso
bcorso force-pushed the bcorso/simplify-incremental-java-file-manager branch from dea4de8 to ba87740 Compare November 2, 2026 19:43
bcorso added 3 commits November 2, 2026 20:13
@bcorso
Simplify root indexing and remove dead JDK root setup in IncrementalJ…
3254b0b 
…avaFileManager.

CliJavaModuleFinder in IncrementalJavaFileManager.initialize was constructed with jdkHome = null and jdkRelease = null, so jrtFileSystemRoot and modulesRoot are always null and getDefaultJdkModuleRoots always returned an empty list. Additionally, KotlinSymbolProcessing.createAASession already registers kspConfig.jdkHome as a KaLibraryModule via buildKspSdkModule, so StandaloneProjectFactory.getAllBinaryRoots already includes all JDK module roots inside libraryRoots.

Pass the already-computed libraryRoots and createPackagePartProvider from KotlinSymbolProcessing directly to IncrementalJavaFileManager instead of resolving binary roots and creating a package part provider a second time, index libraryRoots directly in init, remove the separate initialize method so both initial and incremental Java source files use add via a shared addRoots helper, and skip adding empty sub-indices when indexing single Java source files.
@bcorso
Extract shared prepareKSFiles helper for prepareAllKSFiles and prepar…
ee8d5bb 
…eNewKSFiles.
@bcorso
Refactor KotlinSymbolProcessing.execute into cohesive helper methods.
e1c3868
@bcorso
bcorso force-pushed the bcorso/simplify-incremental-java-file-manager branch from ba87740 to e1c3868 Compare November 2, 2026 20:38
@bcorso
bcorso requested a review from jaschdoc November 2, 2026 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@troelsbjerre troelsbjerre