Skip to content

[cdac] Pipeline refactor + StackWalk doc cleanup + DataGenerator build fix#129244

Open
max-charlamb wants to merge 4 commits into
dotnet:mainfrom
max-charlamb:cdac-followups
Open

[cdac] Pipeline refactor + StackWalk doc cleanup + DataGenerator build fix#129244
max-charlamb wants to merge 4 commits into
dotnet:mainfrom
max-charlamb:cdac-followups

Conversation

@max-charlamb

Copy link
Copy Markdown
Member

Note

This PR was prepared with assistance from GitHub Copilot CLI.

Consolidated follow-ups to #128872 covering the cDAC stages in runtime-diagnostics.yml plus a small datacontract doc cleanup.

Commits (oldest first)

  1. efe2cdeee14[cdac] StackWalk.md: drop stress-harness detail from datacontract
    The new "Signature-Based Scanning (currently deferred)" section reads like stress-harness README content. Replaced with a one-sentence note + link to tests/StressTests/known-issues.md (which already documents the stub, the sentinel, and the ICallingConvention work needed to re-enable the scan).

  2. 0e4caf52fd5[cdac] Fix DataGeneratorTests build: override Target.ReadNInt
    [cDAC] Implement delegate inspection DacDbi APIs #128784 added public abstract TargetNInt ReadNInt(ulong address) to Target but didn't update the test-only TestTarget mock, so DataGeneratorTests no longer compiles. Adds the missing override, mirroring ReadNUInt.

  3. ebc2c062fbf[cdac pipeline] Refactor runtime-diagnostics: share one build, run unit tests, parallelize per-platform
    See the commit message for the full detail. Highlights:

    • Shared CdacBuild stage so coreclr+libs is built ONCE per platform (Checked) and consumed by Dump / Stress / XPlatDumpGen / XPlatDumpTest legs.
    • Stress vs dump cDAC split: stress uses the Release native cDAC shim (in the shared testhost); dumps use Debug managed cDAC assemblies, with libraries pinned at Release + pre-flight verification.
    • Per-platform parallelism via job-level dependsOnGlobalBuilds: CdacBuild (the canonical runtime.yml pattern). CdacXPlatDumpTest adds an explicit per-source-platform dependsOn so the whole thing fits in one stage.
    • Stress tests now run on every trigger (previously gated to not Schedule).
    • CdacUnitTests runs the cDAC managed-side unit tests + DataGeneratorTests on linux_x64 Checked using a minimal -s tools.cdactests -test (modeled after eng/pipelines/coreclr/ilasm.yml:52). Also adds the previously-unbuilt DataGeneratorTests project to the tools.cdactests subset.
    • 3 new reusable templates (sos-test-leg.yml, cdac-helix-test-leg.yml, download-cdac-build-artifact.yml) compress ~250 lines of repeated boilerplate.
    • Top-of-file overview header summarizing every stage. Build -> SOSTests, Cdac -> CdacTests.
    • runtime-diagnostics.yml shrinks from 581 -> 351 lines.

Files changed

  • docs/design/datacontracts/StackWalk.md
  • eng/Subsets.props
  • eng/pipelines/runtime-diagnostics.yml
  • eng/pipelines/cdac/prepare-cdac-helix-steps.yml
  • eng/pipelines/cdac/cdac-helix-test-leg.yml (new)
  • eng/pipelines/cdac/download-cdac-build-artifact.yml (new)
  • eng/pipelines/diagnostics/sos-test-leg.yml (new)
  • src/native/managed/cdac/tests/DataGenerator/TestTarget.cs

Status

Draft. Local YAML parses; needs an end-to-end pipeline run on this branch to validate per-platform parallelism + Debug cDAC payload prep + CdacUnitTests minimal-subset build all work as expected.

Supersedes #129237 and #129241 (will close those).

Max Charlamb and others added 3 commits June 10, 2026 11:26
The "Signature-Based Scanning (currently deferred)" section described
the stub state of GcScanner.PromoteCallerStack along with the
RecordDeferredFrame / CDAC_DEFERRED_FRAME sentinel mechanism the cDAC
stress harness uses. That content belongs in the stress test docs,
not in the StackWalk datacontract spec.

