Skip to content

Add Zstd GPU CI test infrastructure (Phase 1)#108

Merged
brohan203 merged 8 commits into
microsoft:developmentfrom
brohan203:users/rohanborkar/ds_testInfra_phase1
Jul 9, 2026
Merged

Add Zstd GPU CI test infrastructure (Phase 1)#108
brohan203 merged 8 commits into
microsoft:developmentfrom
brohan203:users/rohanborkar/ds_testInfra_phase1

Conversation

@brohan203

@brohan203 brohan203 commented Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown

Adds a thin GTest wrapper (zstdgpu_ci_tests) that shells out to zstdgpu_demo.exe to validate Zstd GPU decompression shaders in CI. Also adds pipeline YAML and a histogram generation script for perf results.

What's here

  • zstd/zstdgpu_ci_tests/ — GTest parameterized test files
    • main.cpp handles setup (CLI parsing, config, file discovery), and zstdgpu_ci_tests.cpp handles executions (test definitions, demo runner)
    • 4 correctness scenarios: SimulationCheck, D3D12DebugLayer, ExternalMemory, GraphicsQueue
    • 2 performance scenarios: OverallThroughput, PerStageTiming
    • Win32 CreateProcess runner with pipe capture and optional timeout
  • zstd/scripts/generate_histogram.py — Generates histogram PNGs from perf CSV output. This can probably be improved

Testing

Testing was executed both on local GPUs (dev box and lab machines) and with the full YAML pipeline

@brohan203 brohan203 force-pushed the users/rohanborkar/ds_testInfra_phase1 branch 2 times, most recently from 5115e9b to 1456908 Compare June 25, 2026 19:30
chmann-micro
chmann-micro reviewed Jun 29, 2026
View reviewed changes
Comment thread zstd/zstdgpu_ci_tests/main.cpp
Comment thread zstd/zstdgpu_ci_tests/main.cpp
Comment thread zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h Outdated
Comment thread zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h Outdated
Comment thread zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h Outdated
Comment thread zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp Outdated
Comment thread zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp
Comment thread zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp Outdated
Comment thread zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp Outdated
Comment thread zstd/zstd.sln Outdated
Rohan Borkar and others added 5 commits July 7, 2026 05:49
Add Zstd GPU CI test infrastructure (Phase 1)
57b8b13 
Add zstdgpu_ci_tests: a thin GTest wrapper that shells out to
zstdgpu_demo.exe for correctness and performance validation of GPU
Zstd decompression. Tests are parameterized over .zst content files
discovered at runtime via --content-path.

Test cases:
- SimulationCheck (--chk-gpu --chk-cpu --sim-gpu)
- D3D12DebugLayer (--d3d-dbg)
- ExternalMemory (--ext-mem)
- GraphicsQueue (--d3d-gfx)
- OverallThroughput (--prf-lvl 0)
- PerStageTiming (--prf-lvl 2)

Also adds scripts/generate_histogram.py for converting performance
CSV output into histogram PNGs suitable for CI reporting.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
zstdgpu_ci_tests: content-path validation + subdirectory-aware test n…
bcb5206 
…ames

Content-path validation warnings when --content-path is empty, missing, non-directory, or contains no .zst files (avoids user confusion with silent 'zero tests' gtest output).

Test name sanitization now uses path relative to --content-path instead of just filename stem, so same-leaf-name files across different subdirectories (e.g. firefly_albedo.DDS.zst under BC1/, BC1mip0/, block4K_*, etc.) don't collide and trigger a fatal gtest assertion at startup.

Extracted from the original commit '9266056e/aefb212b' which also included demo-side exit-code mechanism (dropped in favor of ATG's upcoming upstream PR).
histogram script handles inf values
ab23d5c
zstdgpu_ci_tests: address PR microsoft#108 review feedback (@chmann-m…
cc4ac18 
…icro)

Six groups of fixes to the CI test infrastructure:

  A. Validate all required inputs in main() with hard exit codes instead of
     silently skipping tests. Also use the non-throwing overload of
     std::filesystem::recursive_directory_iterator so a single unreadable
     subdirectory logs a warning and continues instead of aborting the
     entire process before any test runs.
  B. Cache the discovered .zst file list on TestConfig in main(); reuse in
     GetTestFiles(). Avoids walking the content tree twice.
  C. Remove result.stdOut from ASSERT_EQ / EXPECT_EQ failure messages -
     it was already printed unconditionally as the [DEMO OUT] block above,
     which caused each failing test's stdout to appear twice in the log.
  D. Remove GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(ZstdGpuDemoTests).
     Group A now hard-fails if the content path has zero .zst files, so
     empty instantiation can no longer happen at test time. Keeping the
     macro would let a mis-typed --content-path silently pass green with
     zero coverage.
  E. Call CancelIoEx(hReadPipe, nullptr) after TerminateProcess on the
     timeout path so any pending ReadFile in the drain thread returns and
     readerThread.join() cannot hang the test runner indefinitely. Trust
     that CancelIoEx alone unblocks the reader; no bounded-wait fallback.
  F. Replace placeholder GUID {A1B2C3D4-...} in zstd.sln with a real
     one: {17F60A53-4A7F-4107-AF0A-914497018D67}.
  G. Nits: drop the "Spec implies a global" and "Spec inlines the args"
     comments in the header, and the stray blank line.

