Skip to content

Speed up CI by reusing builds and avoiding standalone artifact copies - #4745

Open
thomhurst wants to merge 1 commit into
mainfrom
perf/ci-reuse-build-output
Open

Speed up CI by reusing builds and avoiding standalone artifact copies#4745
thomhurst wants to merge 1 commit into
mainfrom
perf/ci-reuse-build-output

Conversation

@thomhurst

@thomhurst thomhurst commented Sep 6, 2026

Copy link
Copy Markdown
Owner

CI currently builds every solution before launching the pipeline, then evaluates the same solutions again and stages a build artifact even when all tests run in the same checkout. In run 34016562116, BuildSolutionsModule took 3m 6s and uploaded a 2.7 GB archive after the native build had already completed.

This change reuses the successful Release build through Pipeline__BuildAlreadyCompleted and launches the pipeline with --no-build. Standalone runs use their existing outputs without staging or restoring an archive. Distributed runs retain artifact publication and share one in-flight/completed restore per pipeline process. The required-check job also uses sparse checkout for its single validation script.

BuildSolutionsModule still completes successfully as a dependency when reusing outputs, and ordinary runs still build when the new setting is false. Validation coverage and existing routing remain unchanged. The sampled overhead identifies the work removed; end-to-end savings still need measurement in CI.

Validation:

  • Three focused BuildOutputSharingTests passed with coverage: standalone behavior, concurrent distributed restore sharing, and failure propagation.
  • actionlint .github/workflows/dotnet.yml passed.
  • Test-RequiredPipelineContext.ps1 and Test-ResolveGeneratedIntegrationValidation.ps1 passed.
  • git diff --check passed.
  • Local .NET commands used scripts/Invoke-AgentDotNet.ps1 with its default resource limits. A scoped build of ModularPipelines.Build.csproj encountered missing Sourcy-generated Projects members and MP0006 in the unchanged FindProjectsModule. No compiler errors were reported in the changed files. The full solution and build pipeline were not run locally.

Summary by CodeRabbit

  • New Features

    • Improved distributed pipeline execution by sharing restored build outputs across concurrent pipeline instances.
    • Added support for skipping redundant solution builds when Release outputs are already available.
    • Unit tests can now run using shared or previously generated build outputs.
  • CI/CD

    • Pipeline execution now reuses successful Release builds without rebuilding.
    • Required pipeline validation uses a minimal checkout for faster execution.

@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 64bcbb3b-48e8-4aed-a1aa-85f7d2e84577

📥 Commits

Reviewing files that changed from the base of the PR and between 721e0ab and c7d84c3.

📒 Files selected for processing (8)
  • .github/workflows/dotnet.yml
  • src/ModularPipelines.Build/Helpers/BuildOutputSharing.cs
  • src/ModularPipelines.Build/Modules/BuildSolutionsModule.cs
  • src/ModularPipelines.Build/Modules/UnitTests/RunUnitTestModule.cs
  • src/ModularPipelines.Build/Program.cs
  • src/ModularPipelines.Build/Settings/PipelineSettings.cs
  • test/ModularPipelines.UnitTests/Build/BuildOutputSharingTests.cs
  • test/ModularPipelines.UnitTests/ModularPipelines.UnitTests.csproj

Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The pipeline now reuses compiled Release outputs. Distributed instances share one build-output artifact restore. Solution builds can be skipped when CI already built them. Unit tests restore shared output explicitly, and CI runs the pipeline with --no-build.

Changes

Build Output Sharing

