From 57b8b13a3117233cd3600ad8823a72aa7e2941c0 Mon Sep 17 00:00:00 2001 From: Rohan Borkar Date: Thu, 4 Jun 2026 11:07:52 -0700 Subject: [PATCH 1/8] Add Zstd GPU CI test infrastructure (Phase 1) 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> --- zstd/scripts/generate_histogram.py | 67 +++ zstd/zstd.sln | 14 + zstd/zstdgpu_ci_tests/main.cpp | 152 +++++++ zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp | 393 ++++++++++++++++++ zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h | 73 ++++ .../zstdgpu_ci_tests/zstdgpu_ci_tests.vcxproj | 239 +++++++++++ 6 files changed, 938 insertions(+) create mode 100644 zstd/scripts/generate_histogram.py create mode 100644 zstd/zstdgpu_ci_tests/main.cpp create mode 100644 zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp create mode 100644 zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h create mode 100644 zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.vcxproj diff --git a/zstd/scripts/generate_histogram.py b/zstd/scripts/generate_histogram.py new file mode 100644 index 0000000..59a1295 --- /dev/null +++ b/zstd/scripts/generate_histogram.py @@ -0,0 +1,67 @@ +#!/usr/bin/env python3 +""" +Generate histogram PNGs from zstdgpu_demo performance CSV output. + +Usage: + python generate_histogram.py --input --output [--title ] + +Supports two CSV formats based on profiling level: + - prf-lvl 0 (OverallThroughput): column "Throughput_GBs" + - prf-lvl 2 (PerStageTiming): column "Microseconds" +""" + +import argparse +import csv +import sys + +def try_import_matplotlib(): + try: + import matplotlib + matplotlib.use("Agg") # Non-interactive backend for CI + import matplotlib.pyplot as plt + return plt + except ImportError: + print( + "WARNING: matplotlib not installed. Install with: pip install matplotlib", + file=sys.stderr, + ) + return None + + +def main(): + parser = argparse.ArgumentParser(description="Generate histogram from zstdgpu perf CSV") + parser.add_argument("--input", required=True, help="Path to input CSV file") + parser.add_argument("--output", required=True, help="Path to output PNG file") + parser.add_argument("--title", default="Throughput", help="Chart title") + args = parser.parse_args() + + plt = try_import_matplotlib() + if plt is None: + return 1 + + data = [] + with open(args.input, newline="") as f: + reader = csv.DictReader(f) + for row in reader: + if "Throughput_GBs" in row: + data.append(float(row["Throughput_GBs"])) + elif "Microseconds" in row: + data.append(float(row["Microseconds"])) + + if not data: + print(f"No Throughput_GBs or Microseconds data found in {args.input}", file=sys.stderr) + return 1 + + plt.figure() + plt.hist(data, bins=20) + plt.xlabel("Throughput (GB/s)" if "throughput" in args.input.lower() else "Time (us)") + plt.ylabel("Count") + plt.title(args.title) + plt.savefig(args.output) + plt.close() + print(f"Generated: {args.output}") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/zstd/zstd.sln b/zstd/zstd.sln index 7c5888c..8c30eb9 100644 --- a/zstd/zstd.sln +++ b/zstd/zstd.sln @@ -11,6 +11,8 @@ Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "zstdgpu_tests", "zstdgpu_te EndProject Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "googletest_static", "ThirdParty\googletest_static.vcxproj", "{49811F10-3D14-403E-859D-40DFCBB35C7B}" EndProject +Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "zstdgpu_ci_tests", "zstdgpu_ci_tests\zstdgpu_ci_tests.vcxproj", "{A1B2C3D4-E5F6-7890-ABCD-EF1234567890}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|ARM64 = Debug|ARM64 @@ -69,6 +71,18 @@ Global {49811F10-3D14-403E-859D-40DFCBB35C7B}.Release|x64.Build.0 = Release|x64 {49811F10-3D14-403E-859D-40DFCBB35C7B}.Release|x86.ActiveCfg = Release|Win32 {49811F10-3D14-403E-859D-40DFCBB35C7B}.Release|x86.Build.0 = Release|Win32 + {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Debug|ARM64.ActiveCfg = Debug|ARM64 + {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Debug|ARM64.Build.0 = Debug|ARM64 + {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Debug|x64.ActiveCfg = Debug|x64 + {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Debug|x64.Build.0 = Debug|x64 + {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Debug|x86.ActiveCfg = Debug|Win32 + {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Debug|x86.Build.0 = Debug|Win32 + {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Release|ARM64.ActiveCfg = Release|ARM64 + {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Release|ARM64.Build.0 = Release|ARM64 + {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Release|x64.ActiveCfg = Release|x64 + {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Release|x64.Build.0 = Release|x64 + {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Release|x86.ActiveCfg = Release|Win32 + {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Release|x86.Build.0 = Release|Win32 EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/zstd/zstdgpu_ci_tests/main.cpp b/zstd/zstdgpu_ci_tests/main.cpp new file mode 100644 index 0000000..3a52744 --- /dev/null +++ b/zstd/zstdgpu_ci_tests/main.cpp @@ -0,0 +1,152 @@ +/** + * Copyright (c) Microsoft. All rights reserved. + * This code is licensed under the MIT License (MIT). + * THIS CODE IS PROVIDED *AS IS* WITHOUT WARRANTY OF + * ANY KIND, EITHER EXPRESS OR IMPLIED, INCLUDING ANY + * IMPLIED WARRANTIES OF FITNESS FOR A PARTICULAR + * PURPOSE, MERCHANTABILITY, OR NON-INFRINGEMENT. + */ + +// Entry point for the Zstd GPU CI tests. This is a thin GTest wrapper +// that shells out to zstdgpu_demo.exe to validate Zstd GPU decompression shaders. +// +// - parses custom CLI flags (--content-path, --demo-path, etc.), resolves the +// demo executable, then hands off to GTest which runs parameterized tests defined +// in zstdgpu_ci_tests.cpp. Each test spawns the demo as a child process. +// +// If no .zst content files are found, zero tests are instantiated and the test +// binary exits 0 (success). If the demo exe is missing, tests are skipped (not failed). +// +// This file also implements the TestConfig singleton and file discovery helpers +// declared in zstdgpu_ci_tests.h. + +#include "zstdgpu_ci_tests.h" +#include <gtest/gtest.h> +#include <algorithm> +#include <cstring> +#include <filesystem> +#include <iostream> +#include <string> + +// TestConfig singleton +// Implementation of the singleton declared in zstdgpu_ci_tests.h. + +static TestConfig g_testConfig; + +const TestConfig& GetTestConfig() +{ + return g_testConfig; +} + +void SetTestConfig(TestConfig config) +{ + g_testConfig = std::move(config); +} + +// File discovery + +std::vector<std::string> DiscoverZstFiles(const std::string& contentPath) +{ + std::vector<std::string> files; + + if (contentPath.empty() || !std::filesystem::exists(contentPath) || !std::filesystem::is_directory(contentPath)) + { + return files; + } + + for (const auto& entry : std::filesystem::recursive_directory_iterator(contentPath)) + { + if (entry.is_regular_file() && entry.path().extension() == ".zst") + { + files.push_back(entry.path().string()); + } + } + + std::sort(files.begin(), files.end()); + return files; +} + +// CLI and entry point + +// QOL for diagnostics. For running manually +// Activate with --help-ci to avoid conflicting with GTest's own --help output. +static void PrintUsage(const char* exe) +{ + std::cout << "Usage: " << exe << " [gtest_options] [options]\n" + << "\n" + << "Options:\n" + << " --content-path <dir> Directory containing .zst test files\n" + << " --demo-path <path> Path to zstdgpu_demo.exe\n" + << " --log-dir <dir> Directory for logs and CSV output\n" + << " --log-file <path> Consolidated text log file\n" + << " --run-count <N> Perf test iteration count (default: 40)\n" + << " --timeout <seconds> Per-test process timeout (default: no timeout)\n" + << std::endl; +} + +int main(int argc, char** argv) +{ + // Parse custom flags before handing off to GTest. GTest's InitGoogleTest() + // is called later and will consume its own flags (e.g. --gtest_filter). + TestConfig config; + + for (int i = 1; i < argc; ++i) + { + if (std::strcmp(argv[i], "--content-path") == 0 && i + 1 < argc) + { + config.contentPath = argv[++i]; + } + else if (std::strcmp(argv[i], "--demo-path") == 0 && i + 1 < argc) + { + config.demoPath = argv[++i]; + } + else if (std::strcmp(argv[i], "--log-dir") == 0 && i + 1 < argc) + { + config.logDir = argv[++i]; + } + else if (std::strcmp(argv[i], "--log-file") == 0 && i + 1 < argc) + { + config.logFile = argv[++i]; + } + else if (std::strcmp(argv[i], "--run-count") == 0 && i + 1 < argc) + { + config.runCount = std::atoi(argv[++i]); + if (config.runCount <= 0) + config.runCount = 40; + } + else if (std::strcmp(argv[i], "--timeout") == 0 && i + 1 < argc) + { + config.timeoutSeconds = std::atoi(argv[++i]); + if (config.timeoutSeconds < 0) + config.timeoutSeconds = 0; + } + else if (std::strcmp(argv[i], "--help-ci") == 0) + { + PrintUsage(argv[0]); + return 0; + } + } + + if (config.demoPath.empty()) + { + std::cerr << "Warning: --demo-path not set. Tests will skip.\n"; + } + + // Default log dir to current directory. + if (config.logDir.empty()) + { + config.logDir = std::filesystem::current_path().string(); + } + + // Ensure log directory exists. + if (!std::filesystem::exists(config.logDir)) + { + std::filesystem::create_directories(config.logDir); + } + + SetTestConfig(std::move(config)); + + testing::InitGoogleTest(&argc, argv); + testing::GTEST_FLAG(catch_exceptions) = false; + return RUN_ALL_TESTS(); +} diff --git a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp new file mode 100644 index 0000000..7d60c86 --- /dev/null +++ b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp @@ -0,0 +1,393 @@ +/** + * Copyright (c) Microsoft. All rights reserved. + * This code is licensed under the MIT License (MIT). + * THIS CODE IS PROVIDED *AS IS* WITHOUT WARRANTY OF + * ANY KIND, EITHER EXPRESS OR IMPLIED, INCLUDING ANY + * IMPLIED WARRANTIES OF FITNESS FOR A PARTICULAR + * PURPOSE, MERCHANTABILITY, OR NON-INFRINGEMENT. + */ + +// Test definitions and demo runner for the Zstd GPU CI tests. +// +// Contains a single parameterized test suite (ZstdGpuDemoTests) instantiated +// once per .zst file found in the content directory. Each file gets 6 test +// scenarios: +// +// Correctness tests (4 scenarios per file): +// - SimulationCheck: Software GPU simulation (--sim-gpu) with CPU+GPU validation +// - D3D12DebugLayer: Hardware GPU with D3D12 debug layer (--d3d-dbg) +// - ExternalMemory: External memory mode (--ext-mem) +// - GraphicsQueue: Graphics queue instead of compute (--d3d-gfx) +// +// Performance tests (2 scenarios per file): +// - OverallThroughput: Profiling level 0 — CSV: results/throughput_<stem>.csv +// - PerStageTiming: Profiling level 2 — CSV: results/stages_<stem>.csv +// Performance tests use EXPECT (not ASSERT) — they fail on infrastructure +// errors but do not check performance values against thresholds. +// +// If no .zst files are found, GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST +// prevents GTest from reporting an error — zero tests run, exit code 0. +// +// The demo runner at the bottom of this file spawns zstdgpu_demo.exe as a child +// process using Win32 CreateProcess with anonymous pipes. A background thread +// drains stdout to avoid pipe-buffer deadlocks. If the process exceeds the +// configured timeout, it is terminated. This avoids any D3D12/GPU dependency +// in the test binary itself — all GPU work happens inside the demo process. + +#include "zstdgpu_ci_tests.h" +#include <gtest/gtest.h> +#include <array> +#include <chrono> +#include <cstdio> +#include <filesystem> +#include <fstream> +#include <iostream> +#include <sstream> +#include <thread> +#include <Windows.h> + +// Helpers + +// Returns the list of .zst files to parameterize over. +// GTest evaluates this lazily when the test suite is instantiated (after main has parsed CLI args and set TestConfig), so contentPath is available here. +static std::vector<std::string> GetTestFiles() +{ + const auto& config = GetTestConfig(); + return DiscoverZstFiles(config.contentPath); +} + +// Converts a full file path to a valid GTest parameter name. +// GTest names must be alphanumeric + underscore, no leading digits. +static std::string SanitizeTestName(const testing::TestParamInfo<std::string>& info) +{ + std::string name = std::filesystem::path(info.param).stem().string(); + std::string result; + result.reserve(name.size()); + for (char c : name) + { + result += std::isalnum(static_cast<unsigned char>(c)) ? c : '_'; + } + if (!result.empty() && std::isdigit(static_cast<unsigned char>(result[0]))) + { + result = "_" + result; + } + return result.empty() ? "Unknown" : result; +} + +// Appends per-test output to the consolidated log file (--log-file). +static void WriteToLogFile(const std::string& zstFile, const DemoResult& result) +{ + const auto& config = GetTestConfig(); + if (config.logFile.empty()) + return; + + std::ofstream log(config.logFile, std::ios::app); + auto* testInfo = ::testing::UnitTest::GetInstance()->current_test_info(); + log << "=== " << testInfo->test_suite_name() << "." << testInfo->name() << " ===\n"; + log << "File: " << zstFile << "\n"; + log << "Exit code: " << result.exitCode << "\n"; + log << result.stdOut << "\n"; +} + +// Test runners + +// Run a correctness scenario. Spawns zstdgpu_demo.exe with the given .zst file and scenario flags, then asserts exit code == 0. +// Failures include the full command line and demo stdout for diagnostic output. +static void RunCorrectnessTest(const std::string& zstFile, const std::vector<std::string>& scenarioFlags) +{ + const auto& config = GetTestConfig(); + + if (config.demoPath.empty()) + { + GTEST_SKIP() << "zstdgpu_demo.exe not found. Set --demo-path."; + } + + auto args = BuildCorrectnessArgs(zstFile, scenarioFlags); + auto result = RunDemo(config.demoPath, args, config.timeoutSeconds); + + // Write to log file before assertions so logs are captured even if an ASSERT aborts early. + WriteToLogFile(zstFile, result); + + // Log the output regardless of pass/fail. + std::cout << "[DEMO CMD] " << result.commandLine << "\n"; + if (!result.stdOut.empty()) + { + std::cout << "[DEMO OUT] " << result.stdOut << "\n"; + } + + ASSERT_FALSE(result.timedOut) + << "Demo process timed out after " << config.timeoutSeconds << " seconds.\n" + << "Command: " << result.commandLine; + + ASSERT_TRUE(result.launchError.empty()) + << "Failed to launch demo: " << result.launchError << "\n" + << "Command: " << result.commandLine; + + ASSERT_EQ(result.exitCode, 0) + << "Demo process returned non-zero exit code: " << result.exitCode << "\n" + << "Command: " << result.commandLine << "\n" + << "Output:\n" + << result.stdOut; +} + +// Run a performance scenario. Spawns zstdgpu_demo.exe with profiling flags and requests CSV output. Uses EXPECT (not ASSERT) to verify the demo executed successfully and produced CSV output. +static void RunPerformanceTest(const std::string& zstFile, int profilingLevel) +{ + const auto& config = GetTestConfig(); + + if (config.demoPath.empty()) + { + GTEST_SKIP() << "zstdgpu_demo.exe not found. Set --demo-path."; + } + + // Build CSV output path matching spec convention: + // prf-lvl 0 → results/throughput_<stem>.csv + // prf-lvl 2 → results/stages_<stem>.csv + std::string stem = std::filesystem::path(zstFile).stem().string(); + std::string prefix = (profilingLevel == 0) ? "throughput" : "stages"; + std::filesystem::path resultsDir = std::filesystem::path(config.logDir) / "results"; + if (!std::filesystem::exists(resultsDir)) + { + std::filesystem::create_directories(resultsDir); + } + std::string csvPath = (resultsDir / (prefix + "_" + stem + ".csv")).string(); + + auto args = BuildPerformanceArgs(zstFile, profilingLevel, config.runCount, csvPath); + auto result = RunDemo(config.demoPath, args, config.timeoutSeconds); + + // Write to log file before assertions so logs are captured even if a check fails. + WriteToLogFile(zstFile, result); + + std::cout << "[DEMO CMD] " << result.commandLine << "\n"; + if (!result.stdOut.empty()) + { + std::cout << "[DEMO OUT] " << result.stdOut << "\n"; + } + + EXPECT_FALSE(result.timedOut) + << "Demo process timed out after " << config.timeoutSeconds << " seconds.\n" + << "Command: " << result.commandLine; + + EXPECT_TRUE(result.launchError.empty()) + << "Failed to launch demo: " << result.launchError << "\n" + << "Command: " << result.commandLine; + + EXPECT_EQ(result.exitCode, 0) + << "Demo process returned non-zero exit code: " << result.exitCode << "\n" + << "Command: " << result.commandLine << "\n" + << "Output:\n" + << result.stdOut; + + EXPECT_TRUE(std::filesystem::exists(csvPath)) << "CSV not created: " << csvPath; + + if (std::filesystem::exists(csvPath)) + { + std::cout << "[PERF CSV] Written to: " << csvPath << "\n"; + } +} + +// Test fixture and test cases + +// Test fixture parameterized over .zst file paths (spec: ZstdGpuDemoTests). +// Both correctness and performance tests share this fixture — correctness tests +// use ASSERT (hard fail), performance tests use EXPECT (soft fail). +class ZstdGpuDemoTests : public ::testing::TestWithParam<std::string> +{ +}; + +GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(ZstdGpuDemoTests); + +// --- Correctness tests --- + +TEST_P(ZstdGpuDemoTests, SimulationCheck) +{ + RunCorrectnessTest(GetParam(), {"--chk-gpu", "--chk-cpu", "--sim-gpu"}); +} + +TEST_P(ZstdGpuDemoTests, D3D12DebugLayer) +{ + RunCorrectnessTest(GetParam(), {"--chk-gpu", "--d3d-dbg"}); +} + +TEST_P(ZstdGpuDemoTests, ExternalMemory) +{ + RunCorrectnessTest(GetParam(), {"--chk-gpu", "--ext-mem"}); +} + +TEST_P(ZstdGpuDemoTests, GraphicsQueue) +{ + RunCorrectnessTest(GetParam(), {"--chk-gpu", "--d3d-gfx"}); +} + +// --- Performance tests --- + +TEST_P(ZstdGpuDemoTests, OverallThroughput) +{ + RunPerformanceTest(GetParam(), 0); +} + +TEST_P(ZstdGpuDemoTests, PerStageTiming) +{ + RunPerformanceTest(GetParam(), 2); +} + +INSTANTIATE_TEST_SUITE_P( + ContentTests, + ZstdGpuDemoTests, + ::testing::ValuesIn(GetTestFiles()), + SanitizeTestName); + +// Demo runner implementation + +// Builds a command line string with proper quoting for arguments containing spaces. +static std::string BuildCommandLine(const std::string& exe, const std::vector<std::string>& args) +{ + std::ostringstream cmd; + cmd << "\"" << exe << "\""; + for (const auto& arg : args) + { + cmd << " "; + if (arg.find(' ') != std::string::npos) + cmd << "\"" << arg << "\""; + else + cmd << arg; + } + return cmd.str(); +} + +DemoResult RunDemo( + const std::string& demoPath, + const std::vector<std::string>& args, + int timeoutSeconds) +{ + DemoResult result; + result.commandLine = BuildCommandLine(demoPath, args); + + // Create an anonymous pipe for capturing the child process's stdout/stderr. + SECURITY_ATTRIBUTES sa{}; + sa.nLength = sizeof(sa); + sa.bInheritHandle = TRUE; + + HANDLE hReadPipe = nullptr; + HANDLE hWritePipe = nullptr; + if (!CreatePipe(&hReadPipe, &hWritePipe, &sa, 0)) + { + result.launchError = "Failed to create pipe for demo process."; + return result; + } + + // Prevent the read end from being inherited by the child process. + SetHandleInformation(hReadPipe, HANDLE_FLAG_INHERIT, 0); + + // Redirect child's stdout and stderr to the write end of the pipe. + STARTUPINFOA si{}; + si.cb = sizeof(si); + si.dwFlags = STARTF_USESTDHANDLES; + si.hStdOutput = hWritePipe; + si.hStdError = hWritePipe; + + PROCESS_INFORMATION pi{}; + + std::vector<char> cmdBuf(result.commandLine.begin(), result.commandLine.end()); + cmdBuf.push_back('\0'); + + if (!CreateProcessA( + nullptr, + cmdBuf.data(), + nullptr, + nullptr, + TRUE, // inherit handles + 0, + nullptr, + nullptr, + &si, + &pi)) + { + CloseHandle(hReadPipe); + CloseHandle(hWritePipe); + result.launchError = "Failed to launch demo process. Error: " + std::to_string(GetLastError()); + return result; + } + + // Close the write end in the parent so ReadFile on the read end returns + // EOF when the child exits. + CloseHandle(hWritePipe); + + // Read the child's output on a background thread to prevent pipe buffer + // deadlocks (the pipe has a finite buffer; if it fills, the child blocks). + std::string capturedOutput; + std::thread readerThread([&capturedOutput, hReadPipe]() { + std::array<char, 4096> buf; + DWORD bytesRead = 0; + while (ReadFile(hReadPipe, buf.data(), static_cast<DWORD>(buf.size()), &bytesRead, nullptr) && bytesRead > 0) + { + capturedOutput.append(buf.data(), bytesRead); + } + }); + + // Wait for the child process, enforcing the timeout. + DWORD waitMs = (timeoutSeconds > 0) ? static_cast<DWORD>(timeoutSeconds) * 1000 : INFINITE; + DWORD waitResult = WaitForSingleObject(pi.hProcess, waitMs); + + if (waitResult == WAIT_TIMEOUT) + { + result.timedOut = true; + TerminateProcess(pi.hProcess, 1); + WaitForSingleObject(pi.hProcess, 5000); + } + + DWORD exitCode = 0; + GetExitCodeProcess(pi.hProcess, &exitCode); + result.exitCode = static_cast<int>(exitCode); + + CloseHandle(pi.hProcess); + CloseHandle(pi.hThread); + + // Wait for the reader thread to finish draining the pipe, then clean up. + readerThread.join(); + CloseHandle(hReadPipe); + + result.stdOut = std::move(capturedOutput); + return result; +} + +// Builds argument list for correctness tests: decompress once (--run-cnt 1) +// with GPU and CPU validation enabled, plus scenario-specific flags. +std::vector<std::string> BuildCorrectnessArgs( + const std::string& zstFile, + const std::vector<std::string>& scenarioFlags) +{ + std::vector<std::string> args; + args.push_back("--zst"); + args.push_back(zstFile); + args.push_back("--run-cnt"); + args.push_back("1"); + for (const auto& flag : scenarioFlags) + { + args.push_back(flag); + } + return args; +} + +// Builds argument list for performance tests: run N iterations at the specified +// profiling level, optionally writing per-run timing data to a CSV file. +std::vector<std::string> BuildPerformanceArgs( + const std::string& zstFile, + int profilingLevel, + int runCount, + const std::string& csvOutputPath) +{ + std::vector<std::string> args; + args.push_back("--zst"); + args.push_back(zstFile); + args.push_back("--prf-lvl"); + args.push_back(std::to_string(profilingLevel)); + args.push_back("--run-cnt"); + args.push_back(std::to_string(runCount)); + if (!csvOutputPath.empty()) + { + args.push_back("--out-csv"); + args.push_back(csvOutputPath); + } + return args; +} diff --git a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h new file mode 100644 index 0000000..66c3c88 --- /dev/null +++ b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h @@ -0,0 +1,73 @@ +/** + * Copyright (c) Microsoft. All rights reserved. + * This code is licensed under the MIT License (MIT). + * THIS CODE IS PROVIDED *AS IS* WITHOUT WARRANTY OF + * ANY KIND, EITHER EXPRESS OR IMPLIED, INCLUDING ANY + * IMPLIED WARRANTIES OF FITNESS FOR A PARTICULAR + * PURPOSE, MERCHANTABILITY, OR NON-INFRINGEMENT. + */ + +// Shared header for the Zstd GPU CI tests. Defines the runtime configuration, +// demo process result type, and declarations for the demo runner and file +// discovery helpers. Both main.cpp and zstdgpu_ci_tests.cpp include this. + +#pragma once + +#include <string> +#include <vector> + +// Test configuration — parsed from CLI in main(), read by tests. + +struct TestConfig +{ + std::string contentPath; // Directory containing .zst test files + std::string demoPath; // Full path to zstdgpu_demo.exe + std::string logDir; // Directory for logs, CSVs, and GTest XML output + std::string logFile; // Consolidated text log file path (--log-file) + int runCount = 40; // Number of iterations for performance tests + int timeoutSeconds = 0; // Max seconds before killing a demo process (0 = no timeout) +}; + +// Singleton access — SetTestConfig called once from main(), GetTestConfig +// called from test helpers. +// Spec implies a global but doesn't show accessor pattern. +// Needed for GTest's TEST_P bodies to access config without passing it through parameters. +const TestConfig& GetTestConfig(); +void SetTestConfig(TestConfig config); + +// Demo runner — spawns zstdgpu_demo.exe and captures output. + +// Captures the outcome of a single demo process invocation. +struct DemoResult +{ + int exitCode = -1; // Process exit code (0 = success) + std::string stdOut; // Captured stdout + stderr + std::string launchError; // Error message if the process failed to launch + std::string commandLine; // The exact command line that was executed + bool timedOut = false; // True if the process was killed due to timeout +}; + +// Spawns zstdgpu_demo.exe with the given arguments, captures output, and +// returns the result. timeoutSeconds=0 means no timeout. +DemoResult RunDemo( + const std::string& demoPath, + const std::vector<std::string>& args, + int timeoutSeconds = 0); + +// Convenience: builds the full argument list for a correctness scenario. +// Spec inlines the args in each test. Extracting them avoids repeating --zst, --run-cnt 1, etc +std::vector<std::string> BuildCorrectnessArgs( + const std::string& zstFile, + const std::vector<std::string>& scenarioFlags); + +// Convenience: builds the full argument list for a performance scenario. +std::vector<std::string> BuildPerformanceArgs( + const std::string& zstFile, + int profilingLevel, + int runCount, + const std::string& csvOutputPath); + +// File discovery — scans directories for .zst test content. + +// Recursively scans a directory for *.zst files. Returns sorted full paths. +std::vector<std::string> DiscoverZstFiles(const std::string& contentPath); diff --git a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.vcxproj b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.vcxproj new file mode 100644 index 0000000..f756ce4 --- /dev/null +++ b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.vcxproj @@ -0,0 +1,239 @@ +<?xml version="1.0" encoding="utf-8"?> +<Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> + <ItemGroup Label="ProjectConfigurations"> + <ProjectConfiguration Include="Debug|ARM64"> + <Configuration>Debug</Configuration> + <Platform>ARM64</Platform> + </ProjectConfiguration> + <ProjectConfiguration Include="Debug|Win32"> + <Configuration>Debug</Configuration> + <Platform>Win32</Platform> + </ProjectConfiguration> + <ProjectConfiguration Include="Debug|x64"> + <Configuration>Debug</Configuration> + <Platform>x64</Platform> + </ProjectConfiguration> + <ProjectConfiguration Include="Release|ARM64"> + <Configuration>Release</Configuration> + <Platform>ARM64</Platform> + </ProjectConfiguration> + <ProjectConfiguration Include="Release|Win32"> + <Configuration>Release</Configuration> + <Platform>Win32</Platform> + </ProjectConfiguration> + <ProjectConfiguration Include="Release|x64"> + <Configuration>Release</Configuration> + <Platform>x64</Platform> + </ProjectConfiguration> + </ItemGroup> + <PropertyGroup Label="Globals"> + <VCProjectVersion>16.0</VCProjectVersion> + <Keyword>Win32Proj</Keyword> + <ProjectGuid>{A1B2C3D4-E5F6-7890-ABCD-EF1234567890}</ProjectGuid> + <RootNamespace>zstdgpu_ci_tests</RootNamespace> + <WindowsTargetPlatformVersion>10.0</WindowsTargetPlatformVersion> + </PropertyGroup> + <Import Project="$(VCTargetsPath)\Microsoft.Cpp.Default.props" /> + <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'" Label="Configuration"> + <ConfigurationType>Application</ConfigurationType> + <UseDebugLibraries>true</UseDebugLibraries> + <PlatformToolset>v143</PlatformToolset> + <CharacterSet>Unicode</CharacterSet> + </PropertyGroup> + <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'" Label="Configuration"> + <ConfigurationType>Application</ConfigurationType> + <UseDebugLibraries>true</UseDebugLibraries> + <PlatformToolset>v143</PlatformToolset> + <CharacterSet>Unicode</CharacterSet> + </PropertyGroup> + <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|ARM64'" Label="Configuration"> + <ConfigurationType>Application</ConfigurationType> + <UseDebugLibraries>true</UseDebugLibraries> + <PlatformToolset>v143</PlatformToolset> + <CharacterSet>Unicode</CharacterSet> + </PropertyGroup> + <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'" Label="Configuration"> + <ConfigurationType>Application</ConfigurationType> + <UseDebugLibraries>false</UseDebugLibraries> + <PlatformToolset>v143</PlatformToolset> + <WholeProgramOptimization>true</WholeProgramOptimization> + <CharacterSet>Unicode</CharacterSet> + </PropertyGroup> + <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|Win32'" Label="Configuration"> + <ConfigurationType>Application</ConfigurationType> + <UseDebugLibraries>false</UseDebugLibraries> + <PlatformToolset>v143</PlatformToolset> + <WholeProgramOptimization>true</WholeProgramOptimization> + <CharacterSet>Unicode</CharacterSet> + </PropertyGroup> + <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|ARM64'" Label="Configuration"> + <ConfigurationType>Application</ConfigurationType> + <UseDebugLibraries>false</UseDebugLibraries> + <PlatformToolset>v143</PlatformToolset> + <WholeProgramOptimization>true</WholeProgramOptimization> + <CharacterSet>Unicode</CharacterSet> + </PropertyGroup> + <Import Project="$(VCTargetsPath)\Microsoft.Cpp.props" /> + <ImportGroup Label="ExtensionSettings"> + </ImportGroup> + <ImportGroup Label="Shared"> + </ImportGroup> + <ImportGroup Label="PropertySheets" Condition="'$(Configuration)|$(Platform)'=='Debug|x64'"> + <Import Project="$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props" Condition="exists('$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props')" Label="LocalAppDataPlatform" /> + </ImportGroup> + <ImportGroup Label="PropertySheets" Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'"> + <Import Project="$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props" Condition="exists('$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props')" Label="LocalAppDataPlatform" /> + </ImportGroup> + <ImportGroup Label="PropertySheets" Condition="'$(Configuration)|$(Platform)'=='Debug|ARM64'"> + <Import Project="$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props" Condition="exists('$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props')" Label="LocalAppDataPlatform" /> + </ImportGroup> + <ImportGroup Label="PropertySheets" Condition="'$(Configuration)|$(Platform)'=='Release|x64'"> + <Import Project="$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props" Condition="exists('$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props')" Label="LocalAppDataPlatform" /> + </ImportGroup> + <ImportGroup Label="PropertySheets" Condition="'$(Configuration)|$(Platform)'=='Release|Win32'"> + <Import Project="$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props" Condition="exists('$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props')" Label="LocalAppDataPlatform" /> + </ImportGroup> + <ImportGroup Label="PropertySheets" Condition="'$(Configuration)|$(Platform)'=='Release|ARM64'"> + <Import Project="$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props" Condition="exists('$(UserRootDir)\Microsoft.Cpp.$(Platform).user.props')" Label="LocalAppDataPlatform" /> + </ImportGroup> + <PropertyGroup Label="UserMacros" /> + <!-- Output to same directory structure as other projects --> + <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'"> + <OutDir>$(SolutionDir)$(Platform)\$(Configuration)\</OutDir> + <IntDir>$(Platform)\$(Configuration)\</IntDir> + </PropertyGroup> + <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'"> + <OutDir>$(SolutionDir)$(Platform)\$(Configuration)\</OutDir> + <IntDir>$(Platform)\$(Configuration)\</IntDir> + </PropertyGroup> + <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|ARM64'"> + <OutDir>$(SolutionDir)$(Platform)\$(Configuration)\</OutDir> + <IntDir>$(Platform)\$(Configuration)\</IntDir> + </PropertyGroup> + <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'"> + <OutDir>$(SolutionDir)$(Platform)\$(Configuration)\</OutDir> + <IntDir>$(Platform)\$(Configuration)\</IntDir> + </PropertyGroup> + <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|Win32'"> + <OutDir>$(SolutionDir)$(Platform)\$(Configuration)\</OutDir> + <IntDir>$(Platform)\$(Configuration)\</IntDir> + </PropertyGroup> + <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|ARM64'"> + <OutDir>$(SolutionDir)$(Platform)\$(Configuration)\</OutDir> + <IntDir>$(Platform)\$(Configuration)\</IntDir> + </PropertyGroup> + <!-- Compiler settings --> + <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'"> + <ClCompile> + <WarningLevel>Level3</WarningLevel> + <SDLCheck>true</SDLCheck> + <PreprocessorDefinitions>_DEBUG;_CONSOLE;WIN32_LEAN_AND_MEAN;NOMINMAX;%(PreprocessorDefinitions)</PreprocessorDefinitions> + <ConformanceMode>true</ConformanceMode> + <LanguageStandard>stdcpp17</LanguageStandard> + <AdditionalIncludeDirectories>..\ThirdParty\googletest\googletest\include</AdditionalIncludeDirectories> + </ClCompile> + <Link> + <SubSystem>Console</SubSystem> + <GenerateDebugInformation>true</GenerateDebugInformation> + </Link> + </ItemDefinitionGroup> + <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'"> + <ClCompile> + <WarningLevel>Level3</WarningLevel> + <SDLCheck>true</SDLCheck> + <PreprocessorDefinitions>WIN32;_DEBUG;_CONSOLE;WIN32_LEAN_AND_MEAN;NOMINMAX;%(PreprocessorDefinitions)</PreprocessorDefinitions> + <ConformanceMode>true</ConformanceMode> + <LanguageStandard>stdcpp17</LanguageStandard> + <AdditionalIncludeDirectories>..\ThirdParty\googletest\googletest\include</AdditionalIncludeDirectories> + </ClCompile> + <Link> + <SubSystem>Console</SubSystem> + <GenerateDebugInformation>true</GenerateDebugInformation> + </Link> + </ItemDefinitionGroup> + <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|ARM64'"> + <ClCompile> + <WarningLevel>Level3</WarningLevel> + <SDLCheck>true</SDLCheck> + <PreprocessorDefinitions>_DEBUG;_CONSOLE;WIN32_LEAN_AND_MEAN;NOMINMAX;%(PreprocessorDefinitions)</PreprocessorDefinitions> + <ConformanceMode>true</ConformanceMode> + <LanguageStandard>stdcpp17</LanguageStandard> + <AdditionalIncludeDirectories>..\ThirdParty\googletest\googletest\include</AdditionalIncludeDirectories> + </ClCompile> + <Link> + <SubSystem>Console</SubSystem> + <GenerateDebugInformation>true</GenerateDebugInformation> + </Link> + </ItemDefinitionGroup> + <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'"> + <ClCompile> + <WarningLevel>Level3</WarningLevel> + <FunctionLevelLinking>true</FunctionLevelLinking> + <IntrinsicFunctions>true</IntrinsicFunctions> + <SDLCheck>true</SDLCheck> + <PreprocessorDefinitions>NDEBUG;_CONSOLE;WIN32_LEAN_AND_MEAN;NOMINMAX;%(PreprocessorDefinitions)</PreprocessorDefinitions> + <ConformanceMode>true</ConformanceMode> + <LanguageStandard>stdcpp17</LanguageStandard> + <AdditionalIncludeDirectories>..\ThirdParty\googletest\googletest\include</AdditionalIncludeDirectories> + </ClCompile> + <Link> + <SubSystem>Console</SubSystem> + <EnableCOMDATFolding>true</EnableCOMDATFolding> + <OptimizeReferences>true</OptimizeReferences> + <GenerateDebugInformation>true</GenerateDebugInformation> + </Link> + </ItemDefinitionGroup> + <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|Win32'"> + <ClCompile> + <WarningLevel>Level3</WarningLevel> + <FunctionLevelLinking>true</FunctionLevelLinking> + <IntrinsicFunctions>true</IntrinsicFunctions> + <SDLCheck>true</SDLCheck> + <PreprocessorDefinitions>WIN32;NDEBUG;_CONSOLE;WIN32_LEAN_AND_MEAN;NOMINMAX;%(PreprocessorDefinitions)</PreprocessorDefinitions> + <ConformanceMode>true</ConformanceMode> + <LanguageStandard>stdcpp17</LanguageStandard> + <AdditionalIncludeDirectories>..\ThirdParty\googletest\googletest\include</AdditionalIncludeDirectories> + </ClCompile> + <Link> + <SubSystem>Console</SubSystem> + <EnableCOMDATFolding>true</EnableCOMDATFolding> + <OptimizeReferences>true</OptimizeReferences> + <GenerateDebugInformation>true</GenerateDebugInformation> + </Link> + </ItemDefinitionGroup> + <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|ARM64'"> + <ClCompile> + <WarningLevel>Level3</WarningLevel> + <FunctionLevelLinking>true</FunctionLevelLinking> + <IntrinsicFunctions>true</IntrinsicFunctions> + <SDLCheck>true</SDLCheck> + <PreprocessorDefinitions>NDEBUG;_CONSOLE;WIN32_LEAN_AND_MEAN;NOMINMAX;%(PreprocessorDefinitions)</PreprocessorDefinitions> + <ConformanceMode>true</ConformanceMode> + <LanguageStandard>stdcpp17</LanguageStandard> + <AdditionalIncludeDirectories>..\ThirdParty\googletest\googletest\include</AdditionalIncludeDirectories> + </ClCompile> + <Link> + <SubSystem>Console</SubSystem> + <EnableCOMDATFolding>true</EnableCOMDATFolding> + <OptimizeReferences>true</OptimizeReferences> + <GenerateDebugInformation>true</GenerateDebugInformation> + </Link> + </ItemDefinitionGroup> + <!-- Source files --> + <ItemGroup> + <ClCompile Include="main.cpp" /> + <ClCompile Include="zstdgpu_ci_tests.cpp" /> + </ItemGroup> + <ItemGroup> + <ClInclude Include="zstdgpu_ci_tests.h" /> + </ItemGroup> + <!-- Project references: only googletest --> + <ItemGroup> + <ProjectReference Include="..\ThirdParty\googletest_static.vcxproj"> + <Project>{49811f10-3d14-403e-859d-40dfcbb35c7b}</Project> + </ProjectReference> + </ItemGroup> + <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" /> + <ImportGroup Label="ExtensionTargets"> + </ImportGroup> +</Project> From bcb5206b9e21c4f335d512f0cfd91e2792b3c198 Mon Sep 17 00:00:00 2001 From: Rohan Borkar <rohanborkar@microsoft.com> Date: Mon, 6 Jul 2026 12:28:33 -0700 Subject: [PATCH 2/8] zstdgpu_ci_tests: content-path validation + subdirectory-aware test names 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). --- zstd/zstdgpu_ci_tests/main.cpp | 30 ++++++++++++++++++ zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp | 36 ++++++++++++++++++++-- 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/zstd/zstdgpu_ci_tests/main.cpp b/zstd/zstdgpu_ci_tests/main.cpp index 3a52744..33bb333 100644 --- a/zstd/zstdgpu_ci_tests/main.cpp +++ b/zstd/zstdgpu_ci_tests/main.cpp @@ -132,6 +132,36 @@ int main(int argc, char** argv) std::cerr << "Warning: --demo-path not set. Tests will skip.\n"; } + if (config.contentPath.empty()) + { + std::cerr << "Warning: --content-path not set. Zero tests will be discovered " + "(gtest will print 'This test program does NOT link in any test case').\n"; + } + else if (!std::filesystem::exists(config.contentPath)) + { + std::cerr << "Warning: --content-path '" << config.contentPath + << "' does not exist. Zero tests will be discovered.\n"; + } + else if (!std::filesystem::is_directory(config.contentPath)) + { + std::cerr << "Warning: --content-path '" << config.contentPath + << "' is not a directory. Zero tests will be discovered.\n"; + } + else + { + const size_t fileCount = DiscoverZstFiles(config.contentPath).size(); + if (fileCount == 0) + { + std::cerr << "Warning: --content-path '" << config.contentPath + << "' contains no .zst files. Zero tests will be discovered.\n"; + } + else + { + std::cout << "Discovered " << fileCount << " .zst file(s) at '" + << config.contentPath << "'.\n"; + } + } + // Default log dir to current directory. if (config.logDir.empty()) { diff --git a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp index 7d60c86..e836463 100644 --- a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp +++ b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp @@ -34,6 +34,8 @@ // configured timeout, it is terminated. This avoids any D3D12/GPU dependency // in the test binary itself — all GPU work happens inside the demo process. +// NOTES: IF THE PATH IS EMPTY, FAIL THE TEST + #include "zstdgpu_ci_tests.h" #include <gtest/gtest.h> #include <array> @@ -57,10 +59,40 @@ static std::vector<std::string> GetTestFiles() } // Converts a full file path to a valid GTest parameter name. -// GTest names must be alphanumeric + underscore, no leading digits. +// GTest names must be alphanumeric + underscore, no leading digits, AND UNIQUE +// across the full INSTANTIATE_TEST_SUITE_P set. Using just the filename stem +// collides when the same leaf name appears across different subdirectories +// (e.g. firefly_albedo.DDS.zst exists under BC1/, BC1mip0/, block4K_*, etc.), +// causing a fatal gtest assertion at startup. Use the path relative to +// --content-path so different folders produce different names. static std::string SanitizeTestName(const testing::TestParamInfo<std::string>& info) { - std::string name = std::filesystem::path(info.param).stem().string(); + const auto& config = GetTestConfig(); + std::filesystem::path full(info.param); + std::filesystem::path rel; + if (!config.contentPath.empty()) + { + std::error_code ec; + rel = std::filesystem::relative(full, config.contentPath, ec); + if (ec || rel.empty() || rel.string().rfind("..", 0) == 0) + { + rel = full.filename(); // fallback: out-of-tree, just use leaf + } + } + else + { + rel = full.filename(); + } + + // Drop the trailing .zst extension for readability; everything else stays. + std::string name = rel.string(); + const std::string ext = ".zst"; + if (name.size() >= ext.size() && + name.compare(name.size() - ext.size(), ext.size(), ext) == 0) + { + name.resize(name.size() - ext.size()); + } + std::string result; result.reserve(name.size()); for (char c : name) From ab23d5ca6b59ca928f3150c7e4eee9157b26a1de Mon Sep 17 00:00:00 2001 From: Rohan Borkar <rohanborkar@microsoft.com> Date: Thu, 25 Jun 2026 07:02:13 -0700 Subject: [PATCH 3/8] histogram script handles inf values --- zstd/scripts/generate_histogram.py | 68 +++++++++++++++++++++++++----- 1 file changed, 58 insertions(+), 10 deletions(-) diff --git a/zstd/scripts/generate_histogram.py b/zstd/scripts/generate_histogram.py index 59a1295..b335b0a 100644 --- a/zstd/scripts/generate_histogram.py +++ b/zstd/scripts/generate_histogram.py @@ -5,15 +5,21 @@ Usage: python generate_histogram.py --input <csv_file> --output <png_file> [--title <title>] -Supports two CSV formats based on profiling level: - - prf-lvl 0 (OverallThroughput): column "Throughput_GBs" - - prf-lvl 2 (PerStageTiming): column "Microseconds" +Consumes the wide-format CSV emitted by zstdgpu_demo's --out-csv flag: + RunIdx, Stage 0 (us), Stage 0 :: <scope> (us), ..., Readback 0 (us), + Stage 1 (us), ..., Stage 2 (us), Bandwidth (GB/s) + +Plots a histogram of the 'Bandwidth (GB/s)' column. Per-stage timing columns +are preserved in the CSV but not plotted here (the histogram is the summary +view; users wanting per-stage detail can read the CSV directly). """ import argparse import csv +import math import sys + def try_import_matplotlib(): try: import matplotlib @@ -28,11 +34,18 @@ def try_import_matplotlib(): return None +# Match Pavel's --out-csv column header verbatim ("Bandwidth (GB/s)"). +# Kept case-insensitive and whitespace-tolerant in case the schema spelling +# drifts upstream — the eyeballed match is "bandwidth". +def _is_bandwidth_column(col_name: str) -> bool: + return col_name is not None and "bandwidth" in col_name.strip().lower() + + def main(): parser = argparse.ArgumentParser(description="Generate histogram from zstdgpu perf CSV") parser.add_argument("--input", required=True, help="Path to input CSV file") parser.add_argument("--output", required=True, help="Path to output PNG file") - parser.add_argument("--title", default="Throughput", help="Chart title") + parser.add_argument("--title", default="Bandwidth", help="Chart title") args = parser.parse_args() plt = try_import_matplotlib() @@ -40,21 +53,56 @@ def main(): return 1 data = [] + skipped_non_finite = 0 + bandwidth_col = None with open(args.input, newline="") as f: reader = csv.DictReader(f) + if reader.fieldnames is None: + print(f"CSV has no header row: {args.input}", file=sys.stderr) + return 1 + # Pick the bandwidth column by name (resilient to header drift). + for col in reader.fieldnames: + if _is_bandwidth_column(col): + bandwidth_col = col + break + if bandwidth_col is None: + print( + f"No Bandwidth column found in {args.input} " + f"(headers: {reader.fieldnames})", + file=sys.stderr, + ) + return 1 + for row in reader: - if "Throughput_GBs" in row: - data.append(float(row["Throughput_GBs"])) - elif "Microseconds" in row: - data.append(float(row["Microseconds"])) + raw = row.get(bandwidth_col, "") + if raw is None or raw == "": + continue + try: + val = float(raw) + except ValueError: + # Pavel may write empty strings for skipped iterations; ignore. + continue + if math.isfinite(val): + data.append(val) + else: + skipped_non_finite += 1 + + if skipped_non_finite > 0: + print( + f"WARNING: Skipped {skipped_non_finite} non-finite (Inf/NaN) value(s) from {args.input}", + file=sys.stderr, + ) if not data: - print(f"No Throughput_GBs or Microseconds data found in {args.input}", file=sys.stderr) + print( + f"No finite Bandwidth (GB/s) data found in {args.input}", + file=sys.stderr, + ) return 1 plt.figure() plt.hist(data, bins=20) - plt.xlabel("Throughput (GB/s)" if "throughput" in args.input.lower() else "Time (us)") + plt.xlabel("Bandwidth (GB/s)") plt.ylabel("Count") plt.title(args.title) plt.savefig(args.output) From cc4ac18900569dd5d266d42a9194f645b0a7cb2f Mon Sep 17 00:00:00 2001 From: Rohan Borkar <rohanborkar@microsoft.com> Date: Wed, 1 Jul 2026 11:44:27 -0700 Subject: [PATCH 4/8] zstdgpu_ci_tests: address PR #108 review feedback (@chmann-micro) 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. --- zstd/zstd.sln | 26 ++--- zstd/zstdgpu_ci_tests/main.cpp | 123 ++++++++++++++------- zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp | 60 +++++----- zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h | 25 +++-- 4 files changed, 144 insertions(+), 90 deletions(-) diff --git a/zstd/zstd.sln b/zstd/zstd.sln index 8c30eb9..e0eb457 100644 --- a/zstd/zstd.sln +++ b/zstd/zstd.sln @@ -11,7 +11,7 @@ Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "zstdgpu_tests", "zstdgpu_te EndProject Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "googletest_static", "ThirdParty\googletest_static.vcxproj", "{49811F10-3D14-403E-859D-40DFCBB35C7B}" EndProject -Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "zstdgpu_ci_tests", "zstdgpu_ci_tests\zstdgpu_ci_tests.vcxproj", "{A1B2C3D4-E5F6-7890-ABCD-EF1234567890}" +Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "zstdgpu_ci_tests", "zstdgpu_ci_tests\zstdgpu_ci_tests.vcxproj", "{17F60A53-4A7F-4107-AF0A-914497018D67}" EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution @@ -71,18 +71,18 @@ Global {49811F10-3D14-403E-859D-40DFCBB35C7B}.Release|x64.Build.0 = Release|x64 {49811F10-3D14-403E-859D-40DFCBB35C7B}.Release|x86.ActiveCfg = Release|Win32 {49811F10-3D14-403E-859D-40DFCBB35C7B}.Release|x86.Build.0 = Release|Win32 - {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Debug|ARM64.ActiveCfg = Debug|ARM64 - {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Debug|ARM64.Build.0 = Debug|ARM64 - {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Debug|x64.ActiveCfg = Debug|x64 - {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Debug|x64.Build.0 = Debug|x64 - {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Debug|x86.ActiveCfg = Debug|Win32 - {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Debug|x86.Build.0 = Debug|Win32 - {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Release|ARM64.ActiveCfg = Release|ARM64 - {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Release|ARM64.Build.0 = Release|ARM64 - {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Release|x64.ActiveCfg = Release|x64 - {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Release|x64.Build.0 = Release|x64 - {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Release|x86.ActiveCfg = Release|Win32 - {A1B2C3D4-E5F6-7890-ABCD-EF1234567890}.Release|x86.Build.0 = Release|Win32 + {17F60A53-4A7F-4107-AF0A-914497018D67}.Debug|ARM64.ActiveCfg = Debug|ARM64 + {17F60A53-4A7F-4107-AF0A-914497018D67}.Debug|ARM64.Build.0 = Debug|ARM64 + {17F60A53-4A7F-4107-AF0A-914497018D67}.Debug|x64.ActiveCfg = Debug|x64 + {17F60A53-4A7F-4107-AF0A-914497018D67}.Debug|x64.Build.0 = Debug|x64 + {17F60A53-4A7F-4107-AF0A-914497018D67}.Debug|x86.ActiveCfg = Debug|Win32 + {17F60A53-4A7F-4107-AF0A-914497018D67}.Debug|x86.Build.0 = Debug|Win32 + {17F60A53-4A7F-4107-AF0A-914497018D67}.Release|ARM64.ActiveCfg = Release|ARM64 + {17F60A53-4A7F-4107-AF0A-914497018D67}.Release|ARM64.Build.0 = Release|ARM64 + {17F60A53-4A7F-4107-AF0A-914497018D67}.Release|x64.ActiveCfg = Release|x64 + {17F60A53-4A7F-4107-AF0A-914497018D67}.Release|x64.Build.0 = Release|x64 + {17F60A53-4A7F-4107-AF0A-914497018D67}.Release|x86.ActiveCfg = Release|Win32 + {17F60A53-4A7F-4107-AF0A-914497018D67}.Release|x86.Build.0 = Release|Win32 EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/zstd/zstdgpu_ci_tests/main.cpp b/zstd/zstdgpu_ci_tests/main.cpp index 33bb333..c1cc3b7 100644 --- a/zstd/zstdgpu_ci_tests/main.cpp +++ b/zstd/zstdgpu_ci_tests/main.cpp @@ -10,12 +10,15 @@ // Entry point for the Zstd GPU CI tests. This is a thin GTest wrapper // that shells out to zstdgpu_demo.exe to validate Zstd GPU decompression shaders. // -// - parses custom CLI flags (--content-path, --demo-path, etc.), resolves the -// demo executable, then hands off to GTest which runs parameterized tests defined -// in zstdgpu_ci_tests.cpp. Each test spawns the demo as a child process. +// - parses custom CLI flags (--content-path, --demo-path, etc.), validates +// them (hard failure with a non-zero exit on any misconfiguration — never +// silently skip), discovers .zst files under the content path exactly once, +// then hands off to GTest which runs parameterized tests defined in +// zstdgpu_ci_tests.cpp. Each test spawns the demo as a child process. // -// If no .zst content files are found, zero tests are instantiated and the test -// binary exits 0 (success). If the demo exe is missing, tests are skipped (not failed). +// If the content path is missing, unreadable, or contains no .zst files, or if +// the demo executable is missing, the process exits non-zero before any test +// runs. This intentionally prevents a "green" run with zero coverage. // // This file also implements the TestConfig singleton and file discovery helpers // declared in zstdgpu_ci_tests.h. @@ -27,6 +30,7 @@ #include <filesystem> #include <iostream> #include <string> +#include <system_error> // TestConfig singleton // Implementation of the singleton declared in zstdgpu_ci_tests.h. @@ -44,6 +48,12 @@ void SetTestConfig(TestConfig config) } // File discovery +// +// Uses the non-throwing overloads of recursive_directory_iterator so a single +// unreadable subdirectory anywhere in the tree does not abort the entire walk +// (which would take down the process before any test runs — see review +// feedback on the throwing overload). Per-entry errors are logged as warnings +// and the walk continues. std::vector<std::string> DiscoverZstFiles(const std::string& contentPath) { @@ -54,11 +64,40 @@ std::vector<std::string> DiscoverZstFiles(const std::string& contentPath) return files; } - for (const auto& entry : std::filesystem::recursive_directory_iterator(contentPath)) + std::error_code ec; + auto iter = std::filesystem::recursive_directory_iterator( + contentPath, + std::filesystem::directory_options::skip_permission_denied, + ec); + if (ec) { - if (entry.is_regular_file() && entry.path().extension() == ".zst") + std::cerr << "Warning: could not open '" << contentPath << "' for scanning: " + << ec.message() << "\n"; + return files; + } + + auto endIter = std::filesystem::recursive_directory_iterator(); + while (iter != endIter) + { + std::error_code entryEc; + if (iter->is_regular_file(entryEc) && !entryEc) + { + if (iter->path().extension() == ".zst") + { + files.push_back(iter->path().string()); + } + } + else if (entryEc) { - files.push_back(entry.path().string()); + std::cerr << "Warning: skipping '" << iter->path().string() << "': " + << entryEc.message() << "\n"; + } + iter.increment(ec); + if (ec) + { + std::cerr << "Warning: recursive walk aborted early after '" + << iter->path().string() << "': " << ec.message() << "\n"; + break; } } @@ -75,8 +114,8 @@ static void PrintUsage(const char* exe) std::cout << "Usage: " << exe << " [gtest_options] [options]\n" << "\n" << "Options:\n" - << " --content-path <dir> Directory containing .zst test files\n" - << " --demo-path <path> Path to zstdgpu_demo.exe\n" + << " --content-path <dir> Directory containing .zst test files (required)\n" + << " --demo-path <path> Path to zstdgpu_demo.exe (required)\n" << " --log-dir <dir> Directory for logs and CSV output\n" << " --log-file <path> Consolidated text log file\n" << " --run-count <N> Perf test iteration count (default: 40)\n" @@ -84,6 +123,15 @@ static void PrintUsage(const char* exe) << std::endl; } +// Prints an error prefixed with "Error:" to stderr and returns the exit code so +// main() can 'return Fail(...)' in a single expression. Keeps validation blocks +// short and consistent. +static int Fail(const std::string& msg) +{ + std::cerr << "Error: " << msg << std::endl; + return 1; +} + int main(int argc, char** argv) { // Parse custom flags before handing off to GTest. GTest's InitGoogleTest() @@ -127,40 +175,33 @@ int main(int argc, char** argv) } } + // Fail-loud validation. Any of these misconfigurations means the run cannot + // produce meaningful test coverage, so we exit non-zero before any test + // instantiates. Silent skipping (previously done via warnings + GTEST_SKIP) + // would let broken pipelines pass green with zero coverage. if (config.demoPath.empty()) - { - std::cerr << "Warning: --demo-path not set. Tests will skip.\n"; - } + return Fail("--demo-path is required."); + if (!std::filesystem::exists(config.demoPath)) + return Fail("--demo-path '" + config.demoPath + "' does not exist."); + if (!std::filesystem::is_regular_file(config.demoPath)) + return Fail("--demo-path '" + config.demoPath + "' is not a regular file."); if (config.contentPath.empty()) - { - std::cerr << "Warning: --content-path not set. Zero tests will be discovered " - "(gtest will print 'This test program does NOT link in any test case').\n"; - } - else if (!std::filesystem::exists(config.contentPath)) - { - std::cerr << "Warning: --content-path '" << config.contentPath - << "' does not exist. Zero tests will be discovered.\n"; - } - else if (!std::filesystem::is_directory(config.contentPath)) - { - std::cerr << "Warning: --content-path '" << config.contentPath - << "' is not a directory. Zero tests will be discovered.\n"; - } - else - { - const size_t fileCount = DiscoverZstFiles(config.contentPath).size(); - if (fileCount == 0) - { - std::cerr << "Warning: --content-path '" << config.contentPath - << "' contains no .zst files. Zero tests will be discovered.\n"; - } - else - { - std::cout << "Discovered " << fileCount << " .zst file(s) at '" - << config.contentPath << "'.\n"; - } - } + return Fail("--content-path is required."); + if (!std::filesystem::exists(config.contentPath)) + return Fail("--content-path '" + config.contentPath + "' does not exist."); + if (!std::filesystem::is_directory(config.contentPath)) + return Fail("--content-path '" + config.contentPath + "' is not a directory."); + + // Discover exactly once. Cached on TestConfig for GetTestFiles() to reuse + // when instantiating the parameterized fixture — avoids walking the tree + // twice. + config.discoveredFiles = DiscoverZstFiles(config.contentPath); + if (config.discoveredFiles.empty()) + return Fail("--content-path '" + config.contentPath + "' contains no .zst files."); + + std::cout << "Discovered " << config.discoveredFiles.size() << " .zst file(s) at '" + << config.contentPath << "'.\n"; // Default log dir to current directory. if (config.logDir.empty()) diff --git a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp index e836463..d6b7878 100644 --- a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp +++ b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp @@ -25,9 +25,14 @@ // Performance tests use EXPECT (not ASSERT) — they fail on infrastructure // errors but do not check performance values against thresholds. // -// If no .zst files are found, GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST -// prevents GTest from reporting an error — zero tests run, exit code 0. +// Correctness tests use ASSERT, performance tests use EXPECT. See per-test docs. // +// main() validates that at least one .zst file is present before we reach +// this point, so INSTANTIATE_TEST_SUITE_P always has at least one parameter. +// A previous version used GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST to +// allow zero .zst files to pass green — that's now a hard failure at startup +// (see main.cpp) so this macro is no longer needed. + // The demo runner at the bottom of this file spawns zstdgpu_demo.exe as a child // process using Win32 CreateProcess with anonymous pipes. A background thread // drains stdout to avoid pipe-buffer deadlocks. If the process exceeds the @@ -51,11 +56,13 @@ // Helpers // Returns the list of .zst files to parameterize over. -// GTest evaluates this lazily when the test suite is instantiated (after main has parsed CLI args and set TestConfig), so contentPath is available here. +// GTest evaluates this lazily when the test suite is instantiated (after main +// has parsed CLI args, validated inputs, and cached the discovered file list +// in TestConfig). We just return the cached list here — the actual filesystem +// walk happened once, up front, in main(). static std::vector<std::string> GetTestFiles() { - const auto& config = GetTestConfig(); - return DiscoverZstFiles(config.contentPath); + return GetTestConfig().discoveredFiles; } // Converts a full file path to a valid GTest parameter name. @@ -124,23 +131,22 @@ static void WriteToLogFile(const std::string& zstFile, const DemoResult& result) // Test runners // Run a correctness scenario. Spawns zstdgpu_demo.exe with the given .zst file and scenario flags, then asserts exit code == 0. -// Failures include the full command line and demo stdout for diagnostic output. +// main() has already validated the demo path exists, so we don't re-check here. +// stdout is printed via the unconditional [DEMO OUT] block below and does NOT +// appear a second time in the ASSERT_EQ failure message — that would duplicate +// the same text in the log. static void RunCorrectnessTest(const std::string& zstFile, const std::vector<std::string>& scenarioFlags) { const auto& config = GetTestConfig(); - if (config.demoPath.empty()) - { - GTEST_SKIP() << "zstdgpu_demo.exe not found. Set --demo-path."; - } - auto args = BuildCorrectnessArgs(zstFile, scenarioFlags); auto result = RunDemo(config.demoPath, args, config.timeoutSeconds); // Write to log file before assertions so logs are captured even if an ASSERT aborts early. WriteToLogFile(zstFile, result); - // Log the output regardless of pass/fail. + // Log the output regardless of pass/fail. This IS the demo stdout capture — + // the assertion messages below intentionally do NOT reprint result.stdOut. std::cout << "[DEMO CMD] " << result.commandLine << "\n"; if (!result.stdOut.empty()) { @@ -157,21 +163,16 @@ static void RunCorrectnessTest(const std::string& zstFile, const std::vector<std ASSERT_EQ(result.exitCode, 0) << "Demo process returned non-zero exit code: " << result.exitCode << "\n" - << "Command: " << result.commandLine << "\n" - << "Output:\n" - << result.stdOut; + << "Command: " << result.commandLine + << " (stdout already printed above as [DEMO OUT])"; } // Run a performance scenario. Spawns zstdgpu_demo.exe with profiling flags and requests CSV output. Uses EXPECT (not ASSERT) to verify the demo executed successfully and produced CSV output. +// main() has already validated the demo path exists. static void RunPerformanceTest(const std::string& zstFile, int profilingLevel) { const auto& config = GetTestConfig(); - if (config.demoPath.empty()) - { - GTEST_SKIP() << "zstdgpu_demo.exe not found. Set --demo-path."; - } - // Build CSV output path matching spec convention: // prf-lvl 0 → results/throughput_<stem>.csv // prf-lvl 2 → results/stages_<stem>.csv @@ -190,6 +191,8 @@ static void RunPerformanceTest(const std::string& zstFile, int profilingLevel) // Write to log file before assertions so logs are captured even if a check fails. WriteToLogFile(zstFile, result); + // Log the output regardless of pass/fail. This IS the demo stdout capture — + // the assertion messages below intentionally do NOT reprint result.stdOut. std::cout << "[DEMO CMD] " << result.commandLine << "\n"; if (!result.stdOut.empty()) { @@ -206,9 +209,8 @@ static void RunPerformanceTest(const std::string& zstFile, int profilingLevel) EXPECT_EQ(result.exitCode, 0) << "Demo process returned non-zero exit code: " << result.exitCode << "\n" - << "Command: " << result.commandLine << "\n" - << "Output:\n" - << result.stdOut; + << "Command: " << result.commandLine + << " (stdout already printed above as [DEMO OUT])"; EXPECT_TRUE(std::filesystem::exists(csvPath)) << "CSV not created: " << csvPath; @@ -227,8 +229,6 @@ class ZstdGpuDemoTests : public ::testing::TestWithParam<std::string> { }; -GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(ZstdGpuDemoTests); - // --- Correctness tests --- TEST_P(ZstdGpuDemoTests, SimulationCheck) @@ -366,6 +366,16 @@ DemoResult RunDemo( result.timedOut = true; TerminateProcess(pi.hProcess, 1); WaitForSingleObject(pi.hProcess, 5000); + + // Cancel any pending ReadFile on hReadPipe so the reader thread exits + // and readerThread.join() below returns. Without this, TerminateProcess + // on a wedged child can leave the child's inherited write-end of the + // pipe orphaned in kernel state briefly, and the reader thread's + // blocking ReadFile never returns — hanging the entire test runner + // indefinitely. CancelIoEx targets outstanding I/O on the handle from + // any thread. Safe to call even if no I/O is pending (returns FALSE + // with ERROR_NOT_FOUND, harmless). + CancelIoEx(hReadPipe, nullptr); } DWORD exitCode = 0; diff --git a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h index 66c3c88..b5b58d9 100644 --- a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h +++ b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h @@ -20,18 +20,23 @@ struct TestConfig { - std::string contentPath; // Directory containing .zst test files - std::string demoPath; // Full path to zstdgpu_demo.exe - std::string logDir; // Directory for logs, CSVs, and GTest XML output - std::string logFile; // Consolidated text log file path (--log-file) - int runCount = 40; // Number of iterations for performance tests - int timeoutSeconds = 0; // Max seconds before killing a demo process (0 = no timeout) + std::string contentPath; // Directory containing .zst test files + std::string demoPath; // Full path to zstdgpu_demo.exe + std::string logDir; // Directory for logs, CSVs, and GTest XML output + std::string logFile; // Consolidated text log file path (--log-file) + int runCount = 40; // Number of iterations for performance tests + int timeoutSeconds = 0; // Max seconds before killing a demo process (0 = no timeout) + + // Cached list of .zst files discovered under contentPath. Populated once + // in main() after validation; consumed by GetTestFiles() at fixture + // instantiation. Avoids walking the tree twice. + std::vector<std::string> discoveredFiles; }; -// Singleton access — SetTestConfig called once from main(), GetTestConfig +// Singleton access. SetTestConfig called once from main(), GetTestConfig // called from test helpers. -// Spec implies a global but doesn't show accessor pattern. -// Needed for GTest's TEST_P bodies to access config without passing it through parameters. +// Needed for GTest's TEST_P bodies to access config without passing it through +// parameters. const TestConfig& GetTestConfig(); void SetTestConfig(TestConfig config); @@ -55,7 +60,6 @@ DemoResult RunDemo( int timeoutSeconds = 0); // Convenience: builds the full argument list for a correctness scenario. -// Spec inlines the args in each test. Extracting them avoids repeating --zst, --run-cnt 1, etc std::vector<std::string> BuildCorrectnessArgs( const std::string& zstFile, const std::vector<std::string>& scenarioFlags); @@ -68,6 +72,5 @@ std::vector<std::string> BuildPerformanceArgs( const std::string& csvOutputPath); // File discovery — scans directories for .zst test content. - // Recursively scans a directory for *.zst files. Returns sorted full paths. std::vector<std::string> DiscoverZstFiles(const std::string& contentPath); From 7a5d8f2f134bb861de16e3ea1e697c7cfc2b492e Mon Sep 17 00:00:00 2001 From: Rohan Borkar <rohanborkar@microsoft.com> Date: Wed, 1 Jul 2026 18:00:23 -0700 Subject: [PATCH 5/8] small cleanup, no change to functionality --- zstd/zstdgpu_ci_tests/main.cpp | 91 ++++++++------- zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp | 122 +++++++++++++-------- zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h | 54 ++------- 3 files changed, 137 insertions(+), 130 deletions(-) diff --git a/zstd/zstdgpu_ci_tests/main.cpp b/zstd/zstdgpu_ci_tests/main.cpp index c1cc3b7..298211d 100644 --- a/zstd/zstdgpu_ci_tests/main.cpp +++ b/zstd/zstdgpu_ci_tests/main.cpp @@ -7,21 +7,21 @@ * PURPOSE, MERCHANTABILITY, OR NON-INFRINGEMENT. */ -// Entry point for the Zstd GPU CI tests. This is a thin GTest wrapper -// that shells out to zstdgpu_demo.exe to validate Zstd GPU decompression shaders. +// Entry point for the Zstd GPU CI tests. Thin GTest wrapper that shells out +// to zstdgpu_demo.exe to validate Zstd GPU decompression shaders. // -// - parses custom CLI flags (--content-path, --demo-path, etc.), validates -// them (hard failure with a non-zero exit on any misconfiguration — never -// silently skip), discovers .zst files under the content path exactly once, -// then hands off to GTest which runs parameterized tests defined in -// zstdgpu_ci_tests.cpp. Each test spawns the demo as a child process. +// main() parses custom CLI flags (--content-path, --demo-path, etc.), +// validates them (hard failure with non-zero exit on any misconfiguration — +// never silently skip), discovers .zst files under the content path exactly +// once, then hands off to GTest which runs the parameterized suite defined +// in zstdgpu_ci_tests.cpp. Each test spawns the demo as a child process. // -// If the content path is missing, unreadable, or contains no .zst files, or if -// the demo executable is missing, the process exits non-zero before any test -// runs. This intentionally prevents a "green" run with zero coverage. +// If the content path is missing, unreadable, or contains no .zst files, or +// if the demo executable is missing, the process exits non-zero before any +// test runs. This intentionally prevents a "green" run with zero coverage. // -// This file also implements the TestConfig singleton and file discovery helpers -// declared in zstdgpu_ci_tests.h. +// This file also owns the g_testConfig storage and the file discovery +// implementation declared in zstdgpu_ci_tests.h. #include "zstdgpu_ci_tests.h" #include <gtest/gtest.h> @@ -32,20 +32,9 @@ #include <string> #include <system_error> -// TestConfig singleton -// Implementation of the singleton declared in zstdgpu_ci_tests.h. - -static TestConfig g_testConfig; - -const TestConfig& GetTestConfig() -{ - return g_testConfig; -} - -void SetTestConfig(TestConfig config) -{ - g_testConfig = std::move(config); -} +// Global config, declared extern in zstdgpu_ci_tests.h. Set once in main(), +// read from test bodies. +TestConfig g_testConfig; // File discovery // @@ -132,12 +121,13 @@ static int Fail(const std::string& msg) return 1; } -int main(int argc, char** argv) +// Parse custom flags out of argv before we hand argv to GTest. GTest's own +// InitGoogleTest() runs later and will consume its own flags (e.g. +// --gtest_filter). Returns true on success; on --help-ci, prints usage and +// sets shouldExit=true so main() can return 0 cleanly. +static bool ParseArgs(int argc, char** argv, TestConfig& config, bool& shouldExit) { - // Parse custom flags before handing off to GTest. GTest's InitGoogleTest() - // is called later and will consume its own flags (e.g. --gtest_filter). - TestConfig config; - + shouldExit = false; for (int i = 1; i < argc; ++i) { if (std::strcmp(argv[i], "--content-path") == 0 && i + 1 < argc) @@ -171,14 +161,22 @@ int main(int argc, char** argv) else if (std::strcmp(argv[i], "--help-ci") == 0) { PrintUsage(argv[0]); - return 0; + shouldExit = true; + return true; } } + return true; +} - // Fail-loud validation. Any of these misconfigurations means the run cannot - // produce meaningful test coverage, so we exit non-zero before any test - // instantiates. Silent skipping (previously done via warnings + GTEST_SKIP) - // would let broken pipelines pass green with zero coverage. +// Fail-loud validation + one-shot filesystem discovery. Any misconfiguration +// means the run cannot produce meaningful test coverage, so we return a +// non-zero exit code before any test instantiates. Silent skipping would let +// broken pipelines pass green with zero coverage. +// +// On success also creates the log directory (defaulting to cwd) so downstream +// writes don't have to check. +static int ValidateAndDiscover(TestConfig& config) +{ if (config.demoPath.empty()) return Fail("--demo-path is required."); if (!std::filesystem::exists(config.demoPath)) @@ -203,19 +201,32 @@ int main(int argc, char** argv) std::cout << "Discovered " << config.discoveredFiles.size() << " .zst file(s) at '" << config.contentPath << "'.\n"; - // Default log dir to current directory. if (config.logDir.empty()) { config.logDir = std::filesystem::current_path().string(); } - - // Ensure log directory exists. if (!std::filesystem::exists(config.logDir)) { std::filesystem::create_directories(config.logDir); } - SetTestConfig(std::move(config)); + return 0; +} + +int main(int argc, char** argv) +{ + TestConfig config; + + bool shouldExit = false; + if (!ParseArgs(argc, argv, config, shouldExit)) + return 1; + if (shouldExit) + return 0; // --help-ci printed usage and asked to exit cleanly + + if (int rc = ValidateAndDiscover(config); rc != 0) + return rc; + + g_testConfig = std::move(config); testing::InitGoogleTest(&argc, argv); testing::GTEST_FLAG(catch_exceptions) = false; diff --git a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp index d6b7878..ee61532 100644 --- a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp +++ b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp @@ -9,43 +9,22 @@ // Test definitions and demo runner for the Zstd GPU CI tests. // -// Contains a single parameterized test suite (ZstdGpuDemoTests) instantiated -// once per .zst file found in the content directory. Each file gets 6 test -// scenarios: +// Parameterized test suite (ZstdGpuDemoTests) instantiated once per .zst file +// under the content path. Each file produces 6 scenarios: // -// Correctness tests (4 scenarios per file): -// - SimulationCheck: Software GPU simulation (--sim-gpu) with CPU+GPU validation -// - D3D12DebugLayer: Hardware GPU with D3D12 debug layer (--d3d-dbg) -// - ExternalMemory: External memory mode (--ext-mem) -// - GraphicsQueue: Graphics queue instead of compute (--d3d-gfx) +// Correctness (ASSERT — hard fail): +// - SimulationCheck : --sim-gpu with CPU+GPU validation +// - D3D12DebugLayer : --d3d-dbg +// - ExternalMemory : --ext-mem +// - GraphicsQueue : --d3d-gfx // -// Performance tests (2 scenarios per file): -// - OverallThroughput: Profiling level 0 — CSV: results/throughput_<stem>.csv -// - PerStageTiming: Profiling level 2 — CSV: results/stages_<stem>.csv -// Performance tests use EXPECT (not ASSERT) — they fail on infrastructure -// errors but do not check performance values against thresholds. -// -// Correctness tests use ASSERT, performance tests use EXPECT. See per-test docs. -// -// main() validates that at least one .zst file is present before we reach -// this point, so INSTANTIATE_TEST_SUITE_P always has at least one parameter. -// A previous version used GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST to -// allow zero .zst files to pass green — that's now a hard failure at startup -// (see main.cpp) so this macro is no longer needed. - -// The demo runner at the bottom of this file spawns zstdgpu_demo.exe as a child -// process using Win32 CreateProcess with anonymous pipes. A background thread -// drains stdout to avoid pipe-buffer deadlocks. If the process exceeds the -// configured timeout, it is terminated. This avoids any D3D12/GPU dependency -// in the test binary itself — all GPU work happens inside the demo process. - -// NOTES: IF THE PATH IS EMPTY, FAIL THE TEST +// Performance (EXPECT — soft fail, also verify CSV output was written): +// - OverallThroughput: --prf-lvl 0 → results/throughput_<stem>.csv +// - PerStageTiming : --prf-lvl 2 → results/stages_<stem>.csv #include "zstdgpu_ci_tests.h" #include <gtest/gtest.h> #include <array> -#include <chrono> -#include <cstdio> #include <filesystem> #include <fstream> #include <iostream> @@ -53,6 +32,40 @@ #include <thread> #include <Windows.h> +// Internal types + forward declarations +// +// These are used only inside this translation unit; keeping them out of the +// header limits the wrapper's public surface to the two things main.cpp +// actually needs (TestConfig, DiscoverZstFiles). + +namespace +{ + // Captures the outcome of a single demo process invocation. + struct DemoResult + { + int exitCode = -1; // Process exit code (0 = success) + std::string stdOut; // Captured stdout + stderr + std::string launchError; // Error message if the process failed to launch + std::string commandLine; // The exact command line that was executed + bool timedOut = false; // True if the process was killed due to timeout + }; + + DemoResult RunDemo( + const std::string& demoPath, + const std::vector<std::string>& args, + int timeoutSeconds); + + std::vector<std::string> BuildCorrectnessArgs( + const std::string& zstFile, + const std::vector<std::string>& scenarioFlags); + + std::vector<std::string> BuildPerformanceArgs( + const std::string& zstFile, + int profilingLevel, + int runCount, + const std::string& csvOutputPath); +} + // Helpers // Returns the list of .zst files to parameterize over. @@ -62,7 +75,7 @@ // walk happened once, up front, in main(). static std::vector<std::string> GetTestFiles() { - return GetTestConfig().discoveredFiles; + return g_testConfig.discoveredFiles; } // Converts a full file path to a valid GTest parameter name. @@ -74,13 +87,12 @@ static std::vector<std::string> GetTestFiles() // --content-path so different folders produce different names. static std::string SanitizeTestName(const testing::TestParamInfo<std::string>& info) { - const auto& config = GetTestConfig(); std::filesystem::path full(info.param); std::filesystem::path rel; - if (!config.contentPath.empty()) + if (!g_testConfig.contentPath.empty()) { std::error_code ec; - rel = std::filesystem::relative(full, config.contentPath, ec); + rel = std::filesystem::relative(full, g_testConfig.contentPath, ec); if (ec || rel.empty() || rel.string().rfind("..", 0) == 0) { rel = full.filename(); // fallback: out-of-tree, just use leaf @@ -116,11 +128,16 @@ static std::string SanitizeTestName(const testing::TestParamInfo<std::string>& i // Appends per-test output to the consolidated log file (--log-file). static void WriteToLogFile(const std::string& zstFile, const DemoResult& result) { - const auto& config = GetTestConfig(); - if (config.logFile.empty()) + if (g_testConfig.logFile.empty()) return; - std::ofstream log(config.logFile, std::ios::app); + std::ofstream log(g_testConfig.logFile, std::ios::app); + if (!log) + { + std::cerr << "Warning: could not open --log-file '" + << g_testConfig.logFile << "' for append.\n"; + return; + } auto* testInfo = ::testing::UnitTest::GetInstance()->current_test_info(); log << "=== " << testInfo->test_suite_name() << "." << testInfo->name() << " ===\n"; log << "File: " << zstFile << "\n"; @@ -137,10 +154,8 @@ static void WriteToLogFile(const std::string& zstFile, const DemoResult& result) // the same text in the log. static void RunCorrectnessTest(const std::string& zstFile, const std::vector<std::string>& scenarioFlags) { - const auto& config = GetTestConfig(); - auto args = BuildCorrectnessArgs(zstFile, scenarioFlags); - auto result = RunDemo(config.demoPath, args, config.timeoutSeconds); + auto result = RunDemo(g_testConfig.demoPath, args, g_testConfig.timeoutSeconds); // Write to log file before assertions so logs are captured even if an ASSERT aborts early. WriteToLogFile(zstFile, result); @@ -154,7 +169,7 @@ static void RunCorrectnessTest(const std::string& zstFile, const std::vector<std } ASSERT_FALSE(result.timedOut) - << "Demo process timed out after " << config.timeoutSeconds << " seconds.\n" + << "Demo process timed out after " << g_testConfig.timeoutSeconds << " seconds.\n" << "Command: " << result.commandLine; ASSERT_TRUE(result.launchError.empty()) @@ -171,22 +186,20 @@ static void RunCorrectnessTest(const std::string& zstFile, const std::vector<std // main() has already validated the demo path exists. static void RunPerformanceTest(const std::string& zstFile, int profilingLevel) { - const auto& config = GetTestConfig(); - // Build CSV output path matching spec convention: // prf-lvl 0 → results/throughput_<stem>.csv // prf-lvl 2 → results/stages_<stem>.csv std::string stem = std::filesystem::path(zstFile).stem().string(); std::string prefix = (profilingLevel == 0) ? "throughput" : "stages"; - std::filesystem::path resultsDir = std::filesystem::path(config.logDir) / "results"; + std::filesystem::path resultsDir = std::filesystem::path(g_testConfig.logDir) / "results"; if (!std::filesystem::exists(resultsDir)) { std::filesystem::create_directories(resultsDir); } std::string csvPath = (resultsDir / (prefix + "_" + stem + ".csv")).string(); - auto args = BuildPerformanceArgs(zstFile, profilingLevel, config.runCount, csvPath); - auto result = RunDemo(config.demoPath, args, config.timeoutSeconds); + auto args = BuildPerformanceArgs(zstFile, profilingLevel, g_testConfig.runCount, csvPath); + auto result = RunDemo(g_testConfig.demoPath, args, g_testConfig.timeoutSeconds); // Write to log file before assertions so logs are captured even if a check fails. WriteToLogFile(zstFile, result); @@ -200,7 +213,7 @@ static void RunPerformanceTest(const std::string& zstFile, int profilingLevel) } EXPECT_FALSE(result.timedOut) - << "Demo process timed out after " << config.timeoutSeconds << " seconds.\n" + << "Demo process timed out after " << g_testConfig.timeoutSeconds << " seconds.\n" << "Command: " << result.commandLine; EXPECT_TRUE(result.launchError.empty()) @@ -270,6 +283,17 @@ INSTANTIATE_TEST_SUITE_P( SanitizeTestName); // Demo runner implementation +// +// Spawns zstdgpu_demo.exe as a child process via CreateProcess with anonymous +// pipes. A background thread drains stdout to avoid pipe-buffer deadlocks. If +// the child exceeds the configured timeout, it is terminated and CancelIoEx +// releases the reader thread so the wrapper does not itself hang. +// +// All GPU / D3D12 dependencies live in the demo process; the test binary +// itself has no GPU dependency. + +namespace +{ // Builds a command line string with proper quoting for arguments containing spaces. static std::string BuildCommandLine(const std::string& exe, const std::vector<std::string>& args) @@ -433,3 +457,5 @@ std::vector<std::string> BuildPerformanceArgs( } return args; } + +} // namespace diff --git a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h index b5b58d9..aeec97c 100644 --- a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h +++ b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h @@ -7,9 +7,11 @@ * PURPOSE, MERCHANTABILITY, OR NON-INFRINGEMENT. */ -// Shared header for the Zstd GPU CI tests. Defines the runtime configuration, -// demo process result type, and declarations for the demo runner and file -// discovery helpers. Both main.cpp and zstdgpu_ci_tests.cpp include this. +// Shared header for the Zstd GPU CI tests. +// Exposes only what genuinely crosses translation-unit boundaries: +// the TestConfig struct, a global instance, and file discovery. +// The demo runner, DemoResult, and argument builders are internal +// to zstdgpu_ci_tests.cpp and stay hidden as static functions there. #pragma once @@ -33,44 +35,12 @@ struct TestConfig std::vector<std::string> discoveredFiles; }; -// Singleton access. SetTestConfig called once from main(), GetTestConfig -// called from test helpers. -// Needed for GTest's TEST_P bodies to access config without passing it through -// parameters. -const TestConfig& GetTestConfig(); -void SetTestConfig(TestConfig config); +// Global config, set once in main() before RUN_ALL_TESTS(), then read-only +// from test bodies. A plain extern global instead of a getter/setter is enough +// here — the "set once, then read" contract is enforced by the main() call +// sequence and there is no benefit to hiding the storage. +extern TestConfig g_testConfig; -// Demo runner — spawns zstdgpu_demo.exe and captures output. - -// Captures the outcome of a single demo process invocation. -struct DemoResult -{ - int exitCode = -1; // Process exit code (0 = success) - std::string stdOut; // Captured stdout + stderr - std::string launchError; // Error message if the process failed to launch - std::string commandLine; // The exact command line that was executed - bool timedOut = false; // True if the process was killed due to timeout -}; - -// Spawns zstdgpu_demo.exe with the given arguments, captures output, and -// returns the result. timeoutSeconds=0 means no timeout. -DemoResult RunDemo( - const std::string& demoPath, - const std::vector<std::string>& args, - int timeoutSeconds = 0); - -// Convenience: builds the full argument list for a correctness scenario. -std::vector<std::string> BuildCorrectnessArgs( - const std::string& zstFile, - const std::vector<std::string>& scenarioFlags); - -// Convenience: builds the full argument list for a performance scenario. -std::vector<std::string> BuildPerformanceArgs( - const std::string& zstFile, - int profilingLevel, - int runCount, - const std::string& csvOutputPath); - -// File discovery — scans directories for .zst test content. -// Recursively scans a directory for *.zst files. Returns sorted full paths. +// File discovery — scans a directory for *.zst files. Returns sorted full paths. +// Called by main() during startup. std::vector<std::string> DiscoverZstFiles(const std::string& contentPath); From a3b1cb5451c3a4a3f88fbe3709864cca2a874fb2 Mon Sep 17 00:00:00 2001 From: Rohan Borkar <rohanborkar@microsoft.com> Date: Tue, 7 Jul 2026 18:24:02 -0700 Subject: [PATCH 6/8] zstdgpu_ci_tests: gate D3D12DebugLayer's --d3d-dbg run on reference-comparison 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. --- zstd/zstdgpu_ci_tests/main.cpp | 7 + zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp | 147 ++++++++++++++++++++- zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h | 1 + 3 files changed, 154 insertions(+), 1 deletion(-) diff --git a/zstd/zstdgpu_ci_tests/main.cpp b/zstd/zstdgpu_ci_tests/main.cpp index 298211d..aa0ac45 100644 --- a/zstd/zstdgpu_ci_tests/main.cpp +++ b/zstd/zstdgpu_ci_tests/main.cpp @@ -109,6 +109,9 @@ static void PrintUsage(const char* exe) << " --log-file <path> Consolidated text log file\n" << " --run-count <N> Perf test iteration count (default: 40)\n" << " --timeout <seconds> Per-test process timeout (default: no timeout)\n" + << " --force-gbv Skip the D3D12DebugLayer two-phase gate — always run with --d3d-dbg\n" + << " (default: two-phase mode — run without --d3d-dbg first,\n" + << " only re-run with --d3d-dbg if the first run fails)\n" << std::endl; } @@ -158,6 +161,10 @@ static bool ParseArgs(int argc, char** argv, TestConfig& config, bool& shouldExi if (config.timeoutSeconds < 0) config.timeoutSeconds = 0; } + else if (std::strcmp(argv[i], "--force-gbv") == 0) + { + config.forceGbv = true; + } else if (std::strcmp(argv[i], "--help-ci") == 0) { PrintUsage(argv[0]); diff --git a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp index ee61532..a83938b 100644 --- a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp +++ b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp @@ -24,6 +24,7 @@ #include "zstdgpu_ci_tests.h" #include <gtest/gtest.h> +#include <atomic> #include <array> #include <filesystem> #include <fstream> @@ -145,6 +146,59 @@ static void WriteToLogFile(const std::string& zstFile, const DemoResult& result) log << result.stdOut << "\n"; } +// Appends the phase-2 (--d3d-dbg diagnostic) demo output to the same +// consolidated log file with a marker separator. Used only by the +// D3D12DebugLayer gated helper below when phase 1 fails and we re-run +// with --d3d-dbg for GBV diagnostic output. Keeping both phases in a +// single log entry preserves the "one log block per gtest case" contract +// so downstream log consumers don't need to know about the two-phase flow. +static void AppendPhaseTwoToLogFile(const std::string& zstFile, const DemoResult& result) +{ + if (g_testConfig.logFile.empty()) + return; + + std::ofstream log(g_testConfig.logFile, std::ios::app); + if (!log) + { + std::cerr << "Warning: could not open --log-file '" + << g_testConfig.logFile << "' for phase-2 append.\n"; + return; + } + log << "--- PHASE 2 (--d3d-dbg diagnostic re-run) ---\n"; + log << "File: " << zstFile << "\n"; + log << "Exit code: " << result.exitCode << "\n"; + log << result.stdOut << "\n"; +} + +// D3D12DebugLayer gated-fallback metrics. Incremented from +// RunCorrectnessTestWithGbvFallback below. Destructor prints a one-line +// summary at program exit so operators can see at a glance how often the +// GBV re-run fired. File-static + destructor-print keeps the reporting +// self-contained here without touching main.cpp or the header. +namespace { +struct D3D12DebugLayerFallbackCounters +{ + std::atomic<int> totalRuns{0}; + std::atomic<int> fallbackFires{0}; + + ~D3D12DebugLayerFallbackCounters() + { + const int runs = totalRuns.load(); + if (runs > 0) + { + // std::endl to force flush during static destruction — some + // stdout paths swallow buffered output when the runtime is + // tearing down. + std::cout << "\n[D3D12DebugLayer] " << fallbackFires.load() + << " of " << runs + << " test(s) re-ran with --d3d-dbg for GBV diagnostics." + << std::endl; + } + } +}; +D3D12DebugLayerFallbackCounters g_d3d12Metrics; +} // anonymous namespace + // Test runners // Run a correctness scenario. Spawns zstdgpu_demo.exe with the given .zst file and scenario flags, then asserts exit code == 0. @@ -182,6 +236,97 @@ static void RunCorrectnessTest(const std::string& zstFile, const std::vector<std << " (stdout already printed above as [DEMO OUT])"; } +// Two-phase D3D12DebugLayer runner (per DirectStorage team spec, 2026-07-07): +// GBV validation is expensive, and most tests pass reference comparison +// without needing any GBV diagnostic output. This helper skips GBV for the +// passing majority and only re-runs with --d3d-dbg when the reference +// comparison actually fails, keeping GBV output focused on genuinely +// failing tests. +// +// Phase 1: spawn demo with just [--chk-gpu] (reference comparison only). +// If phase 1 exits 0 -> test passes, we're done. GBV cost skipped. +// If phase 1 fails -> spawn demo again with [--chk-gpu, --d3d-dbg] and +// append its output to the same log block with a +// marker separator. Test verdict stays based on +// phase 1's exit code; phase 2 is diagnostic only. +// +// The --force-gbv CLI flag bypasses the gate entirely and always runs +// with --d3d-dbg (matches pre-gating behavior). Useful when someone wants +// unconditional GBV output on all tests for a stress run. +// +// A file-static counter (g_d3d12Metrics) tracks how often phase 2 fires +// and prints a one-line summary at program exit — makes it easy to +// eyeball whether the gate is doing meaningful work. +static void RunCorrectnessTestWithGbvFallback(const std::string& zstFile) +{ + // --force-gbv: skip the gate entirely; behave like the original + // RunCorrectnessTest with the full flag set. We don't touch the + // g_d3d12Metrics counters in this path so the summary line at exit + // only prints when the gate was actually active. + if (g_testConfig.forceGbv) + { + RunCorrectnessTest(zstFile, {"--chk-gpu", "--d3d-dbg"}); + return; + } + + ++g_d3d12Metrics.totalRuns; + + // Phase 1: reference comparison only (no --d3d-dbg). + auto phase1Args = BuildCorrectnessArgs(zstFile, {"--chk-gpu"}); + auto phase1 = RunDemo(g_testConfig.demoPath, phase1Args, g_testConfig.timeoutSeconds); + WriteToLogFile(zstFile, phase1); + + std::cout << "[DEMO CMD] " << phase1.commandLine << "\n"; + if (!phase1.stdOut.empty()) + { + std::cout << "[DEMO OUT] " << phase1.stdOut << "\n"; + } + + // If phase 1 passed cleanly, we're done — skip GBV, test passes. + const bool phase1Ok = !phase1.timedOut + && phase1.launchError.empty() + && phase1.exitCode == 0; + if (phase1Ok) + { + return; + } + + // Phase 2: re-run with --d3d-dbg for GBV diagnostic. Log gets the + // marker separator + phase-2 output appended to the same test's block. + ++g_d3d12Metrics.fallbackFires; + std::cout << "[D3D12DebugLayer PHASE 2] Phase 1 (reference comparison) failed. " + << "Re-running with --d3d-dbg for GBV diagnostic output.\n"; + + auto phase2Args = BuildCorrectnessArgs(zstFile, {"--chk-gpu", "--d3d-dbg"}); + auto phase2 = RunDemo(g_testConfig.demoPath, phase2Args, g_testConfig.timeoutSeconds); + AppendPhaseTwoToLogFile(zstFile, phase2); + + std::cout << "[DEMO CMD] " << phase2.commandLine << "\n"; + if (!phase2.stdOut.empty()) + { + std::cout << "[DEMO OUT] " << phase2.stdOut << "\n"; + } + + // Assert on PHASE 1 exit code — phase 2 is diagnostic only. If phase 2 + // also failed (which it usually will since it's the same input), the + // GBV output is already captured in the log for post-mortem analysis. + ASSERT_FALSE(phase1.timedOut) + << "Phase 1 (reference comparison) timed out after " + << g_testConfig.timeoutSeconds << " seconds.\n" + << "Command: " << phase1.commandLine; + + ASSERT_TRUE(phase1.launchError.empty()) + << "Failed to launch demo (phase 1): " << phase1.launchError << "\n" + << "Command: " << phase1.commandLine; + + ASSERT_EQ(phase1.exitCode, 0) + << "Phase 1 (reference comparison) returned non-zero exit code: " + << phase1.exitCode << "\n" + << "Command: " << phase1.commandLine + << "\n (phase 2 with --d3d-dbg was re-run — its GBV output is " + << "in the log above under the PHASE 2 marker)"; +} + // Run a performance scenario. Spawns zstdgpu_demo.exe with profiling flags and requests CSV output. Uses EXPECT (not ASSERT) to verify the demo executed successfully and produced CSV output. // main() has already validated the demo path exists. static void RunPerformanceTest(const std::string& zstFile, int profilingLevel) @@ -251,7 +396,7 @@ TEST_P(ZstdGpuDemoTests, SimulationCheck) TEST_P(ZstdGpuDemoTests, D3D12DebugLayer) { - RunCorrectnessTest(GetParam(), {"--chk-gpu", "--d3d-dbg"}); + RunCorrectnessTestWithGbvFallback(GetParam()); } TEST_P(ZstdGpuDemoTests, ExternalMemory) diff --git a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h index aeec97c..e03f210 100644 --- a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h +++ b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h @@ -28,6 +28,7 @@ struct TestConfig std::string logFile; // Consolidated text log file path (--log-file) int runCount = 40; // Number of iterations for performance tests int timeoutSeconds = 0; // Max seconds before killing a demo process (0 = no timeout) + bool forceGbv = false; // Skip the D3D12DebugLayer two-phase gate (always run with --d3d-dbg) // Cached list of .zst files discovered under contentPath. Populated once // in main() after validation; consumed by GetTestFiles() at fixture From d93ec9f64174ca17ef2bb3ada7a8dc9fdcc63ac6 Mon Sep 17 00:00:00 2001 From: Rohan Borkar <rohanborkar@microsoft.com> Date: Wed, 8 Jul 2026 11:54:40 -0700 Subject: [PATCH 7/8] Revert "zstdgpu_ci_tests: gate D3D12DebugLayer's --d3d-dbg run on reference-comparison failure" This reverts commit a3b1cb5451c3a4a3f88fbe3709864cca2a874fb2. --- zstd/zstdgpu_ci_tests/main.cpp | 7 - zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp | 147 +-------------------- zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h | 1 - 3 files changed, 1 insertion(+), 154 deletions(-) diff --git a/zstd/zstdgpu_ci_tests/main.cpp b/zstd/zstdgpu_ci_tests/main.cpp index aa0ac45..298211d 100644 --- a/zstd/zstdgpu_ci_tests/main.cpp +++ b/zstd/zstdgpu_ci_tests/main.cpp @@ -109,9 +109,6 @@ static void PrintUsage(const char* exe) << " --log-file <path> Consolidated text log file\n" << " --run-count <N> Perf test iteration count (default: 40)\n" << " --timeout <seconds> Per-test process timeout (default: no timeout)\n" - << " --force-gbv Skip the D3D12DebugLayer two-phase gate — always run with --d3d-dbg\n" - << " (default: two-phase mode — run without --d3d-dbg first,\n" - << " only re-run with --d3d-dbg if the first run fails)\n" << std::endl; } @@ -161,10 +158,6 @@ static bool ParseArgs(int argc, char** argv, TestConfig& config, bool& shouldExi if (config.timeoutSeconds < 0) config.timeoutSeconds = 0; } - else if (std::strcmp(argv[i], "--force-gbv") == 0) - { - config.forceGbv = true; - } else if (std::strcmp(argv[i], "--help-ci") == 0) { PrintUsage(argv[0]); diff --git a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp index a83938b..ee61532 100644 --- a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp +++ b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.cpp @@ -24,7 +24,6 @@ #include "zstdgpu_ci_tests.h" #include <gtest/gtest.h> -#include <atomic> #include <array> #include <filesystem> #include <fstream> @@ -146,59 +145,6 @@ static void WriteToLogFile(const std::string& zstFile, const DemoResult& result) log << result.stdOut << "\n"; } -// Appends the phase-2 (--d3d-dbg diagnostic) demo output to the same -// consolidated log file with a marker separator. Used only by the -// D3D12DebugLayer gated helper below when phase 1 fails and we re-run -// with --d3d-dbg for GBV diagnostic output. Keeping both phases in a -// single log entry preserves the "one log block per gtest case" contract -// so downstream log consumers don't need to know about the two-phase flow. -static void AppendPhaseTwoToLogFile(const std::string& zstFile, const DemoResult& result) -{ - if (g_testConfig.logFile.empty()) - return; - - std::ofstream log(g_testConfig.logFile, std::ios::app); - if (!log) - { - std::cerr << "Warning: could not open --log-file '" - << g_testConfig.logFile << "' for phase-2 append.\n"; - return; - } - log << "--- PHASE 2 (--d3d-dbg diagnostic re-run) ---\n"; - log << "File: " << zstFile << "\n"; - log << "Exit code: " << result.exitCode << "\n"; - log << result.stdOut << "\n"; -} - -// D3D12DebugLayer gated-fallback metrics. Incremented from -// RunCorrectnessTestWithGbvFallback below. Destructor prints a one-line -// summary at program exit so operators can see at a glance how often the -// GBV re-run fired. File-static + destructor-print keeps the reporting -// self-contained here without touching main.cpp or the header. -namespace { -struct D3D12DebugLayerFallbackCounters -{ - std::atomic<int> totalRuns{0}; - std::atomic<int> fallbackFires{0}; - - ~D3D12DebugLayerFallbackCounters() - { - const int runs = totalRuns.load(); - if (runs > 0) - { - // std::endl to force flush during static destruction — some - // stdout paths swallow buffered output when the runtime is - // tearing down. - std::cout << "\n[D3D12DebugLayer] " << fallbackFires.load() - << " of " << runs - << " test(s) re-ran with --d3d-dbg for GBV diagnostics." - << std::endl; - } - } -}; -D3D12DebugLayerFallbackCounters g_d3d12Metrics; -} // anonymous namespace - // Test runners // Run a correctness scenario. Spawns zstdgpu_demo.exe with the given .zst file and scenario flags, then asserts exit code == 0. @@ -236,97 +182,6 @@ static void RunCorrectnessTest(const std::string& zstFile, const std::vector<std << " (stdout already printed above as [DEMO OUT])"; } -// Two-phase D3D12DebugLayer runner (per DirectStorage team spec, 2026-07-07): -// GBV validation is expensive, and most tests pass reference comparison -// without needing any GBV diagnostic output. This helper skips GBV for the -// passing majority and only re-runs with --d3d-dbg when the reference -// comparison actually fails, keeping GBV output focused on genuinely -// failing tests. -// -// Phase 1: spawn demo with just [--chk-gpu] (reference comparison only). -// If phase 1 exits 0 -> test passes, we're done. GBV cost skipped. -// If phase 1 fails -> spawn demo again with [--chk-gpu, --d3d-dbg] and -// append its output to the same log block with a -// marker separator. Test verdict stays based on -// phase 1's exit code; phase 2 is diagnostic only. -// -// The --force-gbv CLI flag bypasses the gate entirely and always runs -// with --d3d-dbg (matches pre-gating behavior). Useful when someone wants -// unconditional GBV output on all tests for a stress run. -// -// A file-static counter (g_d3d12Metrics) tracks how often phase 2 fires -// and prints a one-line summary at program exit — makes it easy to -// eyeball whether the gate is doing meaningful work. -static void RunCorrectnessTestWithGbvFallback(const std::string& zstFile) -{ - // --force-gbv: skip the gate entirely; behave like the original - // RunCorrectnessTest with the full flag set. We don't touch the - // g_d3d12Metrics counters in this path so the summary line at exit - // only prints when the gate was actually active. - if (g_testConfig.forceGbv) - { - RunCorrectnessTest(zstFile, {"--chk-gpu", "--d3d-dbg"}); - return; - } - - ++g_d3d12Metrics.totalRuns; - - // Phase 1: reference comparison only (no --d3d-dbg). - auto phase1Args = BuildCorrectnessArgs(zstFile, {"--chk-gpu"}); - auto phase1 = RunDemo(g_testConfig.demoPath, phase1Args, g_testConfig.timeoutSeconds); - WriteToLogFile(zstFile, phase1); - - std::cout << "[DEMO CMD] " << phase1.commandLine << "\n"; - if (!phase1.stdOut.empty()) - { - std::cout << "[DEMO OUT] " << phase1.stdOut << "\n"; - } - - // If phase 1 passed cleanly, we're done — skip GBV, test passes. - const bool phase1Ok = !phase1.timedOut - && phase1.launchError.empty() - && phase1.exitCode == 0; - if (phase1Ok) - { - return; - } - - // Phase 2: re-run with --d3d-dbg for GBV diagnostic. Log gets the - // marker separator + phase-2 output appended to the same test's block. - ++g_d3d12Metrics.fallbackFires; - std::cout << "[D3D12DebugLayer PHASE 2] Phase 1 (reference comparison) failed. " - << "Re-running with --d3d-dbg for GBV diagnostic output.\n"; - - auto phase2Args = BuildCorrectnessArgs(zstFile, {"--chk-gpu", "--d3d-dbg"}); - auto phase2 = RunDemo(g_testConfig.demoPath, phase2Args, g_testConfig.timeoutSeconds); - AppendPhaseTwoToLogFile(zstFile, phase2); - - std::cout << "[DEMO CMD] " << phase2.commandLine << "\n"; - if (!phase2.stdOut.empty()) - { - std::cout << "[DEMO OUT] " << phase2.stdOut << "\n"; - } - - // Assert on PHASE 1 exit code — phase 2 is diagnostic only. If phase 2 - // also failed (which it usually will since it's the same input), the - // GBV output is already captured in the log for post-mortem analysis. - ASSERT_FALSE(phase1.timedOut) - << "Phase 1 (reference comparison) timed out after " - << g_testConfig.timeoutSeconds << " seconds.\n" - << "Command: " << phase1.commandLine; - - ASSERT_TRUE(phase1.launchError.empty()) - << "Failed to launch demo (phase 1): " << phase1.launchError << "\n" - << "Command: " << phase1.commandLine; - - ASSERT_EQ(phase1.exitCode, 0) - << "Phase 1 (reference comparison) returned non-zero exit code: " - << phase1.exitCode << "\n" - << "Command: " << phase1.commandLine - << "\n (phase 2 with --d3d-dbg was re-run — its GBV output is " - << "in the log above under the PHASE 2 marker)"; -} - // Run a performance scenario. Spawns zstdgpu_demo.exe with profiling flags and requests CSV output. Uses EXPECT (not ASSERT) to verify the demo executed successfully and produced CSV output. // main() has already validated the demo path exists. static void RunPerformanceTest(const std::string& zstFile, int profilingLevel) @@ -396,7 +251,7 @@ TEST_P(ZstdGpuDemoTests, SimulationCheck) TEST_P(ZstdGpuDemoTests, D3D12DebugLayer) { - RunCorrectnessTestWithGbvFallback(GetParam()); + RunCorrectnessTest(GetParam(), {"--chk-gpu", "--d3d-dbg"}); } TEST_P(ZstdGpuDemoTests, ExternalMemory) diff --git a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h index e03f210..aeec97c 100644 --- a/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h +++ b/zstd/zstdgpu_ci_tests/zstdgpu_ci_tests.h @@ -28,7 +28,6 @@ struct TestConfig std::string logFile; // Consolidated text log file path (--log-file) int runCount = 40; // Number of iterations for performance tests int timeoutSeconds = 0; // Max seconds before killing a demo process (0 = no timeout) - bool forceGbv = false; // Skip the D3D12DebugLayer two-phase gate (always run with --d3d-dbg) // Cached list of .zst files discovered under contentPath. Populated once // in main() after validation; consumed by GetTestFiles() at fixture From e8ce8472ec31fb4daf7d02e57b3d4c2254b4776d Mon Sep 17 00:00:00 2001 From: Rohan Borkar <rohanborkar@microsoft.com> Date: Thu, 9 Jul 2026 10:58:04 -0700 Subject: [PATCH 8/8] zstdgpu_demo: preserve default auto-naming when --out-csv is not specified 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. --- zstd/zstdgpu_demo/main.cpp | 62 +++++++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 24 deletions(-) diff --git a/zstd/zstdgpu_demo/main.cpp b/zstd/zstdgpu_demo/main.cpp index 91345e5..34e3628 100644 --- a/zstd/zstdgpu_demo/main.cpp +++ b/zstd/zstdgpu_demo/main.cpp @@ -1796,41 +1796,55 @@ static int demoRun(void *demoCtx) if (NULL == csvFile) { - // find the start of .zst file name (to further add into .csv name) - const wchar_t *zstNameStart = wcsrchr(zstFilePath, L'\\'); - if (NULL == zstNameStart) + if (NULL != csvFilePathStorage) { - zstNameStart = zstFilePath; + // --out-csv <path> was specified by the caller. + // Honor the exact path so automated callers can locate the CSV + // at the location they requested. + _wfopen_s(&csvFile, csvFilePath, L"w"); } else { - zstNameStart += 1; - } + // --out-csv was not specified; default csvFilePath is "perf.csv". + // Append the .zst stem and a timestamp so repeated interactive runs + // don't clobber each other. + + // find the start of .zst file name (to further add into .csv name) + const wchar_t *zstNameStart = wcsrchr(zstFilePath, L'\\'); + if (NULL == zstNameStart) + { + zstNameStart = zstFilePath; + } + else + { + zstNameStart += 1; + } - // find the start of .zst extension (to remove it from the name when adding into .csv name) - const wchar_t *zstExtStart = wcsrchr(zstNameStart, L'.'); + // find the start of .zst extension (to remove it from the name when adding into .csv name) + const wchar_t *zstExtStart = wcsrchr(zstNameStart, L'.'); - // calculate the length of .zst name without extension - size_t zstNameLen = NULL != zstExtStart ? (zstExtStart - zstNameStart) : wcslen(zstNameStart); + // calculate the length of .zst name without extension + size_t zstNameLen = NULL != zstExtStart ? (zstExtStart - zstNameStart) : wcslen(zstNameStart); - const wchar_t *csvExtStart = wcsrchr(csvFilePath, L'.'); - size_t csvFilePathLenNoExt = NULL != csvExtStart ? (csvExtStart - csvFilePath) : wcslen(csvFilePath); - csvExtStart = L".csv"; + const wchar_t *csvExtStart = wcsrchr(csvFilePath, L'.'); + size_t csvFilePathLenNoExt = NULL != csvExtStart ? (csvExtStart - csvFilePath) : wcslen(csvFilePath); + csvExtStart = L".csv"; - time_t timeData = time(NULL); - tm timeDataLocal; - localtime_s(&timeDataLocal, &timeData); + time_t timeData = time(NULL); + tm timeDataLocal; + localtime_s(&timeDataLocal, &timeData); - const int printBufSize = ZSTDGPU_WARN_DISABLE_MSVC(4996, _snwprintf(NULL, 0, L"%.*s_%.*s_%4d-%02d-%02d_%02d-%02d-%02d%s", (int)csvFilePathLenNoExt, csvFilePath, (int)zstNameLen, zstNameStart, 1900 + timeDataLocal.tm_year, 1 + timeDataLocal.tm_mon, timeDataLocal.tm_mday, timeDataLocal.tm_hour, timeDataLocal.tm_min, timeDataLocal.tm_sec, csvExtStart) + 1); - wchar_t *printBuf = (wchar_t *)malloc(printBufSize * sizeof(wchar_t)); + const int printBufSize = ZSTDGPU_WARN_DISABLE_MSVC(4996, _snwprintf(NULL, 0, L"%.*s_%.*s_%4d-%02d-%02d_%02d-%02d-%02d%s", (int)csvFilePathLenNoExt, csvFilePath, (int)zstNameLen, zstNameStart, 1900 + timeDataLocal.tm_year, 1 + timeDataLocal.tm_mon, timeDataLocal.tm_mday, timeDataLocal.tm_hour, timeDataLocal.tm_min, timeDataLocal.tm_sec, csvExtStart) + 1); + wchar_t *printBuf = (wchar_t *)malloc(printBufSize * sizeof(wchar_t)); - if (NULL != printBuf) - { - ZSTDGPU_WARN_DISABLE_MSVC(4996, _snwprintf(printBuf, printBufSize, L"%.*s_%.*s_%4d-%02d-%02d_%02d-%02d-%02d%s", (int)csvFilePathLenNoExt, csvFilePath, (int)zstNameLen, zstNameStart, 1900 + timeDataLocal.tm_year, 1 + timeDataLocal.tm_mon, timeDataLocal.tm_mday, timeDataLocal.tm_hour, timeDataLocal.tm_min, timeDataLocal.tm_sec, csvExtStart)); - _wfopen_s(&csvFile, printBuf, L"w"); - } + if (NULL != printBuf) + { + ZSTDGPU_WARN_DISABLE_MSVC(4996, _snwprintf(printBuf, printBufSize, L"%.*s_%.*s_%4d-%02d-%02d_%02d-%02d-%02d%s", (int)csvFilePathLenNoExt, csvFilePath, (int)zstNameLen, zstNameStart, 1900 + timeDataLocal.tm_year, 1 + timeDataLocal.tm_mon, timeDataLocal.tm_mday, timeDataLocal.tm_hour, timeDataLocal.tm_min, timeDataLocal.tm_sec, csvExtStart)); + _wfopen_s(&csvFile, printBuf, L"w"); + } - free(printBuf); + free(printBuf); + } } if (NULL != csvFile)