Regression tested locally on RTX 2080 against a 20-file fuzz subset with
--run-count 5 (all 6 scenarios per file):

  Flavor         Before wall   After wall   Pass/fail counts
  -------------  -----------   ----------   ----------------
  r1+fflush         223.6s       225.5s      unchanged (58/125)
  vanilla+fflush    543.1s       528.2s      unchanged (58/125)

Wall time within run-to-run noise (+/-0.8%). Marker-count reductions
([FAIL] -50%, [INFO] -14.5%) are the expected consequence of Group C
removing the duplicate stdout print on failure - same content, half
the log lines when tests fail.
small cleanup, no change to functionality
7a5d8f2
@brohan203 brohan203 force-pushed the users/rohanborkar/ds_testInfra_phase1 branch from ee9e029 to 7a5d8f2 Compare July 7, 2026 14:18
@brohan203 brohan203 changed the title DRAFT: Add Zstd GPU CI test infrastructure (Phase 1) Add Zstd GPU CI test infrastructure (Phase 1) Jul 7, 2026
Rohan Borkar added 2 commits July 7, 2026 18:24
zstdgpu_ci_tests: gate D3D12DebugLayer's --d3d-dbg run on reference-c…
a3b1cb5 
…omparison failure

Per DirectStorage team spec (2026-07-07): GBV validation is expensive and
most tests pass reference comparison without needing any GBV diagnostic.
Skip the GBV cost for passing tests and only re-run with --d3d-dbg when
the reference comparison actually fails.

Behavior:

  Phase 1: spawn zstdgpu_demo.exe with just [--chk-gpu] (reference
           comparison only, no --d3d-dbg).
  If phase 1 exits 0 -> test passes, done. GBV cost skipped.
  If phase 1 fails  -> spawn again with [--chk-gpu, --d3d-dbg] and
                       append the GBV output to the same log block
                       under a "--- PHASE 2 (--d3d-dbg diagnostic
                       re-run) ---" marker.

The gtest pass/fail verdict is based on phase 1's exit code only; the
second run is diagnostic. Preserves the "one log block per gtest case"
contract so downstream log consumers don't need to know about the
two-phase flow.

Escape hatch: --force-gbv on the wrapper CLI bypasses the gate and runs
every D3D12DebugLayer test with --d3d-dbg up front (matches pre-gating
behavior). Useful for stress runs when you want unconditional GBV output
on all tests, not just failing ones.

Metrics: file-scope counter prints
  [D3D12DebugLayer] N of M test(s) re-ran with --d3d-dbg for GBV diagnostics.
at program exit, so operators can eyeball whether the gate is doing
meaningful work. Suppressed in --force-gbv mode.

Expected wall-time savings on the D3D12DebugLayer scenario: ~30% (skips
GBV overhead on the ~65% of tests that pass reference comparison).
Overall wrapper savings: ~5-6% (D3D12DebugLayer is 1 of 6 scenarios).
Bigger win is diagnostic clarity: GBV output only appears for genuinely
failing tests instead of being buried in every passing test's log.

Zero demo changes. Zero YAML changes. Wrapper-only. Fits ATG's "no demo
touches from us" constraint since --d3d-dbg is a demo flag they already
own.

Files:
  zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h     +1  (forceGbv TestConfig field)
  zstd/zstdgpu_ci_tests/main.cpp                +7  (--force-gbv arg + usage)
  zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp   +~95 (helpers + counters + gated body)

Verified locally on RTX 2080 (2 fuzz files, D3D12DebugLayer scenario
only): gated mode fires phase 2 on both failing files with clean log
markers + counter output; --force-gbv mode bypasses gate cleanly and
suppresses the summary line.
Revert "zstdgpu_ci_tests: gate D3D12DebugLayer's --d3d-dbg run on ref…
d93ec9f 
…erence-comparison failure"

This reverts commit a3b1cb5.
pm4rtx
pm4rtx requested changes Jul 9, 2026
View reviewed changes

@pm4rtx pm4rtx left a comment •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should be applying this change only in the branch.

Comment thread zstd/zstdgpu_demo/main.cpp Outdated
@brohan203 brohan203 force-pushed the users/rohanborkar/ds_testInfra_phase1 branch from 6d54ad2 to d93ec9f Compare July 9, 2026 17:58
zstdgpu_demo: preserve default auto-naming when --out-csv is not spec…
e8ce847 
…ified

Split the CSV-open path in two so automated callers and interactive users get what they each expect:

- When --out-csv <path> is passed explicitly (csvFilePathStorage != NULL), open exactly that path. Automated callers can then verify the CSV exists at the requested location.

- When --out-csv is not passed, keep the auto-naming behavior (append the .zst stem and a timestamp to the default "perf.csv") so repeated interactive runs don't clobber each other.
@brohan203 brohan203 merged commit 1553fc6 into microsoft:development Jul 9, 2026
1 check passed
@brohan203 brohan203 deleted the users/rohanborkar/ds_testInfra_phase1 branch July 9, 2026 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pm4rtx pm4rtx pm4rtx requested changes
@chmann-micro chmann-micro chmann-micro approved these changes
@coopp coopp Awaiting requested review from coopp
@larryche larryche Awaiting requested review from larryche

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@brohan203 @pm4rtx @chmann-micro