Skip to content

Commit

Permalink
WIP: Reenable modified filepaths entry (#193)
Browse files Browse the repository at this point in the history 
* WIP: Reenable modified filepaths entry

This should greatly speed up hash calculation time

more updates

* more updates

* build
  • Loading branch information
@tinder-maxwellelliott
tinder-maxwellelliott authored Sep 16, 2024
1 parent eedaeba commit 79f4073
Show file tree
Hide file tree
Showing 9 changed files with 121 additions and 32 deletions.
43 changes: 43 additions & 0 deletions cli/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ genrule(
stamp = 1,
)

config_setting(
name = "macos",
constraint_values = [
"@@platforms//os:macos",
],
)

java_binary(
name = "bazel-diff",
jvm_flags = select({
Expand Down Expand Up @@ -48,6 +55,10 @@ kt_jvm_library(

kt_jvm_test(
name = "BuildGraphHasherTest",
jvm_flags = select({
":macos": ["-Djava.security.manager=allow"],
"//conditions:default": [],
}),
test_class = "com.bazel_diff.hash.BuildGraphHasherTest",
runtime_deps = [":cli-test-lib"],
)
Expand All @@ -57,42 +68,70 @@ kt_jvm_test(
data = [
":src/test/kotlin/com/bazel_diff/hash/fixture/foo.ts",
],
jvm_flags = select({
":macos": ["-Djava.security.manager=allow"],
"//conditions:default": [],
}),
test_class = "com.bazel_diff.hash.SourceFileHasherTest",
runtime_deps = [":cli-test-lib"],
)

kt_jvm_test(
name = "CalculateImpactedTargetsInteractorTest",
jvm_flags = select({
":macos": ["-Djava.security.manager=allow"],
"//conditions:default": [],
}),
test_class = "com.bazel_diff.interactor.CalculateImpactedTargetsInteractorTest",
runtime_deps = [":cli-test-lib"],
)

kt_jvm_test(
name = "NormalisingPathConverterTest",
jvm_flags = select({
":macos": ["-Djava.security.manager=allow"],
"//conditions:default": [],
}),
test_class = "com.bazel_diff.cli.converter.NormalisingPathConverterTest",
runtime_deps = [":cli-test-lib"],
)

kt_jvm_test(
name = "OptionsConverterTest",
jvm_flags = select({
":macos": ["-Djava.security.manager=allow"],
"//conditions:default": [],
}),
test_class = "com.bazel_diff.cli.converter.OptionsConverterTest",
runtime_deps = [":cli-test-lib"],
)

kt_jvm_test(
name = "DeserialiseHashesInteractorTest",
jvm_flags = select({
":macos": ["-Djava.security.manager=allow"],
"//conditions:default": [],
}),
test_class = "com.bazel_diff.interactor.DeserialiseHashesInteractorTest",
runtime_deps = [":cli-test-lib"],
)

kt_jvm_test(
name = "BazelRuleTest",
jvm_flags = select({
":macos": ["-Djava.security.manager=allow"],
"//conditions:default": [],
}),
test_class = "com.bazel_diff.bazel.BazelRuleTest",
runtime_deps = [":cli-test-lib"],
)

kt_jvm_test(
name = "E2ETest",
jvm_flags = select({
":macos": ["-Djava.security.manager=allow"],
"//conditions:default": [],
}),
test_class = "com.bazel_diff.e2e.E2ETest",
runtime_deps = [":cli-test-lib"],
)
Expand All @@ -103,6 +142,10 @@ kt_jvm_test(
":src/test/kotlin/com/bazel_diff/io/fixture/correct.json",
":src/test/kotlin/com/bazel_diff/io/fixture/wrong.json",
],
jvm_flags = select({
":macos": ["-Djava.security.manager=allow"],
"//conditions:default": [],
}),
test_class = "com.bazel_diff.io.ContentHashProviderTest",
runtime_deps = [
":cli-test-lib",
Expand Down
6 changes: 3 additions & 3 deletions cli/src/main/kotlin/com/bazel_diff/bazel/BazelClient.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ package com.bazel_diff.bazel

import com.bazel_diff.log.Logger
import com.google.devtools.build.lib.query2.proto.proto2api.Build
import org.koin.core.component.KoinComponent
import org.koin.core.component.inject
import java.nio.file.Path
import java.util.*
import org.koin.core.component.inject
import org.koin.core.component.KoinComponent

class BazelClient(private val useCquery: Boolean, private val fineGrainedHashExternalRepos: Set<String>) : KoinComponent {
private val logger: Logger by inject()
Expand Down Expand Up @@ -54,4 +55,3 @@ class BazelClient(private val useCquery: Boolean, private val fineGrainedHashExt
}
}
}

11 changes: 9 additions & 2 deletions cli/src/main/kotlin/com/bazel_diff/bazel/BazelQueryService.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ class BazelQueryService(
) : KoinComponent {
private val logger: Logger by inject()

suspend fun query(query: String, useCquery: Boolean = false): List<Build.Target> {
suspend fun query(
query: String,
useCquery: Boolean = false)
: List<Build.Target> {
// Unfortunately, there is still no direct way to tell if a target is compatible or not with the proto output
// by itself. So we do an extra cquery with the trick at
// https://bazel.build/extending/platforms#cquery-incompatible-target-detection to first find all compatible
Expand Down Expand Up @@ -58,7 +61,11 @@ class BazelQueryService(
}

@OptIn(ExperimentalCoroutinesApi::class)
private suspend fun runQuery(query: String, useCquery: Boolean, outputCompatibleTargets: Boolean = false): File {
private suspend fun runQuery(
query: String,
useCquery: Boolean,
outputCompatibleTargets: Boolean = false
): File {
val queryFile = Files.createTempFile(null, ".txt").toFile()
queryFile.deleteOnExit()
val outputFile = Files.createTempFile(null, ".bin").toFile()
Expand Down
8 changes: 7 additions & 1 deletion cli/src/main/kotlin/com/bazel_diff/cli/GenerateHashesCommand.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,12 @@ class GenerateHashesCommand : Callable<Int> {
)
var ignoredRuleHashingAttributes: Set<String> = emptySet()

@CommandLine.Option(
names = ["-m", "--modified-filepaths"],
description = ["Experimental: A text file containing a newline separated list of filepaths (relative to the workspace) these filepaths should represent the modified files between the specified revisions and will be used to scope what files are hashed during hash generation."]
)
var modifiedFilepaths: File? = null

@CommandLine.Spec
lateinit var spec: CommandLine.Model.CommandSpec

Expand All @@ -160,7 +166,7 @@ class GenerateHashesCommand : Callable<Int> {
)
}

return when (GenerateHashesInteractor().execute(seedFilepaths, outputPath, ignoredRuleHashingAttributes, targetType, includeTargetType)) {
return when (GenerateHashesInteractor().execute(seedFilepaths, outputPath, ignoredRuleHashingAttributes, targetType, includeTargetType, modifiedFilepaths)) {
true -> CommandLine.ExitCode.OK
false -> CommandLine.ExitCode.SOFTWARE
}.also { stopKoin() }
Expand Down
36 changes: 22 additions & 14 deletions cli/src/main/kotlin/com/bazel_diff/hash/BuildGraphHasher.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,19 @@ import com.bazel_diff.extensions.toHexString
import com.bazel_diff.log.Logger
import com.google.common.collect.Sets
import com.google.devtools.build.lib.query2.proto.proto2api.Build
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.async
import kotlinx.coroutines.runBlocking
import org.koin.core.component.KoinComponent
import org.koin.core.component.inject
import java.io.File
import java.nio.file.Path
import java.util.Calendar
import java.util.concurrent.atomic.AtomicReference
import java.util.concurrent.ConcurrentHashMap
import java.util.concurrent.ConcurrentMap
import java.util.concurrent.atomic.AtomicReference
import java.util.stream.Collectors
import kotlin.io.path.readBytes
import java.util.Calendar
import kotlinx.coroutines.async
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.runBlocking
import org.koin.core.component.inject
import org.koin.core.component.KoinComponent

class BuildGraphHasher(private val bazelClient: BazelClient) : KoinComponent {
private val targetHasher: TargetHasher by inject()
Expand All @@ -28,7 +29,8 @@ class BuildGraphHasher(private val bazelClient: BazelClient) : KoinComponent {

fun hashAllBazelTargetsAndSourcefiles(
seedFilepaths: Set<Path> = emptySet(),
ignoredAttrs: Set<String> = emptySet()
ignoredAttrs: Set<String> = emptySet(),
modifiedFilepaths: Set<Path> = emptySet()
): Map<String, TargetHash> {
val (sourceDigests, allTargets) = runBlocking {
val targetsTask = async(Dispatchers.IO) {
Expand All @@ -39,7 +41,7 @@ class BuildGraphHasher(private val bazelClient: BazelClient) : KoinComponent {

val sourceDigestsFuture = async(Dispatchers.IO) {
val sourceHashDurationEpoch = Calendar.getInstance().getTimeInMillis()
val sourceFileTargets = hashSourcefiles(sourceTargets)
val sourceFileTargets = hashSourcefiles(sourceTargets, modifiedFilepaths)
val sourceHashDuration = Calendar.getInstance().getTimeInMillis() - sourceHashDurationEpoch
logger.i { "Source file hashes calculated in $sourceHashDuration" }
sourceFileTargets
Expand All @@ -52,11 +54,15 @@ class BuildGraphHasher(private val bazelClient: BazelClient) : KoinComponent {
seedForFilepaths,
sourceDigests,
allTargets,
ignoredAttrs
ignoredAttrs,
modifiedFilepaths
)
}

private fun hashSourcefiles(targets: List<BazelTarget.SourceFile>): ConcurrentMap<String, ByteArray> {
private fun hashSourcefiles(
targets: List<BazelTarget.SourceFile>,
modifiedFilepaths: Set<Path>
): ConcurrentMap<String, ByteArray> {
val exception = AtomicReference<Exception?>(null)
val result: ConcurrentMap<String, ByteArray> = targets.parallelStream()
.map { sourceFile: BazelTarget.SourceFile ->
Expand All @@ -68,7 +74,7 @@ class BuildGraphHasher(private val bazelClient: BazelClient) : KoinComponent {
}
try {
val sourceFileTarget = BazelSourceFileTarget(sourceFile.name, seed)
Pair(sourceFileTarget.name, sourceFileHasher.digest(sourceFileTarget))
Pair(sourceFileTarget.name, sourceFileHasher.digest(sourceFileTarget, modifiedFilepaths))
} catch (e: Exception) {
exception.set(e)
null
Expand All @@ -90,7 +96,8 @@ class BuildGraphHasher(private val bazelClient: BazelClient) : KoinComponent {
seedHash: ByteArray,
sourceDigests: ConcurrentMap<String, ByteArray>,
allTargets: List<BazelTarget>,
ignoredAttrs: Set<String>
ignoredAttrs: Set<String>,
modifiedFilepaths: Set<Path>
): Map<String, TargetHash> {
val ruleHashes: ConcurrentMap<String, ByteArray> = ConcurrentHashMap()
val targetToRule: MutableMap<String, BazelRule> = HashMap()
Expand All @@ -104,7 +111,8 @@ class BuildGraphHasher(private val bazelClient: BazelClient) : KoinComponent {
sourceDigests,
ruleHashes,
seedHash,
ignoredAttrs
ignoredAttrs,
modifiedFilepaths
)
Pair(target.name, TargetHash(target.javaClass.name.substringAfterLast('$'), targetDigest.toHexString()))
}
Expand Down
9 changes: 6 additions & 3 deletions cli/src/main/kotlin/com/bazel_diff/hash/RuleHasher.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import com.google.common.annotations.VisibleForTesting
import org.koin.core.component.KoinComponent
import org.koin.core.component.inject
import java.util.concurrent.ConcurrentMap
import java.nio.file.Path

class RuleHasher(private val useCquery: Boolean, private val fineGrainedHashExternalRepos: Set<String>) : KoinComponent {
private val logger: Logger by inject()
Expand All @@ -31,7 +32,8 @@ class RuleHasher(private val useCquery: Boolean, private val fineGrainedHashExte
sourceDigests: ConcurrentMap<String, ByteArray>,
seedHash: ByteArray?,
depPath: LinkedHashSet<String>?,
ignoredAttrs: Set<String>
ignoredAttrs: Set<String>,
modifiedFilepaths: Set<Path>
): ByteArray {
val depPathClone = if (depPath != null) LinkedHashSet(depPath) else LinkedHashSet()
if (depPathClone.contains(rule.name)) {
Expand Down Expand Up @@ -61,13 +63,14 @@ class RuleHasher(private val useCquery: Boolean, private val fineGrainedHashExte
sourceDigests,
seedHash,
depPathClone,
ignoredAttrs
ignoredAttrs,
modifiedFilepaths
)
safePutBytes(ruleInputHash)
}

else -> {
val heuristicDigest = sourceFileHasher.softDigest(BazelSourceFileTarget(ruleInput, ByteArray(0)))
val heuristicDigest = sourceFileHasher.softDigest(BazelSourceFileTarget(ruleInput, ByteArray(0)), modifiedFilepaths)
when {
heuristicDigest != null -> {
logger.i { "Source file $ruleInput picked up as an input for rule ${rule.name}" }
Expand Down
15 changes: 11 additions & 4 deletions cli/src/main/kotlin/com/bazel_diff/hash/SourceFileHasher.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ class SourceFileHasher : KoinComponent {
this.externalRepoResolver = externalRepoResolver
}

fun digest(sourceFileTarget: BazelSourceFileTarget): ByteArray {
fun digest(
sourceFileTarget: BazelSourceFileTarget,
modifiedFilepaths: Set<Path> = emptySet()
): ByteArray {
return sha256 {
val name = sourceFileTarget.name
val filenamePath = if (name.startsWith("//")) {
Expand Down Expand Up @@ -69,7 +72,11 @@ class SourceFileHasher : KoinComponent {
val file = absoluteFilePath.toFile()
if (file.exists()) {
if (file.isFile) {
putFile(file)
if (modifiedFilepaths.isEmpty()) {
putFile(file)
} else if (modifiedFilepaths.stream().anyMatch { workingDirectory.resolve(it) == file }) {
putFile(file)
}
}
} else {
logger.w { "File $absoluteFilePath not found" }
Expand All @@ -80,7 +87,7 @@ class SourceFileHasher : KoinComponent {
}
}

fun softDigest(sourceFileTarget: BazelSourceFileTarget): ByteArray? {
fun softDigest(sourceFileTarget: BazelSourceFileTarget, modifiedFilepaths: Set<Path> = emptySet()): ByteArray? {
val name = sourceFileTarget.name
if (!name.startsWith("//")) return null

Expand All @@ -90,6 +97,6 @@ class SourceFileHasher : KoinComponent {
val file = absoluteFilePath.toFile()
if (!file.exists() || !file.isFile) return null

return digest(sourceFileTarget)
return digest(sourceFileTarget, modifiedFilepaths)
}
}
10 changes: 7 additions & 3 deletions cli/src/main/kotlin/com/bazel_diff/hash/TargetHasher.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import com.bazel_diff.bazel.BazelTarget
import org.koin.core.component.KoinComponent
import org.koin.core.component.inject
import java.util.concurrent.ConcurrentMap
import java.nio.file.Path

class TargetHasher : KoinComponent {
private val ruleHasher: RuleHasher by inject()
Expand All @@ -15,7 +16,8 @@ class TargetHasher : KoinComponent {
sourceDigests: ConcurrentMap<String, ByteArray>,
ruleHashes: ConcurrentMap<String, ByteArray>,
seedHash: ByteArray?,
ignoredAttrs: Set<String>
ignoredAttrs: Set<String>,
modifiedFilepaths: Set<Path>
): ByteArray {
return when (target) {
is BazelTarget.GeneratedFile -> {
Expand All @@ -32,7 +34,8 @@ class TargetHasher : KoinComponent {
sourceDigests,
seedHash,
depPath = null,
ignoredAttrs
ignoredAttrs,
modifiedFilepaths
)
}
}
Expand All @@ -44,7 +47,8 @@ class TargetHasher : KoinComponent {
sourceDigests,
seedHash,
depPath = null,
ignoredAttrs
ignoredAttrs,
modifiedFilepaths
)
}
is BazelTarget.SourceFile -> sha256 {
Expand Down
Loading

0 comments on commit 79f4073

Please sign in to comment.