From a7080a14688e1ad0ac74994c1b85f2421f9be769 Mon Sep 17 00:00:00 2001 From: sterni Date: Thu, 21 Nov 2024 00:30:52 +0100 Subject: refactor(nix/buildkite): don't calculate deps for skipped targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don't need to calculate dependencies between and on targets that are part of the parent target map since they will be skipped by buildkite anyways. This speeds up 🦙 considerably for pipeline runs that have a limited number of changed targets and a parent target map passed in (i.e. pipeline runs of most CLs, but not canon runs). In my testing it was about a minute faster (1/6 of the time 🦙 takes currently) for a pipeline where under five drv targets changed. For the full pipeline (i.e. no parentTargetMap) 🦙 takes about the same time as before (it's a few seconds slower as is to be expected, but nothing significant). Change-Id: Ia5a80e142da8f40bc591e2c6cfaf48c325b2f577 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12818 Reviewed-by: tazjin Autosubmit: sterni Tested-by: BuildkiteCI --- nix/buildkite/default.nix | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/nix/buildkite/default.nix b/nix/buildkite/default.nix index 9abba9408ada..d9680136100d 100644 --- a/nix/buildkite/default.nix +++ b/nix/buildkite/default.nix @@ -99,11 +99,7 @@ rec { # # See //nix/dependency-analyzer for documentation on the structure of `targetDepMap`. getTargetPipelineDeps = targetDepMap: drvPath: - # Sanity check: We should only call this function on targets explicitly - # passed to mkPipeline. Thus it should have been passed as a “known” drv to - # dependency-analyzer. - assert targetDepMap.${drvPath}.known; - builtins.map keyForDrv targetDepMap.${drvPath}.knownDeps; + builtins.map keyForDrv (targetDepMap.${drvPath}.knownDeps or [ ]); # Create a pipeline step from a single target. mkStep = { headBranch, parentTargetMap, targetDepMap, target, cancelOnBuildFailing }: @@ -231,7 +227,19 @@ rec { buildEnabled = elem "build" enabledPhases; # Dependency relations between the `drvTargets`. See also //nix/dependency-analyzer. - targetDepMap = dependency-analyzer (dependency-analyzer.drvsToPaths drvTargets); + targetDepMap = + let + # Only calculate dependencies between drvTargets that were not part of + # the previous pipeline (per parentTargetMap). Unchanged targets will + # be skipped (assumed already built), so it's useless to emit deps + # on their steps. + changedDrvTargets = builtins.filter + (target: + parentTargetMap.${mkLabel target}.drvPath or null != target.drvPath + ) + drvTargets; + in + dependency-analyzer (dependency-analyzer.drvsToPaths changedDrvTargets); # Convert a target into all of its steps, separated by build # phase (as phases end up in different chunks). -- cgit 1.4.1