Replace the section with a one-sentence note in the GcScanRoots
dispatch list saying the fallback is currently stubbed and linking to
tests/StressTests/known-issues.md (which already documents the stub,
the sentinel, and the ICallingConvention work needed to re-enable
the scan) and tracking issue dotnet#127765.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
dotnet#128784 added `public abstract TargetNInt ReadNInt(ulong address)` to
Target but didn't update the test-only TestTarget mock in
DataGeneratorTests, so that project no longer compiles.

Add the missing override, mirroring the existing ReadNUInt pattern:
`new TargetNInt(PointerSize == 8 ? Read<long> : Read<int>)`.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…it tests, parallelize per-platform

Several related changes to the cDAC stages in runtime-diagnostics.yml:

1. Add a shared CdacBuild stage so coreclr+libs is built ONCE per
   platform (Checked runtime, Release cDAC), with its output published
   as a CdacBuildArtifacts_<plat> tar. Dump tests, stress tests, and
   the x-plat dump-gen leg all download this artifact instead of each
   running their own full clr+libs+tools.cdac+... build.

2. Wire test legs to consume the artifact. Each test leg still runs a
   tiny `-s tools.cdac<X>tests` to initialize .dotnet and rebuild the
   test csproj; the heavy coreclr/libs build is skipped.

3. Stress vs dump cDAC split:
   - Stress tests use the Release native cDAC shim, which ships in the
     shared testhost CdacBuild publishes.
   - Dump tests use the Debug managed cDAC assemblies. prepare-cdac-
     helix-steps.yml gains a cdacTestConfig parameter (defaults to
     $(_BuildConfig) for back-compat); dump legs pass Debug so MSBuild
     rebuilds the managed cDAC + DumpTests in Debug during payload prep.
     A librariesConfiguration parameter pins libraries at Release so
     the cdacTestConfig override never causes MSBuild to look for Debug
     libraries; a pre-flight check verifies that.

4. Per-platform parallelism. Originally each test stage used stage-
   level dependsOn (all-or-nothing -- linux_x64 tests waited for
   osx_arm64 to finish building). Move CdacBuild + CdacDumpTest +
   CdacStressTest + CdacXPlatDumpGen + CdacXPlatDumpTest into a single
   CdacTests stage and use job-level dependsOnGlobalBuilds: CdacBuild
   (the canonical runtime.yml pattern at lines 1591,1622,1653) for
   per-platform parallelism. CdacXPlatDumpTest additionally uses an
   explicit dependsOn list to fan-in on every source platform's
   CdacXPlatDumpGen, removing the need for a separate stage.

5. New CdacUnitTests stage runs the cDAC managed-side unit tests +
   DataGenerator tests on linux_x64 Checked via the existing
   tools.cdactests subset (extended in eng/Subsets.props to include
   the previously-unbuilt DataGeneratorTests project alongside
   UnitTests). Modeled after eng/pipelines/coreclr/ilasm.yml:52's
   minimal `-s tools.X -test` pattern.

6. Stress tests now run on every trigger (previously gated to not
   Schedule); the same Checked artifact CdacBuild publishes is what
   they need.

Holistic readability cleanup:

- Top-of-file pipeline overview header summarizes every stage, its
  trigger gating, and its dependency model so a reader gets the whole
  pipeline shape on screen one.
- Build stage renamed SOSTests (its actual purpose), Cdac stage
  renamed CdacTests.
- Extract three reusable templates:
    eng/pipelines/diagnostics/sos-test-leg.yml -- the SOS legs
      (cDAC/cDAC_no_fallback/DAC) were 3 nearly identical 50-line
      blocks differing only in `name`/`useCdac`/`noFallback`. Now 3
      4-line invocations.
    eng/pipelines/cdac/cdac-helix-test-leg.yml -- the dump-style
      legs (CdacDumpTest/CdacXPlatDumpGen/CdacXPlatDumpTest) shared
      the same artifact-download + prepare + send + fail-on-error
      shape. Parameterized over nameSuffix/platforms/sendParams/etc.
    eng/pipelines/cdac/download-cdac-build-artifact.yml -- 4-copy
      preBuildSteps block compressed to a one-line template ref.

Net: runtime-diagnostics.yml shrinks from 581 to 351 lines while the
behavior expands.

