ImportLayoutStyle: cache classpath conflict data per JavaSourceSet#7911
Draft
knutwannheden wants to merge 3 commits into
Draft
ImportLayoutStyle: cache classpath conflict data per JavaSourceSet#7911knutwannheden wants to merge 3 commits into
knutwannheden wants to merge 3 commits into
Conversation
ImportLayoutConflictDetection walked the entire classpath for every file in setJVMClassNames() and mapNamesInPackageToPackages(), since a fresh detector is created per file by OrderImports/AddImport. That is O(files x classpath) string work and, with a lazy classpath, forces materializing the whole classpath on every file. Add a lazily-built, per-instance package index to JavaSourceSet (typesInPackage(String)) and resolve only the imported packages through it, so the classpath is walked once per source set instead of once per file. New orderImports/addImport overloads thread the source set through; the existing overloads delegate with a null source set (unchanged full-walk behavior). The classpathDirty safe path is preserved. A fast path along these lines was added in #7528 and later reverted in #7845 as an unused SPI; this restores it without any SPI surface.
178a6d1 to
7528ef7
Compare
…classpath-fast-path
buildTypesByPackage() iterated the `classpath` field directly. A subclass that hydrates its classpath lazily — passing an empty list to super(...) and serving the real classpath only via an overridden getClasspath() — would therefore index an empty classpath, silently disabling ImportLayoutStyle's conflict detection (folding imports as if nothing conflicts). Read through getClasspath() so the override is honored; the lazy getter runs after construction, and behavior is unchanged for the base class, whose getter returns the field.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
ImportLayoutStyle.ImportLayoutConflictDetectionwalks the entire classpath for every file insetJVMClassNames()andmapNamesInPackageToPackages()— a fresh detector is created per file byOrderImports/AddImport, so the cost is O(files × classpath) ofgetPackageName()/getFullyQualifiedName()string work plusHashMapchurn. CPU profiling of a static-analysis run over a large multi-module project showedImportLayoutStyleat ~15% of the whole run. With a lazily-resolved classpath it is worse, because the full per-file walk forces materializing the entire lazy classpath on every file.JavaTypeFactoryand extendJavaSourceSetwith classpath fast paths #7528 (aJavaSourceSet.ClasspathIndexSPI) and later reverted in Type signature, marker, and SPI changes across rewrite-java and rewrite-maven #7845 as an unused SPI surface. This restores the optimization without any SPI — a concrete, lazily-cached method onJavaSourceSetwith a live consumer — so it can't be dropped again as "unused".What changed
JavaSourceSetgains a lazily-built, per-instance index of classpath types grouped by package, exposed as a single generic accessor (import-conflict policy stays inImportLayoutStyle):It is a Lombok
@Getter(lazy = true)field: thread-safe lazy init, and excluded from serialization,equals/hashCode,toString, and thewith*constructor threading — so a source set produced bywithClasspath(e.g.addTypesForGav/removeTypesForGav) rebuilds the index for its new classpath rather than reusing a stale one.ImportLayoutConflictDetectionresolves only the imported packages through that index when a source set is available, instead of walking the whole classpath:OrderImports/AddImportpass the CU'sJavaSourceSet. TheclasspathDirtysafe path and the cheapclasspath.isEmpty()guard are unchanged, and the cached path is byte-for-byte equivalent to the full walk (mirrors the per-type semantics of the original loop, including the static-member/FQN branch).Summary
JavaSourceSet.typesInPackage(String)backed by a lazily-builtMap<packageName, List<FullyQualified>>(single classpath walk per instance,@Getter(lazy = true)).ImportLayoutConflictDetectionuses the per-source-set index when present; falls back to the existing full walk when no source set is supplied.orderImports/addImportoverloads carrying theJavaSourceSet; old overloads retained and delegate withnull.OrderImportsandAddImportthread the source set through.Scope note
The cache is keyed to a
JavaSourceSetinstance, so it benefits every file that shares that instance (the in-memory parse/recipe path). On serialized pipelines where each file is re-hydrated into its own marker instance, the transient index is cold per file (same as the existing transienttypeFactory); a follow-up can add anExecutionContext-id-keyed backstop if profiling shows that path still hot. MemoizingJavaType.FullyQualified.getPackageName()is a complementary, independently valuable follow-up.Test plan
JavaSourceSetTest:typesInPackagereturns the types in a package; the classpath is walked exactly once across repeated calls (counting collection); the lazy index is excluded from the Jackson payload and the marker round-trips.ImportLayoutStyleTest: the neworderImports(..., sourceSet)overload is byte-for-byte equal to the full-walk path, both when ajava.langcollision must suppress folding and when folding must proceed.OrderImportsTest,AddImportTest(incl.doNotFoldPackageWithJavaLangClassNames, now exercising the cached path), full:rewrite-java,DirtyJavaSourceSetsProducerTest,ChangeTypeTest/ChangePackageTest/RemoveImportTest/RemoveUnusedImportsTest.