Layer / File(s) Summary
Shared output restoration
src/ModularPipelines.Build/Helpers/BuildOutputSharing.cs, src/ModularPipelines.Build/Program.cs, test/ModularPipelines.UnitTests/Build/*, test/ModularPipelines.UnitTests/ModularPipelines.UnitTests.csproj
The new helper shares one artifact restore task across distributed pipeline instances. The service is registered as a singleton. Tests cover standalone, concurrent, completed, and failed restores.
Build and test module integration
src/ModularPipelines.Build/Settings/PipelineSettings.cs, src/ModularPipelines.Build/Modules/BuildSolutionsModule.cs, src/ModularPipelines.Build/Modules/UnitTests/RunUnitTestModule.cs
BuildAlreadyCompleted controls solution builds. Unit tests restore build output explicitly before test discovery.
CI build reuse
.github/workflows/dotnet.yml
The workflow runs Release with --no-build, sets Pipeline__BuildAlreadyCompleted, and uses sparse checkout for the required pipeline script.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to c7d84

This change reuses successful Release build outputs and avoids redundant CI build work while retaining normal build behavior when reuse is disabled. The shared restore and workflow paths have focused coverage, with no current merge-blocking risk identified.

Sequence Diagram(s)

sequenceDiagram
  participant CI
  participant BuildSolutionsModule
  participant BuildOutputSharing
  participant RunUnitTestModule
  participant ArtifactContext
  CI->>BuildSolutionsModule: Start with BuildAlreadyCompleted=true
  BuildSolutionsModule-->>CI: Skip solution build
  RunUnitTestModule->>BuildOutputSharing: RestoreAsync
  BuildOutputSharing->>ArtifactContext: Download shared build-output
  ArtifactContext-->>BuildOutputSharing: Restore compiled outputs
  BuildOutputSharing-->>RunUnitTestModule: Return restored output
  RunUnitTestModule->>RunUnitTestModule: Discover and run tests
Loading

Poem

A rabbit sees the builds already bright
Shared outputs hop from node to node
One download serves the waiting crowd
Tests follow paths through Release ground
No second build disturbs the code
CI nibbles less and runs along

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 6 files. (2 skipped: 2 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: reusing successful builds and reducing unnecessary artifact handling to speed up CI.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 6 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/ci-reuse-build-output

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Code review

Reviewed the CI build-reuse change (workflow diff, BuildOutputSharing, BuildSolutionsModule, RunUnitTestModule, PipelineSettings, and the new unit tests).

What I checked and confirmed is sound:

  • Pipeline__BuildAlreadyCompleted maps correctly to PipelineSettings.BuildAlreadyCompleted via the existing Pipeline: config section / __ env-var convention, and is only set in the CI step that runs after the native dotnet build loop over BuildSolutions.txt (which includes ModularPipelines.All.slnx, so the ModularPipelines.Build executable itself is already built before dotnet run --no-build is invoked).
  • BuildOutputSharing.IsEnabled (TotalInstances > 1) is currently always false in the actual CI job (distributed/Redis mode is disabled per the existing comment in the workflow), so the new skip-staging path in BuildSolutionsModule and the RestoreAsync call in RunUnitTestModule are inert today and only exercised by the new unit tests — consistent with what the PR description says ("Distributed runs retain artifact publication..."). Not a bug, just worth knowing the distributed path isn't currently integration-tested by CI itself.
  • Removing [ConsumesArtifact(typeof(BuildSolutionsModule), "build-output", ...)] from RunUnitTestModule doesn't break artifact validation (ArtifactContractValidator only validates from the consumer-attribute side) or distributed uploads (TryUploadArtifactsAsync/TryUploadCachedArtifactsAsync upload all [ProducesArtifact]-declared artifacts unconditionally in distributed mode, independent of any [ConsumesArtifact] attribute), and the manual context.Artifacts.DownloadAsync(...) call is an existing, already-public API.
  • BuildOutputSharing's lazy shared-task caching (_restoreTask ??= ... under a lock) is a correct, standard single-flight pattern, matches the three new unit tests (standalone/no-op, concurrent sharing, failure propagation to all callers), and is registered once as a singleton so all RunUnitTestModule subclasses share exactly one restore per process.
  • context.Services.GetRequiredService<T>() is IServicesContext's own method (not an extension needing an extra using), and the linked-compile test file has all the transitive package/project references it needs (Microsoft.Extensions.Options comes in transitively via ModularPipelines.csprojMicrosoft.Extensions.Hosting, already relied on elsewhere in this project).
  • The sparse-checkout addition to required-pipeline's checkout is safe — scripts/Assert-RequiredPipelineContext.ps1 is self-contained with no relative-path dependencies on other repo files.

No actionable bugs or CLAUDE.md violations found. CodeRabbit's automated pass also reported no merge-blocking risk (only a docstring-coverage nit, which is a style/documentation concern rather than a correctness issue).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant