From a56b5a952e578eeb29c19cea2376cf59b936823c Mon Sep 17 00:00:00 2001 From: Tony Robalik Date: Mon, 10 Feb 2025 11:06:22 -0800 Subject: [PATCH] fix: generate supergraph just once per variant (source set). --- .../graph/supers/SuperClassGraphBuilder.kt | 5 +-- .../graph/supers/SuperClassGraphHolder.kt | 36 ------------------- .../model/internal/ProjectVariant.kt | 23 +++++++++--- .../autonomousapps/tasks/ComputeUsagesTask.kt | 9 ++--- .../autonomousapps/visitor/GraphViewReader.kt | 15 ++++++-- .../visitor/GraphViewVisitor.kt | 5 +++ 6 files changed, 40 insertions(+), 53 deletions(-) delete mode 100644 src/main/kotlin/com/autonomousapps/internal/graph/supers/SuperClassGraphHolder.kt diff --git a/src/main/kotlin/com/autonomousapps/internal/graph/supers/SuperClassGraphBuilder.kt b/src/main/kotlin/com/autonomousapps/internal/graph/supers/SuperClassGraphBuilder.kt index 02bfb2065..ea1273366 100644 --- a/src/main/kotlin/com/autonomousapps/internal/graph/supers/SuperClassGraphBuilder.kt +++ b/src/main/kotlin/com/autonomousapps/internal/graph/supers/SuperClassGraphBuilder.kt @@ -3,6 +3,7 @@ package com.autonomousapps.internal.graph.supers import com.autonomousapps.internal.graph.newGraphBuilder import com.autonomousapps.model.Coordinates import com.autonomousapps.model.internal.BinaryClassCapability +import com.autonomousapps.model.internal.Dependency import com.autonomousapps.visitor.GraphViewVisitor import com.google.common.graph.Graph @@ -42,10 +43,10 @@ internal class SuperClassGraphBuilder { companion object { /** Builds a graph from child classes up through super classes and interfaces, up to `java.lang.Object`. */ - fun of(context: GraphViewVisitor.Context): Graph { + fun of(dependencies: Set): Graph { val builder = SuperClassGraphBuilder() - context.dependencies.forEach { dep -> + dependencies.forEach { dep -> dep.findCapability()?.let { capability -> capability.binaryClasses.map { bin -> val from = SuperNode(bin.className).apply { diff --git a/src/main/kotlin/com/autonomousapps/internal/graph/supers/SuperClassGraphHolder.kt b/src/main/kotlin/com/autonomousapps/internal/graph/supers/SuperClassGraphHolder.kt deleted file mode 100644 index 371a09725..000000000 --- a/src/main/kotlin/com/autonomousapps/internal/graph/supers/SuperClassGraphHolder.kt +++ /dev/null @@ -1,36 +0,0 @@ -package com.autonomousapps.internal.graph.supers - -import com.autonomousapps.internal.unsafeLazy -import com.autonomousapps.internal.utils.flatMapToOrderedSet -import com.autonomousapps.internal.utils.mapNotNullToOrderedSet -import com.autonomousapps.internal.utils.mapToOrderedSet -import com.autonomousapps.visitor.GraphViewVisitor -import com.google.common.graph.Graph - -/** - * This calculation is very expensive, so we cache it. - * - * @see Issue 1358 - */ -@Suppress("UnstableApiUsage") // Guava graphs -internal class SuperClassGraphHolder(private val context: GraphViewVisitor.Context) { - - /** Graph from child classes up through super classes and interfaces, up to `java/lang/Object`. */ - val superGraph: Graph by unsafeLazy { SuperClassGraphBuilder.of(context) } - - /** The set of super classes and interfaces not available from [context] (therefore "external" to "this" module). */ - val externals: Set by unsafeLazy { computeExternalSupers() } - - private fun computeExternalSupers(): Set { - val supers = context.project.codeSource.mapNotNullToOrderedSet { src -> src.superClass } - val interfaces = context.project.codeSource.flatMapToOrderedSet { src -> src.interfaces } - // These are all the super classes and interfaces in "this" module - val localClasses = context.project.codeSource.mapToOrderedSet { src -> src.className } - // These super classes and interfaces are not available from "this" module, so must come from dependencies. - val externalSupers = supers - localClasses - val externalInterfaces = interfaces - localClasses - val externals = externalSupers + externalInterfaces - - return externals - } -} diff --git a/src/main/kotlin/com/autonomousapps/model/internal/ProjectVariant.kt b/src/main/kotlin/com/autonomousapps/model/internal/ProjectVariant.kt index e3666b326..6fcbc085c 100644 --- a/src/main/kotlin/com/autonomousapps/model/internal/ProjectVariant.kt +++ b/src/main/kotlin/com/autonomousapps/model/internal/ProjectVariant.kt @@ -3,14 +3,11 @@ package com.autonomousapps.model.internal import com.autonomousapps.internal.unsafeLazy -import com.autonomousapps.internal.utils.flatMapToOrderedSet -import com.autonomousapps.internal.utils.flatMapToSet -import com.autonomousapps.internal.utils.fromJson -import com.autonomousapps.model.internal.CodeSource.Kind +import com.autonomousapps.internal.utils.* import com.autonomousapps.model.Coordinates import com.autonomousapps.model.ProjectCoordinates import com.autonomousapps.model.declaration.Variant -import com.autonomousapps.model.internal.intermediates.consumer.MemberAccess +import com.autonomousapps.model.internal.CodeSource.Kind import com.squareup.moshi.JsonClass import org.gradle.api.file.Directory @@ -86,6 +83,22 @@ internal data class ProjectVariant( usedNonAnnotationClasses - exposedClasses } + /** + * The set of super classes and interfaces not available from [codeSource] (therefore "external" to "this" module). + */ + val externalSupers: Set by unsafeLazy { + val supers = codeSource.mapNotNullToOrderedSet { src -> src.superClass } + val interfaces = codeSource.flatMapToOrderedSet { src -> src.interfaces } + // These are all the super classes and interfaces in "this" module + val localClasses = codeSource.mapToOrderedSet { src -> src.className } + // These super classes and interfaces are not available from "this" module, so must come from dependencies. + val externalSupers = supers - localClasses + val externalInterfaces = interfaces - localClasses + val externals = externalSupers + externalInterfaces + + externals + } + val androidResSource: List by unsafeLazy { sources.filterIsInstance() } diff --git a/src/main/kotlin/com/autonomousapps/tasks/ComputeUsagesTask.kt b/src/main/kotlin/com/autonomousapps/tasks/ComputeUsagesTask.kt index 1faf948bb..262860b5f 100644 --- a/src/main/kotlin/com/autonomousapps/tasks/ComputeUsagesTask.kt +++ b/src/main/kotlin/com/autonomousapps/tasks/ComputeUsagesTask.kt @@ -5,7 +5,6 @@ package com.autonomousapps.tasks import com.autonomousapps.graph.Graphs.parents import com.autonomousapps.graph.Graphs.reachableNodes import com.autonomousapps.graph.Graphs.root -import com.autonomousapps.internal.graph.supers.SuperClassGraphHolder import com.autonomousapps.internal.graph.supers.SuperNode import com.autonomousapps.internal.utils.* import com.autonomousapps.model.Coordinates @@ -122,8 +121,6 @@ private class GraphVisitor( private val kapt: Boolean, ) : GraphViewVisitor { - private lateinit var superClassGraphHolder: SuperClassGraphHolder - val report: DependencyTraceReport get() = reportBuilder.build() private val reportBuilder = DependencyTraceReport.Builder( @@ -134,7 +131,6 @@ private class GraphVisitor( override fun visit(dependency: Dependency, context: GraphViewVisitor.Context) { val dependencyCoordinates = dependency.coordinates - superClassGraphHolder = SuperClassGraphHolder(context) var isAnnotationProcessor = false var isAnnotationProcessorCandidate = false @@ -371,16 +367,15 @@ private class GraphVisitor( } } - // TODO(tsr): consider providing an opt-out or even an opt-in for this (very expensive) analysis. private fun isForMissingSuperclass( coordinates: Coordinates, capability: BinaryClassCapability, context: GraphViewVisitor.Context, ): Boolean { - val superGraph = superClassGraphHolder.superGraph + val superGraph = context.superGraph // collect all the dependencies associated with external supers - val requiredExternalClasses = superClassGraphHolder.externals.asSequence() + val requiredExternalClasses = context.project.externalSupers.asSequence() .flatMap { external -> superGraph.reachableNodes(false) { it.className == external } } .mapNotNull { node -> val deps = node.deps.filterToOrderedSet { dep -> diff --git a/src/main/kotlin/com/autonomousapps/visitor/GraphViewReader.kt b/src/main/kotlin/com/autonomousapps/visitor/GraphViewReader.kt index 3c1290676..d7aedf10e 100644 --- a/src/main/kotlin/com/autonomousapps/visitor/GraphViewReader.kt +++ b/src/main/kotlin/com/autonomousapps/visitor/GraphViewReader.kt @@ -2,11 +2,15 @@ // SPDX-License-Identifier: Apache-2.0 package com.autonomousapps.visitor +import com.autonomousapps.internal.graph.supers.SuperClassGraphBuilder +import com.autonomousapps.internal.graph.supers.SuperNode +import com.autonomousapps.internal.unsafeLazy +import com.autonomousapps.model.DuplicateClass +import com.autonomousapps.model.declaration.internal.Declaration import com.autonomousapps.model.internal.Dependency import com.autonomousapps.model.internal.DependencyGraphView -import com.autonomousapps.model.DuplicateClass import com.autonomousapps.model.internal.ProjectVariant -import com.autonomousapps.model.declaration.internal.Declaration +import com.google.common.graph.Graph internal class GraphViewReader( private val project: ProjectVariant, @@ -30,4 +34,9 @@ internal class DefaultContext( override val graph: DependencyGraphView, override val declarations: Set, override val duplicateClasses: Set, -) : GraphViewVisitor.Context +) : GraphViewVisitor.Context { + + override val superGraph: Graph by unsafeLazy { + SuperClassGraphBuilder.of(dependencies) + } +} diff --git a/src/main/kotlin/com/autonomousapps/visitor/GraphViewVisitor.kt b/src/main/kotlin/com/autonomousapps/visitor/GraphViewVisitor.kt index 491880d56..a08ceeeb6 100644 --- a/src/main/kotlin/com/autonomousapps/visitor/GraphViewVisitor.kt +++ b/src/main/kotlin/com/autonomousapps/visitor/GraphViewVisitor.kt @@ -2,11 +2,13 @@ // SPDX-License-Identifier: Apache-2.0 package com.autonomousapps.visitor +import com.autonomousapps.internal.graph.supers.SuperNode import com.autonomousapps.model.internal.Dependency import com.autonomousapps.model.internal.DependencyGraphView import com.autonomousapps.model.DuplicateClass import com.autonomousapps.model.internal.ProjectVariant import com.autonomousapps.model.declaration.internal.Declaration +import com.google.common.graph.Graph internal interface GraphViewVisitor { fun visit(dependency: Dependency, context: Context) @@ -17,5 +19,8 @@ internal interface GraphViewVisitor { val graph: DependencyGraphView val declarations: Set val duplicateClasses: Set + + /** Graph from child classes up through super classes and interfaces, up to `java/lang/Object`. */ + val superGraph: Graph } }