Files:
- eng/pipelines/runtime-diagnostics.yml
- eng/pipelines/cdac/prepare-cdac-helix-steps.yml
- eng/pipelines/cdac/cdac-helix-test-leg.yml (new)
- eng/pipelines/cdac/download-cdac-build-artifact.yml (new)
- eng/pipelines/diagnostics/sos-test-leg.yml (new)
- eng/Subsets.props -- DataGeneratorTests project added to
  tools.cdactests subset.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the cDAC/diagnostics CI pipeline definitions to reduce duplication (shared builds + reusable job templates), adds missing test build coverage for cDAC DataGenerator tests, and trims the StackWalk datacontract documentation around the currently-deferred signature-scanning path.

Changes:

  • Fix cDAC DataGeneratorTests compilation by adding the missing ReadNInt override to the test Target mock.
  • Add Microsoft.Diagnostics.DataContractReader.DataGeneratorTests.csproj to the tools.cdactests subset so it’s built/run in CI.
  • Refactor eng/pipelines/runtime-diagnostics.yml (new stages/templates, shared cDAC build artifact flow) and update cDAC Helix payload prep to support Debug cDAC assemblies while keeping libraries pinned.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
docs/design/datacontracts/StackWalk.md Removes detailed deferred signature-scan discussion; replaces with a short note + links.
eng/Subsets.props Adds DataGeneratorTests to tools.cdactests subset.
eng/pipelines/runtime-diagnostics.yml Major pipeline refactor: stage layout, shared cDAC build artifact, and template-based test legs.
eng/pipelines/cdac/prepare-cdac-helix-steps.yml Adds cDAC test configuration parameters + a libraries-output preflight check.
eng/pipelines/cdac/cdac-helix-test-leg.yml New reusable template for dump-style Helix legs (dump tests, xplat gen/test).
eng/pipelines/cdac/download-cdac-build-artifact.yml New reusable step template to download/extract shared CdacBuild artifacts.
eng/pipelines/diagnostics/sos-test-leg.yml New reusable template for SOS-style test legs on windows_x64.
src/native/managed/cdac/tests/DataGenerator/TestTarget.cs Implements ReadNInt to match updated Target abstract surface.

Comment thread eng/pipelines/runtime-diagnostics.yml Outdated
jobTemplate: /eng/pipelines/diagnostics/runtime-diag-job.yml
buildConfig: release
jobTemplate: /eng/pipelines/common/global-build-job.yml
buildConfig: debug
Comment on lines +32 to +33
$configMatches = Get-ChildItem -Directory $libsDir -ErrorAction SilentlyContinue |
Where-Object { $_.Name -like "*-${{ parameters.librariesConfiguration }}" }
…e convention

Three small changes on top of the previous pipeline refactor commit:

1. Rename DataGeneratorTests.csproj -> DataGenerator.Tests.csproj
   Arcade SDK's Tests.props gates the `Test` target on IsUnitTestProject
   which is auto-set only when MSBuildProjectName.EndsWith('.UnitTests')
   or EndsWith('.Tests'). The csproj name `DataGeneratorTests.csproj`
   ends in "Tests" but has no leading dot, so IsUnitTestProject stayed
   false, the Test target was skipped, and zero DataGen tests ran in CI
   (verified against build 1458128 -- all 501 published results came
   from Microsoft.Diagnostics.DataContractReader.Tests). Adding the dot
   matches the UnitTests project (.Tests.csproj) convention.

   Updates eng/Subsets.props and src/native/managed/cdac/cdac.slnx
   references too.

2. prepare-cdac-helix-steps.yml: drop the libraries-configuration
   pre-flight check. It was belt-and-suspenders that added nothing
   real -- if the libs are misaligned, the subsequent dotnet build
   fails with a clear unresolved-reference error. Also fixes the
   suffix-glob bug Copilot reviewer flagged.

3. runtime-diagnostics.yml header tweaks: overview now correctly says
   "3 SOS legs" (cDAC, cDAC_no_fallback, DAC) with testInterpreter
   inline, "CdacUnitTests linux_x64 Debug" matches the actual matrix
   config, and the Build stage comment about FEATURE_INTERPRETER is
   reworded for the no-separate-Interpreter-leg model.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.

Comment on lines +176 to 178
jobTemplate: /eng/pipelines/common/global-build-job.yml
buildConfig: debug
platforms:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants