diff --git a/Cargo.lock b/Cargo.lock index a46a744a0..697b40780 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -23,6 +23,16 @@ version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "320119579fcad9c21884f5c4861d16174d0e06250625266f50fe6898340abefa" +[[package]] +name = "adlx" +version = "0.0.0-alpha.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fae45a2d1c9dcfb7fc8bb3069dc821d38218b36d6e1d2ebbd7d80a4751c3aef6" +dependencies = [ + "anyhow", + "libloading", +] + [[package]] name = "aes" version = "0.8.4" @@ -2303,6 +2313,16 @@ version = "0.2.186" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66" +[[package]] +name = "libloading" +version = "0.8.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d7c4b02199fee7c5d21a5ae7d8cfa79a6ef5bb2fc834d6e9058e89c825efdc55" +dependencies = [ + "cfg-if", + "windows-link", +] + [[package]] name = "libm" version = "0.2.16" @@ -3710,6 +3730,7 @@ dependencies = [ name = "rocm-dash-collectors" version = "0.1.0" dependencies = [ + "adlx", "bollard", "chrono", "csv", @@ -3723,6 +3744,7 @@ dependencies = [ "thiserror 2.0.18", "tokio", "tracing", + "windows-sys 0.61.2", "wiremock", ] @@ -3777,6 +3799,7 @@ dependencies = [ "ratatui", "reqwest 0.13.4", "rig-core", + "rocm-dash-collectors", "rocm-dash-core", "rocm-deps", "serde", diff --git a/MANIFEST.md b/MANIFEST.md index 4fdb8bf83..3b2fc107a 100644 --- a/MANIFEST.md +++ b/MANIFEST.md @@ -36,6 +36,7 @@ repository. | Inflector | 0.11.4 | BSD-2-Clause | | addr2line | 0.25.1 | Apache-2.0 OR MIT | | adler2 | 2.0.1 | 0BSD OR MIT OR Apache-2.0 | +| adlx | 0.0.0-alpha.1 | MIT | | aes | 0.8.4 | MIT OR Apache-2.0 | | aho-corasick | 1.1.4 | Unlicense OR MIT | | allocator-api2 | 0.2.21 | MIT OR Apache-2.0 | @@ -258,6 +259,7 @@ repository. | lab | 0.11.0 | MIT | | lazy_static | 1.5.0 | MIT OR Apache-2.0 | | libc | 0.2.186 | MIT OR Apache-2.0 | +| libloading | 0.8.9 | ISC | | libm | 0.2.16 | MIT | | libredox | 0.1.17 | MIT | | line-clipping | 0.3.7 | MIT OR Apache-2.0 | diff --git a/README.md b/README.md index 5bbba2164..9e5fcc175 100644 --- a/README.md +++ b/README.md @@ -31,16 +31,17 @@ for Lemonade and vLLM. | Platform | Prebuilt binary | Notes | |---|---|---| | Linux (x86_64) | Yes | Ubuntu 24.04 or newer; full support, including the live dashboard and both inference engines | -| Windows (x86_64) | Yes | CLI and Lemonade serving; no live dashboard or vLLM | +| Windows (x86_64) | Yes | CLI and Lemonade serving; live dashboard shows GPU/CPU/memory but not the serving-instance registry; no vLLM | | WSL2 (x86_64) | Yes (Linux binary) | Ubuntu 24.04 or newer; full support, including the live dashboard | | macOS | No | No official installer, release, CI, or QA coverage | For WSL2 setup and diagnosis, see [docs/wsl.md](docs/wsl.md). -Live dashboard telemetry requires Linux or WSL2 (see -[Interactive interfaces](#interactive-interfaces)). vLLM serving is Linux or WSL2 -only (see [docs/vllm.md](docs/vllm.md)). +Live dashboard telemetry works on Windows too, but shows only GPU/CPU/memory — +the serving-instance registry view needs the `rocmdashd` unix socket, so it's +Linux/WSL2 only (see [Interactive interfaces](#interactive-interfaces)). vLLM +serving is Linux or WSL2 only (see [docs/vllm.md](docs/vllm.md)). The minimum supported Linux release, native or under WSL2, is Ubuntu 24.04. On other distributions the equivalent requirement is glibc 2.38 with @@ -226,9 +227,10 @@ five tabs (switch with `Tab`/`Shift+Tab` or number keys `1`–`5`): | **Observe** | Live GPU utilization, instances, and benchmark telemetry | | **Chat** | Assistant chat backed by a local server or configured provider | -Live mode reads telemetry over a Unix domain socket, so it requires Linux or -WSL2. Use `rocm dash --demo` for a synthetic session that runs anywhere without a -GPU or daemon. +Live mode works on Windows too, but shows only GPU/CPU/memory — the +serving-instance registry needs the Unix domain socket telemetry, so that view +is Linux/WSL2 only. Use `rocm dash --demo` for a synthetic session that runs +anywhere without a GPU or daemon. ## Configure ROCm and serve a model @@ -937,7 +939,9 @@ For `rocm dash`: - `--demo` runs a deterministic synthetic session with no GPU or daemon needed. It works on all platforms. - `--replay ` replays a recorded NDJSON session. -- Live mode requires Unix domain sockets (Linux and WSL2 only). +- Live mode works on Windows too, but shows only GPU/CPU/memory — there's no + `rocmdashd` unix socket to connect to there, so the serving-instance + registry view stays empty (Linux and WSL2 get the full live view). ### Bench diff --git a/THIRD_PARTY_NOTICES.txt b/THIRD_PARTY_NOTICES.txt index 5da394269..08cc3b414 100644 --- a/THIRD_PARTY_NOTICES.txt +++ b/THIRD_PARTY_NOTICES.txt @@ -8937,6 +8937,27 @@ OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +================================================================================ +ISC License +================================================================================ + +The following component(s) are licensed under ISC License: + - libloading 0.8.9 + +Copyright © 2015, Simonas Kazlauskas + +Permission to use, copy, modify, and/or distribute this software for any purpose with or without +fee is hereby granted, provided that the above copyright notice and this permission notice appear +in all copies. + +THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS +SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE +AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES +WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, +NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF +THIS SOFTWARE. + + ================================================================================ ISC License ================================================================================ @@ -10684,6 +10705,36 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. +================================================================================ +MIT License +================================================================================ + +The following component(s) are licensed under MIT License: + - adlx 0.0.0-alpha.1 + +MIT License + +Copyright (c) 2024 Traverse Research BV + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. + + ================================================================================ MIT License ================================================================================ diff --git a/apps/rocm/src/dash.rs b/apps/rocm/src/dash.rs index 891255c1d..7af55d7bd 100644 --- a/apps/rocm/src/dash.rs +++ b/apps/rocm/src/dash.rs @@ -339,11 +339,12 @@ pub fn resolved_args( /// Re-reads `services_dir` fresh on every [`snapshot`](ManagedRegistryProbe::snapshot) /// call: a local JSON-file read, no daemon round-trip, no network I/O — cheap /// enough for the quit-confirm gate to call on every render while the prompt -/// is open, and the only source of truth at all on a host whose -/// dashboard-embedded daemon client never connects (Windows — -/// `rocm_dash_tui::client`'s connect path and `maybe_spawn_embedded_daemon` -/// are unix-only, so `AppState::instances` never gets a first snapshot -/// there). +/// is open, and the only source of truth at all on Windows, where +/// `rocm_dash_tui::client`'s `#[cfg(windows)]` arm does connect and stream +/// live GPU/CPU/memory every tick but — by design, there being no +/// `rocmdashd` unix socket to source one from (`maybe_spawn_embedded_daemon` +/// is unix-only) — never sends an instance snapshot, so +/// `AppState::instances` never gets a first one there. /// /// **Known divergence, same shape as [`services_past_attempts`]'s:** this /// reads the record's raw on-disk `status`, not process liveness. A service diff --git a/crates/rocm-dash-collectors/Cargo.toml b/crates/rocm-dash-collectors/Cargo.toml index da6c5c631..f9f50b69e 100644 --- a/crates/rocm-dash-collectors/Cargo.toml +++ b/crates/rocm-dash-collectors/Cargo.toml @@ -30,3 +30,11 @@ reqwest = { version = "0.12", default-features = false } tokio = { version = "1", features = ["full"] } tempfile = "3" wiremock = "0.6" + +# ADLX binds dynamically to the AMD driver's bundled amdadlx64.dll at +# runtime (not a build-time link), so this only makes sense on Windows; +# winperf.rs's `AdlxHelper::new()` call already degrades to 0.0/None when +# the DLL isn't present (no AMD driver, non-AMD GPU), same as elsewhere. +[target.'cfg(windows)'.dependencies] +adlx = "0.0.0-alpha.1" +windows-sys = { version = "0.61", features = ["Win32_System_LibraryLoader"] } diff --git a/crates/rocm-dash-collectors/src/lib.rs b/crates/rocm-dash-collectors/src/lib.rs index f5b7bcd8f..6ce97985c 100644 --- a/crates/rocm-dash-collectors/src/lib.rs +++ b/crates/rocm-dash-collectors/src/lib.rs @@ -20,3 +20,4 @@ pub mod proc_scan; pub mod sysfs; pub mod vllm_log; pub mod vllm_prom; +pub mod winperf; diff --git a/crates/rocm-dash-collectors/src/winperf.rs b/crates/rocm-dash-collectors/src/winperf.rs new file mode 100644 index 000000000..444c2939c --- /dev/null +++ b/crates/rocm-dash-collectors/src/winperf.rs @@ -0,0 +1,776 @@ +// Copyright © Advanced Micro Devices, Inc., or its affiliates. +// +// SPDX-License-Identifier: MIT + +//! Live Windows GPU metrics via OS performance counters, no AMD tool needed +//! for utilization/VRAM. +//! +//! Temperature, power, and clock speed additionally use ADLX (AMD Device +//! Library eXtra, via the `adlx` crate) when the AMD driver's bundled +//! `amdadlx64.dll` is present; absence (no AMD driver, non-AMD GPU) degrades +//! those three fields to 0.0/None rather than failing the whole snapshot. +//! +//! Reads the `GPU Engine`/`GPU Adapter Memory` categories — the same +//! WDDM/DXGK-backed source Task Manager's GPU tab reads. Mirrors +//! `amd_smi.rs`'s subprocess + pure-parse shape, shelling out to +//! `powershell.exe` instead of `amd-smi`. Not `#[cfg(windows)]`-gated at the +//! module level, but `snapshot()`'s only caller today is itself +//! `#[cfg(windows)]` (see `client.rs`'s `connect_and_run`). On a plain Linux +//! host `powershell.exe` isn't on `PATH` and this fails cleanly like +//! `amd_smi.rs` does when `amd-smi` is missing — but under WSL interop, +//! `powershell.exe` *is* on `PATH` and this would run against the Windows +//! host's GPUs, so don't add a Linux-reachable caller without reconsidering +//! that. + +use std::collections::{BTreeMap, BTreeSet}; +use std::io; +use std::time::Duration; + +use rocm_dash_core::metrics::{GpuMetrics, GpuSystemInfo}; +use tokio::process::Command; +use tokio::task; +use tokio::time::timeout; + +// 5s was too tight: on real hardware `powershell.exe` child-process spawn +// (not the Get-Counter query itself, which returns near-instantly once +// running) can take well over 5s — observed consistently timing out every +// tick on a Strix Halo laptop while the identical script ran fine +// interactively. 15s gives real-world spawn latency room without masking an +// actually-hung process forever. +const RUN_TIMEOUT: Duration = Duration::from_secs(15); + +/// ADLX init/read/terminate shouldn't normally take anywhere near this, but a +/// wedged driver call must not freeze CPU/memory updates forever alongside it +/// — same reasoning as `RUN_TIMEOUT`, just a tighter bound since there's no +/// process spawn latency to absorb here. +const ADLX_TIMEOUT: Duration = Duration::from_secs(5); + +/// `kindkeyvalue[value2]` lines: per-engine utilization, +/// per-adapter dedicated VRAM usage, and a best-effort adapter inventory +/// (name/RAM/driver) for `GpuSystemInfo`. One process spawn for all three. +const WINPERF_SCRIPT: &str = r#" +$ErrorActionPreference = 'SilentlyContinue' +(Get-Counter '\GPU Engine(*)\Utilization Percentage' -ErrorAction SilentlyContinue).CounterSamples | + ForEach-Object { "UTIL`t$($_.InstanceName)`t$($_.CookedValue)" } +(Get-Counter '\GPU Adapter Memory(*)\Dedicated Usage' -ErrorAction SilentlyContinue).CounterSamples | + ForEach-Object { "VRAM`t$($_.InstanceName)`t$($_.CookedValue)" } +Get-CimInstance -ClassName Win32_VideoController -Property Name,AdapterRAM,DriverVersion -ErrorAction SilentlyContinue | + ForEach-Object { "GPU`t$($_.Name)`t$($_.AdapterRAM)`t$($_.DriverVersion)" } +"#; + +#[derive(Debug, Default)] +pub struct WinPerfCollector; + +impl WinPerfCollector { + pub const fn new() -> Self { + Self + } + + /// One `powershell.exe` spawn covers both metrics and system info (the + /// script already emits both sections) — calling `metrics()` and + /// `system_info()` separately each tick would shell out twice for the + /// same output, doubling exposure to spawn latency for nothing. + /// + /// The returned `Vec` surfaces non-fatal problems the caller + /// should show the user (e.g. a locale mismatch, see + /// `counter_samples_missing_warning`) without failing the whole tick. + pub async fn snapshot(&self) -> io::Result<(Vec, GpuSystemInfo, Vec)> { + let out = Self::run_script().await?; + // adlx_readings() calls into amdadlx64.dll via FFI, which blocks on + // driver I/O — running it directly here would stall a tokio worker + // thread, same hazard run_script() avoids by using an async Command. + // Timed out the same way: a wedged driver call must not freeze every + // other metric this tick forever, and either failure mode is + // surfaced as a warning rather than silently degrading to defaults. + let (adlx, adlx_warning) = + match timeout(ADLX_TIMEOUT, task::spawn_blocking(adlx_readings)).await { + Ok(Ok(readings)) => (readings, None), + Ok(Err(_)) => (Vec::new(), Some("ADLX reading task panicked".to_string())), + Err(_) => ( + Vec::new(), + Some(format!("ADLX query timed out after {ADLX_TIMEOUT:?}")), + ), + }; + let metrics = parse_metrics(&out, &adlx); + let warnings = counter_samples_missing_warning(&out, &metrics) + .into_iter() + .chain(adlx_warning) + .collect(); + Ok((metrics, parse_system_info(&out), warnings)) + } + + async fn run_script() -> io::Result { + let out = timeout( + RUN_TIMEOUT, + Command::new("powershell.exe") + .args(["-NoProfile", "-NonInteractive", "-Command", WINPERF_SCRIPT]) + // Dropping the child on timeout must actually kill it, or a + // hung powershell.exe (and its driver query) leaks forever + // instead of exiting with the future that gave up on it. + .kill_on_drop(true) + .output(), + ) + .await + .map_err(|_| io::Error::other("powershell Get-Counter query timed out"))??; + if !out.status.success() { + return Err(io::Error::other(format!( + "powershell Get-Counter exited {}: {}", + out.status, + String::from_utf8_lossy(&out.stderr).trim() + ))); + } + Ok(String::from_utf8_lossy(&out.stdout).into_owned()) + } +} + +/// `Win32_VideoController.AdapterRAM` is a 32-bit WMI field: a card with real +/// VRAM at or beyond its ~4GiB ceiling gets silently truncated/capped rather +/// than reporting its true size. A value this close to that ceiling is far +/// more likely a truncated sentinel than an improbably-exact ~4GiB card, so +/// treat it as unknown (0) rather than report a number that's probably wrong. +const WMI_ADAPTER_RAM_CAP_THRESHOLD: u64 = 4_000_000_000; + +const fn wmi_vram_total_mb(ram_bytes: u64) -> u64 { + if ram_bytes >= WMI_ADAPTER_RAM_CAP_THRESHOLD { + 0 + } else { + ram_bytes / (1024 * 1024) + } +} + +/// `GPU Engine` instances are per-process-per-engine +/// (`pid_1234_luid_0x..._0x..._phys_0_eng_0_engtype_3d`); `GPU Adapter +/// Memory` instances are per-adapter only (`luid_0x..._0x..._phys_0`, no +/// pid). Strip any `pid_NNNN_` prefix before keying so both counter +/// families collapse onto the same per-adapter key — otherwise every +/// process using the GPU looks like a distinct phantom device. +fn device_key(instance: &str) -> &str { + let instance = instance.find("luid_").map_or(instance, |i| &instance[i..]); + instance.split("_phys_").next().unwrap_or(instance) +} + +/// `GPU Engine` instance names end in `..._eng_N_engtype_TYPE` (e.g. `3d`, +/// `copy`, `videodecode`); this is the engine-type suffix, distinct per +/// engine but shared across every process using that same engine. Pairs +/// with `device_key` in `parse_metrics` to key utilization by (device, +/// engine) rather than by device alone. +fn engine_type(instance: &str) -> &str { + instance.rsplit("_engtype_").next().unwrap_or(instance) +} + +/// Builds one `GpuMetrics` per distinct adapter LUID seen in the counter +/// output. Utilization is the max across that adapter's engines (3D/copy/ +/// video/etc.) — the same "overall GPU" reading Task Manager shows. +/// +/// Windows can report more than one `GPU Adapter Memory` LUID for a single +/// physical GPU (stale instances left behind by a display-mode switch, or +/// separate WDDM engine nodes) — on an integrated APU this shows up as a +/// "3 GPUs" readout with two phantom zero-VRAM devices. When +/// `Win32_VideoController` says there's exactly one real adapter, collapse +/// every LUID into that one device instead of reporting one per LUID: sum +/// VRAM used (a phantom LUID contributes 0), take the max utilization. +/// +/// With more than one real adapter, correlating a `GPU Adapter Memory` LUID +/// to a specific `Win32_VideoController` entry isn't reliably possible +/// without vendor-specific APIs, so `vram_total_mb` is left at 0 (every TUI +/// consumer already guards `vram_total_mb > 0` before dividing). The +/// single-adapter case — the realistic target, an integrated AMD APU — gets +/// an exact total; multi-GPU hosts degrade to 0 instead of a wrong number. +/// Upgrade path if multi-GPU Windows hosts become a real target: DXGI/LUID- +/// based adapter correlation. +/// +/// `adlx` is a parameter (rather than this function calling `adlx_readings()` +/// itself) so tests can exercise the ADLX-matching/VRAM-fallback logic below +/// with fixed readings — `adlx_readings()`'s real FFI chain is Windows+driver +/// only and not unit-testable. +fn parse_metrics(raw: &str, adlx: &[AdlxReading]) -> Vec { + // Keyed by (device, engine type): `GPU Engine` instances are + // per-process-per-engine, so two processes both drawing on the 3D engine + // each contribute their own share of that engine's capacity and must be + // summed, matching Task Manager. Different engine types (3D vs. copy vs. + // video) run concurrently on separate hardware, so those are maxed + // together below instead of summed. + let mut engine_util: BTreeMap<(&str, &str), f32> = BTreeMap::new(); + let mut vram_mb: BTreeMap<&str, u64> = BTreeMap::new(); + // A `BTreeSet` rather than first-seen order: `powershell.exe`'s adapter + // enumeration order isn't guaranteed stable between ticks, which would + // otherwise make `win-gpu-{i}` swap which physical device it names from + // one snapshot to the next (mixing per-device history in the TUI). + // Sorting by the LUID-derived key instead keeps each device's index + // stable for as long as its LUID is (i.e. until reboot/driver reload). + let mut seen: BTreeSet<&str> = BTreeSet::new(); + + for line in raw.lines() { + let mut parts = line.splitn(3, '\t'); + let (Some(kind), Some(instance), Some(value)) = (parts.next(), parts.next(), parts.next()) + else { + continue; + }; + if kind != "UTIL" && kind != "VRAM" { + continue; // GPU inventory lines are handled separately by parse_adapters. + } + let key = device_key(instance); + seen.insert(key); + match kind { + "UTIL" => { + let pct: f32 = value.trim().parse().unwrap_or(0.0); + *engine_util + .entry((key, engine_type(instance))) + .or_insert(0.0) += pct; + } + "VRAM" => { + let bytes: f64 = value.trim().parse().unwrap_or(0.0); + *vram_mb.entry(key).or_insert(0) += (bytes / (1024.0 * 1024.0)) as u64; + } + _ => unreachable!(), + } + } + + let mut util: BTreeMap<&str, f32> = BTreeMap::new(); + for (&(device, _engine), &pct) in &engine_util { + let entry = util.entry(device).or_insert(0.0); + if pct > *entry { + *entry = pct; + } + } + + let adapters = parse_adapters(raw); + if let [adapter] = adapters.as_slice() { + if seen.is_empty() { + return Vec::new(); + } + let vram_used_mb: u64 = vram_mb.values().sum(); + let gpu_utilization_pct = util.values().copied().fold(0.0f32, f32::max).min(100.0); + // ADLX's GPU list isn't otherwise correlated to the perf-counter + // LUIDs here, so it's only trusted at all when there's exactly one + // ADLX reading to match this one real adapter against — same + // single-reading guard `vram_total_mb` below already uses. + let (temperature_c, power_w, clock_mhz) = match adlx { + [reading] => apply_adlx(Some(reading)), + _ => apply_adlx(None), + }; + // `Win32_VideoController.AdapterRAM` is a 32-bit WMI field: any card + // with >=4GiB VRAM truncates/overflows there. ADLX's own VRAM query + // isn't subject to that, so prefer it when exactly one ADLX GPU + // reading exists (matching this single-real-adapter branch) and + // fall back to the WMI value otherwise (no AMD driver, non-AMD GPU). + let vram_total_mb = match adlx { + [reading] => reading + .total_vram_mb + .map_or_else(|| wmi_vram_total_mb(adapter.ram_bytes), u64::from), + _ => wmi_vram_total_mb(adapter.ram_bytes), + }; + return vec![GpuMetrics { + device_id: "win-gpu-0".to_string(), + vram_used_mb, + vram_total_mb, + gpu_utilization_pct, + temperature_c, + power_w, + clock_mhz, + }]; + } + + // More than one real adapter: an ADLX GPU-list index has no reliable + // correlation to a specific perf-counter LUID (same reason + // `vram_total_mb` is left at 0 below), so every slot degrades to + // ADLX-absent defaults rather than risk reporting one GPU's temp/power/ + // clock under a different GPU's device_id. + let (temperature_c, power_w, clock_mhz) = apply_adlx(None); + seen.into_iter() + .enumerate() + .map(|(i, key)| GpuMetrics { + device_id: format!("win-gpu-{i}"), + vram_used_mb: vram_mb.get(key).copied().unwrap_or(0), + vram_total_mb: 0, + gpu_utilization_pct: util.get(key).copied().unwrap_or(0.0).min(100.0), + temperature_c, + power_w, + clock_mhz, + }) + .collect() +} + +/// Per-GPU temperature/power/clock reading from ADLX. `adlx_readings` emits +/// exactly one entry per GPU in ADLX's list, whether or not +/// `current_gpu_metrics` succeeded for it, so a failure degrades the fields +/// to 0.0/None in place rather than shifting every later GPU's reading out +/// of position. +struct AdlxReading { + temperature_c: f32, + power_w: f32, + clock_mhz: Option, + /// ADLX's own VRAM-size query (`IADLXGPU::TotalVRAM`, already in MB). + /// `Win32_VideoController.AdapterRAM` is a 32-bit WMI field that + /// silently truncates/overflows above 4GiB — common on any discrete + /// card, just not on the integrated APU this was tested against — so + /// `parse_metrics` prefers this reading over WMI's when available. + total_vram_mb: Option, +} + +/// `None` (no device-identity correlation to trust, or ADLX unavailable) +/// degrades to the same 0.0/None defaults `parse_metrics` used before ADLX +/// existed. Callers only pass `Some` when there's exactly one real adapter +/// and exactly one ADLX reading — the only case this file can match an ADLX +/// reading to a device without index-based guessing. +fn apply_adlx(reading: Option<&AdlxReading>) -> (f32, f32, Option) { + ( + reading.map_or(0.0, |r| r.temperature_c), + reading.map_or(0.0, |r| r.power_w), + reading.and_then(|r| r.clock_mhz), + ) +} + +/// `AdlxHelper::new()` fails cleanly (`Err`, not a panic) when +/// `amdadlx64.dll` isn't found, so a total init failure can just bail to an +/// empty Vec via `.ok()?` — same graceful-degrade shape as the rest of this +/// file. A single GPU's metrics query failing, though, must NOT drop that +/// GPU's slot (a `filter_map` here would silently change the reading count, +/// which `parse_metrics`'s single-reading guard relies on to decide whether +/// an ADLX reading can be trusted at all) — it degrades to a zeroed +/// `AdlxReading` instead, keeping one slot per GPU. +#[cfg(windows)] +fn adlx_readings() -> Vec { + // `adlx::helper::AdlxHelper::new()` loads `amdadlx64.dll` by bare name + // (`libloading::Library::new("amdadlx64.dll")`, no search-order + // hardening, no way to pass flags into the `adlx` crate). Restricting + // the process-wide default DLL search path to System32 — this call + // affects every subsequent bare-name `LoadLibrary` in the whole process, + // not just `adlx`'s — closes the planted-DLL-in-PATH/cwd hijack vector + // for that unflagged load. Must run before the first `AdlxHelper::new()` + // call; `Once` makes repeated per-tick calls to this function a no-op + // after the first. + static HARDEN_DLL_SEARCH_PATH: std::sync::Once = std::sync::Once::new(); + #[allow(unsafe_code)] // Win32 FFI + HARDEN_DLL_SEARCH_PATH.call_once(|| unsafe { + // Failure here (`FALSE`) means the process keeps its default search + // order (which can include the cwd) rather than System32-only; still + // attempt the `AdlxHelper` load afterwards; a stricter search path is + // defense in depth, not a precondition for ADLX to work. + let hardened = windows_sys::Win32::System::LibraryLoader::SetDefaultDllDirectories( + windows_sys::Win32::System::LibraryLoader::LOAD_LIBRARY_SEARCH_SYSTEM32, + ); + if hardened == 0 { + tracing::warn!( + "SetDefaultDllDirectories failed; amdadlx64.dll load falls back to the default search order" + ); + } + }); + + (|| -> Option> { + let helper = adlx::helper::AdlxHelper::new().ok()?; + let system = helper.system(); + let gpus = system.gpus().ok()?; + let services = system.performance_monitoring_services().ok()?; + Some( + gpus.iter() + .map(|gpu| { + // Independent of current_gpu_metrics: a transient + // metrics-query failure shouldn't also discard an + // otherwise-good VRAM-size reading. + let total_vram_mb = gpu.total_vram().ok(); + match services.current_gpu_metrics(&gpu) { + Ok(m) => AdlxReading { + temperature_c: m.temperature().unwrap_or(0.0) as f32, + power_w: m.power().unwrap_or(0.0) as f32, + clock_mhz: m.clock_speed().ok().map(|c| c as f32), + total_vram_mb, + }, + Err(_) => AdlxReading { + temperature_c: 0.0, + power_w: 0.0, + clock_mhz: None, + total_vram_mb, + }, + } + }) + .collect(), + ) + })() + .unwrap_or_default() +} + +#[cfg(not(windows))] +const fn adlx_readings() -> Vec { + Vec::new() +} + +struct Adapter { + name: String, + ram_bytes: u64, + driver_version: String, +} + +fn parse_adapters(raw: &str) -> Vec { + raw.lines() + .filter_map(|line| { + let mut parts = line.splitn(4, '\t'); + match (parts.next(), parts.next(), parts.next(), parts.next()) { + (Some("GPU"), Some(name), Some(ram), Some(driver)) => Some(Adapter { + name: name.to_string(), + ram_bytes: ram.trim().parse().unwrap_or(0), + driver_version: driver.trim().to_string(), + }), + _ => None, + } + }) + .collect() +} + +fn parse_system_info(raw: &str) -> GpuSystemInfo { + let adapters = parse_adapters(raw); + GpuSystemInfo { + gpu_model: adapters.first().map(|a| a.name.clone()).unwrap_or_default(), + driver_version: adapters.first().map(|a| a.driver_version.clone()), + physical_gpu_count: adapters.len() as u32, + logical_gpu_count: adapters.len() as u32, + ..Default::default() + } +} + +/// `Win32_VideoController` (WMI) and `Get-Counter` (the perf-counter +/// subsystem) are independent queries in the same script: WMI finding an +/// adapter but every counter query coming back empty isn't "no GPU", it's +/// the counters failing silently. The likeliest cause is `Get-Counter`'s +/// English-only counter paths (`\GPU Engine(*)\...`) not matching on a +/// localized Windows install — the script's `-ErrorAction +/// SilentlyContinue` swallows that mismatch rather than erroring, so this +/// is the only signal left to surface it to the user instead of silently +/// showing an empty GPU panel. +fn counter_samples_missing_warning(raw: &str, metrics: &[GpuMetrics]) -> Option { + if metrics.is_empty() && !parse_adapters(raw).is_empty() { + Some( + "Windows GPU utilization/VRAM counters returned no samples (possible non-English \ + Windows locale - Get-Counter counter paths are localized)" + .to_string(), + ) + } else { + None + } +} + +#[cfg(test)] +mod tests { + use super::*; + + // Real `GPU Engine` instances are per-process (`pid_NNNN_luid_...`); mix + // in a pid prefix here so the tests actually exercise that shape. + // 3221225472 (3GiB) and 1073741824 (1GiB) are both representable in the + // real 32-bit `AdapterRAM` WMI field (unlike 16GiB, which would already + // have overflowed it). + const SAMPLE: &str = "UTIL\tpid_1234_luid_0x00000000_0x0000b3a4_phys_0_eng_0_engtype_3d\t12.5\nUTIL\tpid_1234_luid_0x00000000_0x0000b3a4_phys_0_eng_1_engtype_video\t4.0\nVRAM\tluid_0x00000000_0x0000b3a4_phys_0\t1073741824\nGPU\tAMD Radeon(TM) Graphics\t3221225472\t31.0.21021.1000"; + + #[test] + fn parses_single_adapter_metrics_with_total_ram() { + let metrics = parse_metrics(SAMPLE, &[]); + assert_eq!(metrics.len(), 1); + let gpu = &metrics[0]; + // Max across the two engine readings, not the sum. + assert!((gpu.gpu_utilization_pct - 12.5).abs() < 0.01); + assert_eq!(gpu.vram_used_mb, 1024); + assert_eq!(gpu.vram_total_mb, 3072); + } + + #[test] + fn parses_system_info_from_adapter_inventory() { + let info = parse_system_info(SAMPLE); + assert_eq!(info.gpu_model, "AMD Radeon(TM) Graphics"); + assert_eq!(info.driver_version.as_deref(), Some("31.0.21021.1000")); + assert_eq!(info.physical_gpu_count, 1); + } + + #[test] + fn multiple_processes_on_one_adapter_collapse_to_one_device() { + // Two different pids hitting the same adapter's *same* engine, plus a + // *second*, unrelated real adapter. With only one real adapter (as + // the original fixture had), the single-real-adapter collapse rescue + // below would also force everything to one device, masking whether + // this is actually testing `device_key`'s pid-prefix stripping; with + // two real adapters that rescue never fires (its pattern match + // requires exactly one), isolating the two code paths. Both pids + // share the `3d` engine, so their utilization sums (Task Manager + // semantics) rather than taking the max of the two. + let sample = "UTIL\tpid_111_luid_0x00000000_0x0002ff2a_phys_0_eng_0_engtype_3d\t0.682\nUTIL\tpid_222_luid_0x00000000_0x0002ff2a_phys_0_eng_0_engtype_3d\t0.514\nVRAM\tluid_0x00000000_0x0002ff2a_phys_0\t902787072\nUTIL\tluid_0x00000000_0x0000cafe_phys_0_eng_0_engtype_3d\t0.1\nVRAM\tluid_0x00000000_0x0000cafe_phys_0\t1048576\nGPU\tAMD Radeon 780M Graphics\t1073741824\t32.0.31041.1004\nGPU\tOther GPU\t4293918720\t1.0"; + let metrics = parse_metrics(sample, &[]); + assert_eq!(metrics.len(), 2); + // Devices are keyed by sorted LUID string, not first-seen order (see + // `parse_metrics`'s `BTreeSet` comment), so find the device under + // test by its VRAM reading rather than assuming an index. + let gpu = metrics + .iter() + .find(|g| g.vram_used_mb == 902_787_072 / (1024 * 1024)) + .expect("the pid-prefixed adapter's metrics"); + assert!((gpu.gpu_utilization_pct - 1.196).abs() < 0.001); + // Multi-real-adapter case: vram_total_mb always 0, no LUID correlation. + assert_eq!(gpu.vram_total_mb, 0); + } + + #[test] + fn utilization_across_different_engine_types_takes_max_not_sum() { + // Same pid, same device, two *different* engine types running + // concurrently on separate hardware — these must be maxed, not + // summed, unlike same-engine readings from different processes. + let sample = "UTIL\tpid_1_luid_0x00000000_0x0002ff2a_phys_0_eng_0_engtype_3d\t90.0\nUTIL\tpid_1_luid_0x00000000_0x0002ff2a_phys_0_eng_1_engtype_copy\t20.0\nVRAM\tluid_0x00000000_0x0002ff2a_phys_0\t1048576\nGPU\tAMD Radeon 780M Graphics\t1073741824\t32.0.31041.1004"; + let metrics = parse_metrics(sample, &[]); + assert_eq!(metrics.len(), 1); + assert!((metrics[0].gpu_utilization_pct - 90.0).abs() < 0.001); + } + + #[test] + fn summed_engine_utilization_above_100_percent_clamps() { + // Several processes hammering the same engine can sum past 100% of + // a single engine's nominal capacity; the reported utilization must + // still clamp at 100, matching Task Manager's own ceiling. + let sample = "UTIL\tpid_1_luid_0x00000000_0x0002ff2a_phys_0_eng_0_engtype_3d\t60.0\nUTIL\tpid_2_luid_0x00000000_0x0002ff2a_phys_0_eng_0_engtype_3d\t70.0\nVRAM\tluid_0x00000000_0x0002ff2a_phys_0\t1048576\nGPU\tAMD Radeon 780M Graphics\t1073741824\t32.0.31041.1004"; + let metrics = parse_metrics(sample, &[]); + assert_eq!(metrics.len(), 1); + assert!((metrics[0].gpu_utilization_pct - 100.0).abs() < 0.001); + } + + #[test] + fn single_real_adapter_sums_vram_used_across_distinct_luids() { + // Two *distinct* non-zero per-LUID VRAM readings on the one real + // adapter — unlike the phantom-LUID fixture below (0 + 0 + real), + // this is the only fixture where summing vram_used_mb actually + // differs from taking its max, so a sum->max mutation would be + // caught here. + let sample = "UTIL\tluid_0x00000000_0x0002ff2a_phys_0_eng_0_engtype_3d\t1.0\nVRAM\tluid_0x00000000_0x0002ff2a_phys_0\t536870912\nVRAM\tluid_0x00000000_0x000325a2_phys_0\t268435456\nGPU\tAMD Radeon 780M Graphics\t1073741824\t32.0.31041.1004"; + let metrics = parse_metrics(sample, &[]); + assert_eq!(metrics.len(), 1); + assert_eq!(metrics[0].vram_used_mb, 512 + 256); + } + + #[test] + fn gpu_inventory_without_counter_samples_yields_no_metrics() { + // A real adapter shows up in Win32_VideoController but Get-Counter + // returned nothing for it (e.g. a locale where Get-Counter's + // -ErrorAction silently swallows a localized-path mismatch) — must + // not fabricate a zeroed phantom device for it. + let sample = "GPU\tAMD Radeon 780M Graphics\t1073741824\t32.0.31041.1004"; + let metrics = parse_metrics(sample, &[]); + assert!(metrics.is_empty()); + assert_eq!(parse_system_info(sample).physical_gpu_count, 1); + // The empty result must not be silently indistinguishable from "no + // GPU" — the caller needs a warning to show instead. + assert!(counter_samples_missing_warning(sample, &metrics).is_some()); + } + + #[test] + fn counter_samples_missing_warning_is_none_when_metrics_present() { + let sample = "UTIL\tluid_0x00000000_0x0000b3a4_phys_0_eng_0_engtype_3d\t1.0\nVRAM\tluid_0x00000000_0x0000b3a4_phys_0\t1048576\nGPU\tDiscrete GPU\t3221225472\t1.0"; + let metrics = parse_metrics(sample, &[]); + assert!(!metrics.is_empty()); + assert!(counter_samples_missing_warning(sample, &metrics).is_none()); + } + + #[test] + fn counter_samples_missing_warning_is_none_when_no_adapters_at_all() { + // No `Win32_VideoController` line either — genuinely no GPU, not a + // counter-query failure, so no warning is warranted. + assert!(counter_samples_missing_warning("", &[]).is_none()); + } + + #[test] + fn phantom_adapter_memory_luids_collapse_to_one_device_when_only_one_real_adapter() { + // Real-world capture from an AMD Radeon 780M laptop: Windows reports + // 3 distinct `GPU Adapter Memory` LUIDs (2 of them permanently + // zero-VRAM phantoms) for the single physical adapter + // `Win32_VideoController` actually lists. + let sample = "UTIL\tpid_1234_luid_0x00000000_0x0002ff2a_phys_0_eng_0_engtype_3d\t0.5\nVRAM\tluid_0x00000000_0x0002ff2a_phys_0\t897261568\nVRAM\tluid_0x00000000_0x000325a2_phys_0\t0\nVRAM\tluid_0x00000000_0x000325f9_phys_0\t0\nGPU\tAMD Radeon 780M Graphics\t1073741824\t32.0.31041.1004"; + let metrics = parse_metrics(sample, &[]); + assert_eq!(metrics.len(), 1); + let gpu = &metrics[0]; + assert!((gpu.gpu_utilization_pct - 0.5).abs() < 0.001); + assert_eq!(gpu.vram_used_mb, 897_261_568 / (1024 * 1024)); + assert_eq!(gpu.vram_total_mb, 1_073_741_824 / (1024 * 1024)); + } + + #[test] + #[allow(clippy::float_cmp)] + fn single_adapter_prefers_adlx_vram_over_wmi_truncated_value() { + // WMI's AdapterRAM here (4293918720, just under the 4GiB cap + // threshold) is the classic 32-bit-field truncation case; ADLX's own + // query reports the real 8GiB so parse_metrics should prefer it. + let sample = "UTIL\tluid_0x00000000_0x0000b3a4_phys_0_eng_0_engtype_3d\t1.0\nVRAM\tluid_0x00000000_0x0000b3a4_phys_0\t1048576\nGPU\tDiscrete GPU\t4293918720\t1.0"; + let adlx = [AdlxReading { + temperature_c: 50.0, + power_w: 10.0, + clock_mhz: Some(1800.0), + total_vram_mb: Some(8192), + }]; + let metrics = parse_metrics(sample, &adlx); + assert_eq!(metrics.len(), 1); + assert_eq!(metrics[0].vram_total_mb, 8192); + assert_eq!(metrics[0].temperature_c, 50.0); + assert_eq!(metrics[0].power_w, 10.0); + assert_eq!(metrics[0].clock_mhz, Some(1800.0)); + } + + #[test] + fn multiple_adapters_ignore_adlx_vram_and_leave_total_zero() { + let two_adapters = format!( + "{SAMPLE}\nUTIL\tluid_0x00000000_0x0000cafe_phys_0_eng_0_engtype_3d\t1.0\nVRAM\tluid_0x00000000_0x0000cafe_phys_0\t1048576\nGPU\tOther GPU\t4293918720\t1.0" + ); + let adlx = [AdlxReading { + temperature_c: 50.0, + power_w: 10.0, + clock_mhz: Some(1800.0), + total_vram_mb: Some(8192), + }]; + let metrics = parse_metrics(&two_adapters, &adlx); + assert_eq!(metrics.len(), 2); + assert!(metrics.iter().all(|g| g.vram_total_mb == 0)); + assert!( + metrics + .iter() + .all(|g| g.temperature_c == 0.0 && g.power_w == 0.0 && g.clock_mhz.is_none()) + ); + } + + #[test] + fn multiple_adapters_never_trust_adlx_even_when_reading_count_matches_adapter_count() { + // Two ADLX readings for two real adapters: if `parse_metrics` ever + // started correlating by list position again, this is exactly the + // shape that would make it look like it "worked" by coincidence. + let two_adapters = format!( + "{SAMPLE}\nUTIL\tluid_0x00000000_0x0000cafe_phys_0_eng_0_engtype_3d\t1.0\nVRAM\tluid_0x00000000_0x0000cafe_phys_0\t1048576\nGPU\tOther GPU\t4293918720\t1.0" + ); + let two_adlx = [ + AdlxReading { + temperature_c: 50.0, + power_w: 10.0, + clock_mhz: Some(1800.0), + total_vram_mb: Some(8192), + }, + AdlxReading { + temperature_c: 40.0, + power_w: 8.0, + clock_mhz: Some(1200.0), + total_vram_mb: Some(4096), + }, + ]; + let metrics = parse_metrics(&two_adapters, &two_adlx); + assert_eq!(metrics.len(), 2); + assert!(metrics.iter().all(|g| g.vram_total_mb == 0)); + assert!( + metrics + .iter() + .all(|g| g.temperature_c == 0.0 && g.power_w == 0.0 && g.clock_mhz.is_none()) + ); + } + + #[test] + #[allow(clippy::float_cmp)] + fn multi_adapter_utilization_clamps_to_100_when_summed_processes_exceed_it() { + // Two processes both drawing on the same engine of the same adapter + // (summed per the doc comment on `engine_util`) push the raw total + // past 100 on a *second real adapter* alongside the one in `SAMPLE` — + // the multi-adapter branch's own clamp must still catch it, same as + // the single-adapter branch above it. + let two_adapters = format!( + "{SAMPLE}\nUTIL\tpid_9998_luid_0x00000000_0x0000cafe_phys_0_eng_2_engtype_3d\t95.0\nUTIL\tpid_9999_luid_0x00000000_0x0000cafe_phys_0_eng_3_engtype_3d\t95.0\nGPU\tOther GPU\t4293918720\t1.0" + ); + let metrics = parse_metrics(&two_adapters, &[]); + assert_eq!(metrics.len(), 2); + let gpu = metrics + .iter() + .find(|g| g.vram_used_mb == 0) + .expect("the cafe device's metrics"); + assert_eq!(gpu.gpu_utilization_pct, 100.0); + } + + #[test] + #[allow(clippy::float_cmp)] + fn single_adapter_falls_back_to_wmi_vram_when_adlx_reading_count_mismatches() { + // WMI's AdapterRAM here is also at the truncation-cap threshold, so + // the fallback value itself degrades to 0 per `wmi_vram_total_mb` — + // see `single_adapter_falls_back_to_wmi_vram_below_cap_threshold` + // for the case where the WMI fallback value is actually trusted. + let sample = "UTIL\tluid_0x00000000_0x0000b3a4_phys_0_eng_0_engtype_3d\t1.0\nVRAM\tluid_0x00000000_0x0000b3a4_phys_0\t1048576\nGPU\tDiscrete GPU\t4293918720\t1.0"; + let two_adlx = [ + AdlxReading { + temperature_c: 50.0, + power_w: 10.0, + clock_mhz: Some(1800.0), + total_vram_mb: Some(8192), + }, + AdlxReading { + temperature_c: 40.0, + power_w: 8.0, + clock_mhz: Some(1200.0), + total_vram_mb: Some(4096), + }, + ]; + let metrics = parse_metrics(sample, &two_adlx); + assert_eq!(metrics.len(), 1); + assert_eq!(metrics[0].vram_total_mb, 0); + assert_eq!(metrics[0].temperature_c, 0.0); + assert_eq!(metrics[0].power_w, 0.0); + assert_eq!(metrics[0].clock_mhz, None); + } + + #[test] + fn single_adapter_falls_back_to_wmi_vram_below_cap_threshold() { + // No ADLX reading at all, and the WMI value is comfortably below the + // truncation-cap threshold, so it's trusted as-is. + let sample = "UTIL\tluid_0x00000000_0x0000b3a4_phys_0_eng_0_engtype_3d\t1.0\nVRAM\tluid_0x00000000_0x0000b3a4_phys_0\t1048576\nGPU\tDiscrete GPU\t3221225472\t1.0"; + let metrics = parse_metrics(sample, &[]); + assert_eq!(metrics.len(), 1); + assert_eq!(metrics[0].vram_total_mb, 3_221_225_472 / (1024 * 1024)); + } + + #[test] + fn single_adapter_adlx_reading_with_no_total_vram_falls_back_to_wmi() { + // The ADLX reading matches (exactly one, for exactly one real + // adapter) but its own TotalVRAM query failed (`None`) — still + // falls back to the WMI value rather than reporting 0. + let sample = "UTIL\tluid_0x00000000_0x0000b3a4_phys_0_eng_0_engtype_3d\t1.0\nVRAM\tluid_0x00000000_0x0000b3a4_phys_0\t1048576\nGPU\tDiscrete GPU\t3221225472\t1.0"; + let adlx = [AdlxReading { + temperature_c: 50.0, + power_w: 10.0, + clock_mhz: Some(1800.0), + total_vram_mb: None, + }]; + let metrics = parse_metrics(sample, &adlx); + assert_eq!(metrics.len(), 1); + assert_eq!(metrics[0].vram_total_mb, 3_221_225_472 / (1024 * 1024)); + } + + #[test] + fn device_key_without_luid_substring_falls_back_to_whole_instance() { + // Defensive fallback for an unexpected instance-name shape (no + // `luid_` segment at all) — must not panic, and should still key + // consistently rather than silently merging unrelated instances. + assert_eq!( + device_key("some_unusual_instance_name"), + "some_unusual_instance_name" + ); + } + + #[test] + fn empty_output_yields_no_metrics() { + assert!(parse_metrics("", &[]).is_empty()); + assert_eq!(parse_system_info("").physical_gpu_count, 0); + } + + #[test] + fn apply_adlx_falls_back_to_defaults_when_missing() { + assert_eq!(apply_adlx(None), (0.0, 0.0, None)); + } + + #[test] + fn apply_adlx_uses_reading_when_present() { + let reading = AdlxReading { + temperature_c: 65.0, + power_w: 12.5, + clock_mhz: Some(1900.0), + total_vram_mb: Some(16384), + }; + assert_eq!(apply_adlx(Some(&reading)), (65.0, 12.5, Some(1900.0))); + } + + // adlx_readings() itself (the real ADLX FFI chain) is Windows-only and + // hardware/driver-dependent — not unit-testable here; CI's + // windows-build-and-test job covers compilation of the #[cfg(windows)] + // path, and apply_adlx above covers the pure mapping logic. +} diff --git a/crates/rocm-dash-tui/Cargo.toml b/crates/rocm-dash-tui/Cargo.toml index 251b33251..1631f865c 100644 --- a/crates/rocm-dash-tui/Cargo.toml +++ b/crates/rocm-dash-tui/Cargo.toml @@ -16,6 +16,11 @@ path = "src/lib.rs" [dependencies] rocm-dash-core = { path = "../rocm-dash-core" } +# Unconditional dependency, but only exercised by `client.rs`'s +# `#[cfg(windows)]` arm: the in-process metrics ticker there reuses +# `HostCollector`/`WinPerfCollector` instead of a socket (no rocmdashd on +# Windows yet). +rocm-dash-collectors = { path = "../rocm-dash-collectors" } rocm-deps = { path = "../rocm-deps" } # Unified dashboard TUI base: ratatui 0.30 + crossterm 0.28. This is the only # ratatui in the workspace — the `rocm` binary no longer carries a (dead) 0.29 diff --git a/crates/rocm-dash-tui/src/app/event_loop.rs b/crates/rocm-dash-tui/src/app/event_loop.rs index d7604cdb4..dd6f581e8 100644 --- a/crates/rocm-dash-tui/src/app/event_loop.rs +++ b/crates/rocm-dash-tui/src/app/event_loop.rs @@ -1041,6 +1041,7 @@ async fn event_loop(terminal: &mut Tui, args: &ResolvedArgs) -> color_eyre::Resu state.latest = None; } Some(ClientMsg::Event(ev)) => state.apply_event(*ev), + Some(ClientMsg::HostGpuSnapshot(snap)) => state.push_host_gpu_snapshot(*snap), Some(ClientMsg::ReplaySeek) => state.reset_for_seek(), Some(ClientMsg::ReplayPosition { elapsed_s, total_s }) => { if let Some(r) = state.replay.as_mut() { diff --git a/crates/rocm-dash-tui/src/app/mod.rs b/crates/rocm-dash-tui/src/app/mod.rs index f3c0123bd..42a6909d4 100644 --- a/crates/rocm-dash-tui/src/app/mod.rs +++ b/crates/rocm-dash-tui/src/app/mod.rs @@ -649,12 +649,14 @@ impl AppState { /// populated once that snapshot arrives (see `push_snapshot`), so a `q` /// pressed in that startup window would otherwise see this return `false` /// even though a model actually was being served, simply because the live - /// state hadn't arrived yet. On a host whose dashboard-embedded daemon - /// client never connects at all (Windows — `client.rs`'s connect path is - /// unix-only), `has_received_snapshot` never flips, so the live read - /// stays authoritative for the entire session rather than only a brief - /// startup window — the registry read is cheap and re-run every time this - /// is called, so there is no staleness to outlive. Once a snapshot lands + /// state hadn't arrived yet. On Windows, `client.rs`'s `#[cfg(windows)]` + /// arm does connect and stream live GPU/CPU/memory every tick, but by + /// design never sends an instance snapshot (there's no `rocmdashd` unix + /// socket to source one from), so `has_received_snapshot` never flips + /// there either — the live read stays authoritative for the entire + /// session rather than only a brief startup window — the registry read is + /// cheap and re-run every time this is called, so there is no staleness + /// to outlive. Once a snapshot lands /// on a host where the daemon does connect, `instances` is authoritative /// and the read's *result* is no longer consulted for THIS question (a /// model that legitimately stops later in the session is still reflected @@ -1482,6 +1484,27 @@ impl AppState { self.instances .insert(inst.container_id.clone(), inst.clone()); } + self.push_history_and_latest(snap); + } + + /// Windows sibling of [`push_snapshot`](Self::push_snapshot) for + /// `ClientMsg::HostGpuSnapshot`: applies host/GPU metrics for + /// charts/history exactly like a real snapshot, but deliberately leaves + /// `instances` and `has_received_snapshot` untouched. The Windows + /// in-process ticker (`client.rs`) always sends an empty `instances` + /// stand-in (no instance-registry daemon exists there yet), so treating + /// it as authoritative the way `push_snapshot` does would permanently + /// blind the issue #145 quit-confirm prompt to models actually being + /// served — see `has_live_instance_with`'s Windows note. + pub(crate) fn push_host_gpu_snapshot(&mut self, snap: Snapshot) { + self.push_history_and_latest(snap); + } + + /// Shared tail of [`push_snapshot`](Self::push_snapshot) and + /// [`push_host_gpu_snapshot`](Self::push_host_gpu_snapshot): the + /// history/latest bookkeeping both share, independent of how (or + /// whether) `instances` gets updated. + fn push_history_and_latest(&mut self, snap: Snapshot) { if self.history.len() == HISTORY_CAP { self.history.pop_front(); } @@ -3429,6 +3452,51 @@ mod tests { ); } + #[test] + fn push_host_gpu_snapshot_leaves_instances_and_has_received_snapshot_untouched() { + // Mirrors push_snapshot's issue #145 authority handoff, but for the + // Windows in-process path: a HostGpuSnapshot must still land + // host/GPU metrics for charts/history, while leaving `instances` and + // `has_received_snapshot` exactly as they were — see + // `push_host_gpu_snapshot`'s doc comment for why. + let mut s = st(); + s.instances.insert( + "id0".to_string(), + rocm_dash_core::metrics::Instance { + container_id: "id0".to_string(), + ..Default::default() + }, + ); + assert!(!s.has_received_snapshot); + + let snap = Snapshot { + warnings: vec!["gpu-warning".to_string()], + instances: vec![rocm_dash_core::metrics::Instance { + container_id: "phantom".to_string(), + ..Default::default() + }], + ..Snapshot::default() + }; + s.push_host_gpu_snapshot(snap); + + assert!( + !s.has_received_snapshot, + "a HostGpuSnapshot must never flip has_received_snapshot" + ); + assert_eq!( + s.instances.len(), + 1, + "instances must stay exactly as seeded, ignoring the snapshot's own (always-empty-in-practice) instances" + ); + assert!(s.instances.contains_key("id0")); + assert_eq!( + s.latest.as_ref().map(|s| s.warnings.clone()), + Some(vec!["gpu-warning".to_string()]), + "host/GPU metrics must still land for charts/history" + ); + assert_eq!(s.history.len(), 1); + } + #[test] fn has_live_instance_is_always_false_under_simulated() { // `--demo`/`--replay` synthesize `Instance { status: Running, .. }` diff --git a/crates/rocm-dash-tui/src/app/types.rs b/crates/rocm-dash-tui/src/app/types.rs index 34e79a7ad..2c383757e 100644 --- a/crates/rocm-dash-tui/src/app/types.rs +++ b/crates/rocm-dash-tui/src/app/types.rs @@ -172,6 +172,16 @@ pub enum ConnState { }, } +impl ConnState { + /// True for the Windows in-process collector (see + /// `client::connect_and_run`'s `#[cfg(windows)]` arm), which tags its + /// `daemon_version` this way since there's no real `rocmdashd` behind + /// it — and so no instance registry it could ever populate. + pub fn is_in_process(&self) -> bool { + matches!(self, Self::Connected { version, .. } if version.starts_with("in-process/")) + } +} + #[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] pub enum ActiveTab { // 5-tab IA. Home is the default; ROCm and Serving are the two domain tabs diff --git a/crates/rocm-dash-tui/src/client.rs b/crates/rocm-dash-tui/src/client.rs index bd5e50b3f..ab2ad8117 100644 --- a/crates/rocm-dash-tui/src/client.rs +++ b/crates/rocm-dash-tui/src/client.rs @@ -6,13 +6,16 @@ //! forwards everything (including connect/disconnect transitions) to the app loop //! via an mpsc channel. -// On Windows the connection is a #[cfg(unix)] stub, so the unix-socket imports -// below are only used on unix; silence the resulting unused-import noise there. +// On Windows there's no `rocmdashd` unix socket to connect to, so the +// `#[cfg(windows)]` arm builds snapshots in-process instead (see +// `connect_and_run`); the unix-socket imports below are only used on unix, +// so silence the resulting unused-import noise there. #![cfg_attr(windows, allow(unused_imports))] use std::path::PathBuf; use anyhow::{Context, anyhow}; +use rocm_dash_core::metrics::Snapshot; use rocm_dash_core::protocol::{Command, Event, PROTOCOL_VERSION}; use tokio::io::{AsyncWriteExt, BufReader}; use tokio::sync::mpsc::UnboundedSender; @@ -33,6 +36,15 @@ pub enum ClientMsg { reason: String, }, Event(Box), + /// Windows in-process synthetic tick (`#[cfg(windows)] connect_and_run`): + /// host/GPU metrics sourced from local collectors, not a real daemon. + /// Deliberately NOT `Event(Snapshot)` — that path treats `instances` as + /// authoritative truth and latches `has_received_snapshot` forever, but + /// this snapshot's `instances` is always an empty stand-in (Windows has + /// no instance registry daemon yet), so routing it through there would + /// permanently blind the issue #145 quit-confirm prompt to models + /// actually being served. See `AppState::push_host_gpu_snapshot`. + HostGpuSnapshot(Box), /// Replay-only: the scrubber jumped backward. The TUI must wipe any /// state derived from past events before the burst-emitted events for /// the new playhead position arrive. @@ -178,12 +190,65 @@ async fn connect_and_run(connect: &str, tx: UnboundedSender) -> anyho Ok(()) } +/// Windows has no rocmdashd socket yet, so this builds `Snapshot`s in-process +/// instead of connecting anywhere: `HostCollector` (already cross-platform) +/// for CPU/memory, `WinPerfCollector` (OS perf counters, no AMD tool needed) +/// for GPU. Sends `Connected` like the unix arm, but then a stream of +/// `ClientMsg::HostGpuSnapshot` rather than `Event::Snapshot` — deliberately +/// not routed through `Event::Snapshot`, to avoid disturbing the "quit while +/// serving" confirmation prompt (see `AppState::push_host_gpu_snapshot`). #[cfg(windows)] -// Mirrors the signature of the `cfg(unix)` arm, which callers `.await`; -// dropping `async` here would not compile on Windows. -#[allow(clippy::unused_async)] -async fn connect_and_run(connect: &str, _tx: UnboundedSender) -> anyhow::Result<()> { - Err(anyhow!( - "rocm-dash TUI requires Unix domain sockets; not supported on Windows yet (connect={connect})" - )) +async fn connect_and_run(connect: &str, tx: UnboundedSender) -> anyhow::Result<()> { + use rocm_dash_collectors::host::HostCollector; + use rocm_dash_collectors::winperf::WinPerfCollector; + use rocm_dash_core::metrics::GpuSystemInfo; + + info!( + connect, + "windows: collecting metrics in-process (no daemon socket)" + ); + let _ = tx.send(ClientMsg::Connected { + host: "localhost".into(), + daemon_version: format!("in-process/{}", env!("CARGO_PKG_VERSION")), + }); + + let mut host = HostCollector::new(); + let gpu = WinPerfCollector::new(); + // Mirrors the daemon's `gpu_tick` cadence (rocm_dash_daemon::runner::RunnerOptions::default()). + let mut ticker = tokio::time::interval(std::time::Duration::from_secs(1)); + // A slow powershell/ADLX tick (see `WinPerfCollector::snapshot`) must not + // queue up a burst of catch-up ticks once it returns; matches + // `rocm-dash-daemon/src/runner.rs`'s existing explicit choice. + ticker.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); + + loop { + ticker.tick().await; + let host_metrics = host.tick(); + let (gpus, gpu_system_info, warnings) = match gpu.snapshot().await { + Ok(v) => v, + Err(e) => { + warn!(error = %e, "windows GPU perf-counter query failed"); + ( + Vec::new(), + GpuSystemInfo::default(), + vec![format!("windows GPU perf-counter query failed: {e}")], + ) + } + }; + let snapshot = Snapshot { + timestamp: chrono::Utc::now(), + host: host_metrics, + gpus, + gpu_system_info: Some(gpu_system_info), + instances: Vec::new(), + warnings, + }; + if tx + .send(ClientMsg::HostGpuSnapshot(Box::new(snapshot))) + .is_err() + { + break; + } + } + Ok(()) } diff --git a/crates/rocm-dash-tui/src/registry_probe.rs b/crates/rocm-dash-tui/src/registry_probe.rs index 78d0ef1a5..87614553d 100644 --- a/crates/rocm-dash-tui/src/registry_probe.rs +++ b/crates/rocm-dash-tui/src/registry_probe.rs @@ -15,8 +15,10 @@ //! read would freeze out two real cases: a model started mid-session via the //! Serve wizard (its id would never be in a launch-time snapshot, so it would //! never get a stop command or the `s`-to-open-Services hint), and a host -//! whose dashboard-embedded daemon client never connects at all (Windows — -//! `has_received_snapshot` never flips, so `AppState::instances` stays empty +//! whose daemon client never sends an instance snapshot (Windows — it +//! connects and streams live GPU/CPU/memory every tick, but has no +//! `rocmdashd` socket to source an instance list from, so +//! `has_received_snapshot` never flips and `AppState::instances` stays empty //! for the life of the session). A live probe has neither problem — there is //! no window for it to go stale in. diff --git a/crates/rocm-dash-tui/src/ui/mod.rs b/crates/rocm-dash-tui/src/ui/mod.rs index 35848f94b..441cb57c5 100644 --- a/crates/rocm-dash-tui/src/ui/mod.rs +++ b/crates/rocm-dash-tui/src/ui/mod.rs @@ -1102,9 +1102,11 @@ mod tests { #[test] fn quit_confirm_body_prints_the_stop_command_during_the_startup_fallback_when_the_registry_names_an_id() { - // On a host whose dashboard-embedded daemon client never connects at - // all (Windows — `has_received_snapshot` never flips for the whole - // session), `instances` never fills, so without this the quit prompt + // On a host whose daemon client never sends an instance snapshot + // (Windows — it connects and streams live GPU/CPU/memory every tick, + // but has no `rocmdashd` socket to source an instance list from, so + // `has_received_snapshot` never flips for the whole session), + // `instances` never fills, so without this the quit prompt // could only ever print the generic "a model was serving" sentence — // no model name (expected: `instances` is the only source for that), // but also no stop command and no id, even though the live registry diff --git a/crates/rocm-dash-tui/src/ui/tabs/home.rs b/crates/rocm-dash-tui/src/ui/tabs/home.rs index a3d62bf3a..c9d243399 100644 --- a/crates/rocm-dash-tui/src/ui/tabs/home.rs +++ b/crates/rocm-dash-tui/src/ui/tabs/home.rs @@ -23,13 +23,27 @@ use ratatui::widgets::Paragraph; use rocm_dash_core::state::JobStatus; -use crate::app::{AppState, UpdateStatus}; +use crate::app::{AppState, ConnState, UpdateStatus}; use crate::ui::format; use crate::ui::gradient::GradientGauge; use crate::ui::panel::{self, BoxRole}; use crate::ui::sparkline::BrailleSparkline; use crate::ui::theme::Theme; +/// Summed VRAM used / summed VRAM total across all GPUs, as a percentage. +/// `total == 0` is a legitimate state (e.g. the Windows collector can't +/// correlate a multi-adapter host to specific VRAM sizes) — must read as +/// `0.0`, not `used / 1 * 100`. +fn vram_used_pct(snap: &rocm_dash_core::metrics::Snapshot) -> f64 { + let used: u64 = snap.gpus.iter().map(|g| g.vram_used_mb).sum(); + let total: u64 = snap.gpus.iter().map(|g| g.vram_total_mb).sum(); + if total > 0 { + used as f64 / total as f64 * 100.0 + } else { + 0.0 + } +} + /// History series helper: map each snapshot to a 0..=100 magnitude via `pick`. fn history f64>( state: &AppState, @@ -54,6 +68,28 @@ const fn node_load_label(hist_empty: bool, simulated: bool) -> &'static str { } } +/// True until the first snapshot has arrived at all. Drives only the hero +/// title's "Detecting GPU…" placeholder — a snapshot that arrived but +/// carries no GPUs (or a legitimately-zero field) is a known state, not a +/// still-loading one, and reads "Unknown GPU" instead (see +/// `node_throughput_title`). Gated on the connection still being in its +/// initial handshake: `latest` is also wiped on disconnect +/// (`ClientMsg::Disconnected`, including end-of-`--replay`) and on a replay +/// seek, neither of which means detection restarted — those read "Unknown +/// GPU" too rather than reverting to "still loading". For the numeric "—" +/// placeholders, see `gpu_data_unavailable`. +const fn gpu_telemetry_pending(state: &AppState) -> bool { + state.latest.is_none() && matches!(state.conn, ConnState::Initial | ConnState::Connecting) +} + +/// True when there's no usable per-GPU data to show a number for: either no +/// snapshot has arrived yet, or one arrived but carries no GPUs at all. +/// Drives the headline util/vram/temp/power "—" placeholders — distinct +/// from `gpu_telemetry_pending`, which only gates the hero title. +fn gpu_data_unavailable(state: &AppState) -> bool { + state.latest.as_ref().is_none_or(|s| s.gpus.is_empty()) +} + /// Peak GPU utilization across the GPUs in a snapshot (the headline metric). fn snap_util(s: &rocm_dash_core::metrics::Snapshot) -> f64 { s.gpus @@ -108,9 +144,14 @@ fn mini_spark( } /// Hero card title in the mock's form: `Node throughput · N × MODEL · ROCm V`. -/// Falls back to `Unknown GPU` when no GPU model is detected, and drops the -/// count/ROCm segments that aren't known rather than inventing them. +/// Falls back to `Detecting GPU…` while no telemetry has arrived at all yet +/// (see `gpu_telemetry_pending`), or to `Unknown GPU` when telemetry arrived +/// but didn't carry a model name — and drops the count/ROCm segments that +/// aren't known rather than inventing them. fn node_throughput_title(state: &AppState) -> String { + if gpu_telemetry_pending(state) { + return "Node throughput · Detecting GPU…".to_string(); + } let info = state .latest .as_ref() @@ -261,7 +302,11 @@ fn draw_hero_left(f: &mut Frame, area: Rect, state: &AppState, theme: &Theme) { lh[0], ); let util = state.latest.as_ref().map_or(0.0, snap_util); - let util_label = format::pct(util as f32); + let util_label = if gpu_data_unavailable(state) { + "—".to_string() + } else { + format::pct(util as f32) + }; let g = GradientGauge::new(util / 100.0) .stops(theme.ok, theme.warn, theme.err) .track_bg(theme.surface_2) @@ -348,11 +393,9 @@ fn draw_hero_right(f: &mut Frame, area: Rect, state: &AppState, theme: &Theme) { rh[0], ); let latest = state.latest.as_ref(); - let vram = latest.map_or(0.0, |s| { - let used: u64 = s.gpus.iter().map(|g| g.vram_used_mb).sum(); - let total: u64 = s.gpus.iter().map(|g| g.vram_total_mb).sum::().max(1); - used as f64 / total as f64 * 100.0 - }); + let unavailable = gpu_data_unavailable(state); + let vram = latest.map_or(0.0, vram_used_pct); + let vram_total: u64 = latest.map_or(0, |s| s.gpus.iter().map(|g| g.vram_total_mb).sum()); let temp = latest.map_or(0.0, |s| { s.gpus .iter() @@ -362,16 +405,35 @@ fn draw_hero_right(f: &mut Frame, area: Rect, state: &AppState, theme: &Theme) { let power: f64 = latest.map_or(0.0, |s| { s.gpus.iter().map(|g| f64::from(g.power_w)).sum::() }); + // While there's no GPU telemetry at all, show "—" rather than 0%/0°C/0W + // — those look like real (broken) readings rather than a loading/unknown + // state. The same applies per-metric once data has arrived but a given + // metric reads exactly zero: a multi-adapter Windows host can't + // correlate VRAM totals per-device (`vram_total_mb == 0`, see + // `vram_used_pct`), and temp/power read 0.0 with no ADLX reading — a + // real GPU is never actually at 0°C/0W, so an exact zero there means + // "not measured", not "measured as zero". + let vram_label = if unavailable || vram_total == 0 { + "—".to_string() + } else { + format::pct(vram as f32) + }; + let temp_label = if unavailable || temp == 0.0 { + "—".to_string() + } else { + format::celsius(temp as f32) + }; + let power_label = if unavailable || power == 0.0 { + "—".to_string() + } else { + format::watts(power as f32) + }; mini_spark( f, rh[1], "VRAM ", - &format::pct(vram as f32), - &history(state, |s| { - let used: u64 = s.gpus.iter().map(|g| g.vram_used_mb).sum(); - let total: u64 = s.gpus.iter().map(|g| g.vram_total_mb).sum::().max(1); - used as f64 / total as f64 * 100.0 - }), + &vram_label, + &history(state, vram_used_pct), false, theme, ); @@ -379,7 +441,7 @@ fn draw_hero_right(f: &mut Frame, area: Rect, state: &AppState, theme: &Theme) { f, rh[2], "TEMP ", - &format::celsius(temp as f32), + &temp_label, &history(state, |s| { s.gpus .iter() @@ -393,7 +455,7 @@ fn draw_hero_right(f: &mut Frame, area: Rect, state: &AppState, theme: &Theme) { f, rh[3], "POWER", - &format::watts(power as f32), + &power_label, // Power can exceed 100; scale to a nominal 1kW ceiling for the trace. &history(state, |s| { s.gpus.iter().map(|g| f64::from(g.power_w)).sum::() / 10.0 @@ -675,10 +737,198 @@ mod tests { assert!(t.starts_with("Node throughput · "), "prefix: {t}"); assert!(t.contains("Instinct MI355X"), "model: {t}"); assert!(t.contains("ROCm 6.2"), "rocm version: {t}"); - // No telemetry → Unknown GPU, no fabricated count/version. + // No telemetry at all yet → "detecting", not a bogus "Unknown GPU". let empty = AppState::new("t".into(), "default-dark".into()); let t2 = node_throughput_title(&empty); - assert_eq!(t2, "Node throughput · Unknown GPU"); + assert_eq!(t2, "Node throughput · Detecting GPU…"); + } + + #[test] + fn node_throughput_title_unknown_gpu_only_when_telemetry_present_but_unnamed() { + // Telemetry arrived (a GPU is present) but carried no model name — + // distinct from "nothing has arrived yet" and must still read + // "Unknown GPU", not the pending/detecting placeholder. + let mut s = AppState::new("t".into(), "default-dark".into()); + s.latest = Some(Snapshot { + host: SystemMetrics::default(), + gpus: vec![GpuMetrics { + device_id: "GPU0".into(), + vram_used_mb: 0, + vram_total_mb: 0, + gpu_utilization_pct: 0.0, + temperature_c: 0.0, + power_w: 0.0, + clock_mhz: None, + }], + gpu_system_info: None, + ..Default::default() + }); + assert_eq!(node_throughput_title(&s), "Node throughput · Unknown GPU"); + } + + #[test] + fn gpu_telemetry_pending_only_tracks_whether_anything_arrived() { + // Nothing has arrived yet → pending (drives the hero title). + let empty = AppState::new("t".into(), "default-dark".into()); + assert!(gpu_telemetry_pending(&empty)); + // A snapshot with no GPUs at all is a known state, not "still + // loading" — the title falls back to "Unknown GPU", not "Detecting". + let mut no_gpus = AppState::new("t".into(), "default-dark".into()); + no_gpus.latest = Some(Snapshot::default()); + assert!(!gpu_telemetry_pending(&no_gpus)); + assert!(!gpu_telemetry_pending(&state_with_gpu())); + } + + #[test] + fn gpu_data_unavailable_distinguishes_no_data_from_legitimate_zero() { + // Nothing has arrived yet → unavailable. + let empty = AppState::new("t".into(), "default-dark".into()); + assert!(gpu_data_unavailable(&empty)); + // A snapshot with no GPUs at all → still unavailable (no number to show). + let mut no_gpus = AppState::new("t".into(), "default-dark".into()); + no_gpus.latest = Some(Snapshot::default()); + assert!(gpu_data_unavailable(&no_gpus)); + // A real GPU present (even with legitimately-zero fields) → available. + assert!(!gpu_data_unavailable(&state_with_gpu())); + } + + #[test] + fn hero_shows_dash_placeholders_not_fake_zeros_when_metrics_are_unmeasured() { + // A real GPU is present (so the title isn't "Detecting GPU…"), but + // vram_total/temp/power all read exactly zero — e.g. a Windows + // multi-adapter host that can't correlate per-device VRAM totals, + // or no ADLX reading for temp/power. Each must show "—", not a + // fake-looking "0%"/"0°C"/"0W". + let mut s = AppState::new("t".into(), "default-dark".into()); + s.active_tab = ActiveTab::Home; + s.latest = Some(Snapshot { + host: SystemMetrics::default(), + gpus: vec![GpuMetrics { + device_id: "GPU0".into(), + vram_used_mb: 0, + vram_total_mb: 0, + gpu_utilization_pct: 12.0, + temperature_c: 0.0, + power_w: 0.0, + clock_mhz: None, + }], + gpu_system_info: Some(GpuSystemInfo { + gpu_model: "Some GPU".into(), + ..Default::default() + }), + ..Default::default() + }); + let out = render(&s, 160, 30); + // The buffer is row-major with no row separators, so bound each + // check to the segment between consecutive labels — otherwise an + // unrelated "0%"-shaped substring earlier in the row (e.g. the + // "12.0%" utilization gauge) would false-positive the negative + // assertions below. + let vram_segment = out + .split("VRAM") + .nth(1) + .and_then(|s| s.split("TEMP").next()); + let temp_segment = out + .split("TEMP") + .nth(1) + .and_then(|s| s.split("POWER").next()); + let power_segment = out + .split("POWER") + .nth(1) + .and_then(|s| s.split("T/S").next()); + let vram_segment = vram_segment.unwrap_or_else(|| panic!("vram row missing: {out:?}")); + let temp_segment = temp_segment.unwrap_or_else(|| panic!("temp row missing: {out:?}")); + let power_segment = power_segment.unwrap_or_else(|| panic!("power row missing: {out:?}")); + assert!( + vram_segment.contains('—'), + "vram should read —: {vram_segment:?}" + ); + assert!( + !vram_segment.contains('%'), + "vram should not show a fake 0%: {vram_segment:?}" + ); + assert!( + temp_segment.contains('—'), + "temp should read —: {temp_segment:?}" + ); + assert!( + !temp_segment.contains('°'), + "temp should not show a fake 0°C: {temp_segment:?}" + ); + assert!( + power_segment.contains('—'), + "power should read —: {power_segment:?}" + ); + assert!( + !power_segment.contains('W'), + "power should not show a fake 0W: {power_segment:?}" + ); + } + + #[test] + fn gpu_utilization_gauge_shows_dash_not_a_fake_zero_percent_when_no_gpus_are_reported() { + // A snapshot arrived but carries no GPUs at all (`gpu_data_unavailable` + // true) — the gauge label must read "—", not a fake "0.0%" + // (draw_hero_left's `gpu_data_unavailable` branch). + let mut s = AppState::new("t".into(), "default-dark".into()); + s.active_tab = ActiveTab::Home; + s.latest = Some(Snapshot::default()); + let out = render(&s, 160, 30); + let segment = out + .split("GPU UTILIZATION") + .nth(1) + .and_then(|s| s.split("tokens / watt").next()) + .unwrap_or_else(|| panic!("gpu utilization row missing: {out:?}")); + assert!( + segment.contains('—'), + "gpu utilization should read —, not a fake 0.0%: {segment:?}" + ); + assert!( + !segment.contains("0.0%"), + "gpu utilization should not show a fake 0.0%: {segment:?}" + ); + } + + #[test] + fn gpu_telemetry_pending_does_not_resume_after_a_disconnect_or_replay_end() { + // Once connected, losing the connection (end of `--replay`, a daemon + // drop, ClientMsg::Disconnected) wipes `latest` but must not look + // like detection restarted — the title should stay on "Unknown GPU", + // never flip back to "Detecting GPU…". + let mut s = AppState::new("t".into(), "default-dark".into()); + s.conn = ConnState::Connected { + host: "x".into(), + version: "1".into(), + }; + s.latest = Some(Snapshot::default()); + s.conn = ConnState::Disconnected { + reason: "end of recording".into(), + }; + s.latest = None; + assert!(!gpu_telemetry_pending(&s)); + assert_eq!(node_throughput_title(&s), "Node throughput · Unknown GPU"); + } + + #[test] + #[allow(clippy::float_cmp)] + fn vram_used_pct_is_zero_not_garbage_when_total_is_unknown() { + // vram_total_mb == 0 is a legitimate state (e.g. the Windows + // collector can't correlate a multi-adapter host to specific VRAM + // sizes) — must read as 0.0, not `used / 1 * 100` (would be 80800.0). + let snap = Snapshot { + host: SystemMetrics::default(), + gpus: vec![GpuMetrics { + device_id: "GPU0".into(), + vram_used_mb: 808, + vram_total_mb: 0, + gpu_utilization_pct: 0.5, + temperature_c: 0.0, + power_w: 0.0, + clock_mhz: None, + }], + ..Default::default() + }; + assert_eq!(vram_used_pct(&snap), 0.0); } #[test] diff --git a/crates/rocm-dash-tui/src/ui/tabs/instances.rs b/crates/rocm-dash-tui/src/ui/tabs/instances.rs index f7b493ef0..02daa89e1 100644 --- a/crates/rocm-dash-tui/src/ui/tabs/instances.rs +++ b/crates/rocm-dash-tui/src/ui/tabs/instances.rs @@ -375,11 +375,15 @@ const fn status_role(status: InstanceStatus) -> BoxRole { } fn draw_empty(f: &mut Frame, area: Rect, state: &AppState, theme: &Theme) { - let body = match &state.conn { - ConnState::Connected { .. } => { - "no instances · start daemon with --enable-docker on a host with vLLM containers" + let body = if state.conn.is_in_process() { + "no instance registry on this platform · vLLM containers aren't available here" + } else { + match &state.conn { + ConnState::Connected { .. } => { + "no instances · start daemon with --enable-docker on a host with vLLM containers" + } + _ => "waiting for daemon…", } - _ => "waiting for daemon…", }; let inner = panel::bento(f, area, Some("Instances"), BoxRole::Neutral, false, theme); let p = Paragraph::new(Line::from(Span::styled( @@ -1354,6 +1358,33 @@ mod tests { out } + #[test] + fn draw_empty_names_the_platform_reason_for_the_in_process_collector() { + use ratatui::Terminal; + use ratatui::backend::TestBackend; + + // The Windows in-process collector (see `ConnState::is_in_process`) + // has no instance registry at all, ever — distinct from "a real + // daemon is connected but nothing is running" or "still connecting". + let mut state = mk_state(HashMap::new(), 0); + state.conn = ConnState::Connected { + host: "localhost".into(), + version: "in-process/0.0.0".into(), + }; + let mut term = Terminal::new(TestBackend::new(80, 20)).unwrap(); + term.draw(|f| draw(f, f.area(), &state, &state.theme)) + .unwrap(); + let grid = buffer_text(&term); + assert!( + grid.contains("no instance registry on this platform"), + "should name the platform limitation, not claim a daemon could be started; got:\n{grid}" + ); + assert!( + !grid.contains("start daemon"), + "must not suggest starting a daemon that doesn't exist on this platform; got:\n{grid}" + ); + } + #[test] fn render_shows_quantization_and_vram_for_populated_instance() { use ratatui::Terminal; diff --git a/docs/manual-testing.md b/docs/manual-testing.md index 99810713c..220f1c7e4 100644 --- a/docs/manual-testing.md +++ b/docs/manual-testing.md @@ -531,14 +531,14 @@ Expected result: - A prompt appears naming the serving model and warning that it may still be running in the background after you quit, with the real `rocm services stop --yes` command (the actual id, not a - placeholder) to stop it first. From the dashboard (not the launcher) on a - host where its own daemon client never connects (Windows, for the whole - session), the prompt still shows that same stop command and the service - id, but not the model's name — the dashboard's own live view is where the - name comes from, and it never fills there. The launcher's own copy of the - prompt always names the model, Windows included — it reads the - managed-service registry directly rather than through the dashboard's - daemon connection. + placeholder) to stop it first. From the dashboard (not the launcher) on + Windows, where the dashboard streams live GPU/CPU/memory but never gets a + serving-instance snapshot (no `rocmdashd` unix socket there), the prompt + still shows that same stop command and the service id, but not the + model's name — the dashboard's own live view is where the name comes + from, and it never fills there. The launcher's own copy of the prompt + always names the model, Windows included — it reads the managed-service + registry directly rather than through the dashboard's daemon connection. - From the dashboard (not the launcher), the prompt also offers `s`: pressing it closes the prompt and opens the Services overlay, pre-focused on the serving model, so you can stop it there through the overlay's own diff --git a/docs/rocm-docs/install/installation.md b/docs/rocm-docs/install/installation.md index 4ee7ac5f8..3fb9ed7b5 100644 --- a/docs/rocm-docs/install/installation.md +++ b/docs/rocm-docs/install/installation.md @@ -15,7 +15,9 @@ ROCm CLI ships as a single prebuilt binary. Platform support: For WSL2 setup and diagnosis, see [ROCm CLI on WSL2](wsl.md). -Live dashboard telemetry requires Linux or WSL2 (see +Live dashboard GPU/CPU/memory telemetry works on Windows too; the +serving-instance registry view is Linux/WSL2 only, since there's no +`rocmdashd` unix socket on Windows (see [Interactive interfaces](../getting-started.md#interactive-interfaces)). vLLM serving is Linux or WSL2 only (see [vLLM adapter](../engines/vllm.md)). diff --git a/tests/e2e-cucumber/features/dash.feature b/tests/e2e-cucumber/features/dash.feature index cd82e409b..b5a427e79 100644 --- a/tests/e2e-cucumber/features/dash.feature +++ b/tests/e2e-cucumber/features/dash.feature @@ -600,3 +600,27 @@ Feature: Interactive dashboard When the user denies the adopt approval and closes onboarding setup And the user quits the dashboard Then the dashboard exits successfully + + # Exercises the hero's "no GPU telemetry" rendering (see + # `gpu_data_unavailable`/`node_throughput_title` in + # `crates/rocm-dash-tui/src/ui/tabs/home.rs`) with a *real* embedded daemon, + # not `--demo` (whose fixture always has 8 GPUs and can never reach this + # state). The real reason this is a stable, permanent state here rather + # than a race isn't "no AMD GPU" on the runner — it's that the daemon's + # mandatory `/dev/kfd` pre-flight (`amd_smi::device_accessible`, + # `preflight_passes`) fails for this harness process (no read access to the + # device node, or it isn't visible to it at all), the same way it would on + # a real host without render/video group membership. So `detect_with_binary` + # returns `None` and every snapshot reports zero GPUs, regardless of + # whether the GPU lane's runner itself has real hardware. The other half of + # that state machine, the transient "pending" placeholder shown before the + # first snapshot arrives at all, is not covered: it closes in well under a + # second and there's no hook to freeze the daemon mid-handshake, so + # asserting on it would be asserting on a race. + @id:dash-reports-no-gpu-telemetry @requires-os:linux + Scenario: dash-37 - The hero shows placeholders, not fake zeros, when no GPU is detected + When the user opens the dashboard + Then the dashboard home view is displayed + And the hero reports no GPU telemetry available + When the user quits the dashboard + Then the dashboard exits successfully diff --git a/tests/e2e-cucumber/tests/e2e/dash_steps.rs b/tests/e2e-cucumber/tests/e2e/dash_steps.rs index bd979c75a..fa72f8ef7 100644 --- a/tests/e2e-cucumber/tests/e2e/dash_steps.rs +++ b/tests/e2e-cucumber/tests/e2e/dash_steps.rs @@ -1284,6 +1284,46 @@ async fn home_view_displayed(world: &mut E2eWorld) { ); } +#[then("the hero reports no GPU telemetry available")] +async fn hero_reports_no_gpu_telemetry(world: &mut E2eWorld) { + let tui = session(world); + // The VRAM/TEMP/POWER mini-sparks only draw in the hero's wide layout + // (`draw_hero_band`'s `hero.width >= 8 && hero.height >= 6` gate). + tui.use_wide_size() + .unwrap_or_else(|e| panic!("failed to enlarge the dashboard: {e}")); + + // `use_wide_size` only resizes the pty; the TUI redraws asynchronously + // once it processes the resulting SIGWINCH. Wait for that redraw by + // polling for the hero's wide-layout content, not just "Unknown GPU" + // (already visible pre-resize from the title bar alone). + tui.wait_for_screen_where( + "the hero's wide VRAM/TEMP/POWER layout", + |screen| screen.contains("Unknown GPU") && screen.contains("VRAM"), + default_timeout(), + ) + .await + .unwrap_or_else(|e| panic!("the no-GPU hero did not render: {e}")); + let screen = tui.screen_text(); + for label in ["VRAM", "TEMP", "POWER"] { + let row = screen + .lines() + .find(|line| line.contains(label)) + .unwrap_or_else(|| panic!("no {label} row on screen:\n{screen}")); + // Check the text *after* the label, not the whole row: the + // utilization gauge on the same row draws its own "—" placeholder, + // which would make this pass even if this mini-spark's value were a + // fake reading instead of the placeholder. + let after_label = row.split_once(label).map_or_else( + || panic!("no text after {label} on row:\n{row}"), + |(_, rest)| rest, + ); + assert!( + after_label.contains('—'), + "{label} row should show the \"—\" placeholder, not a fake reading:\n{row}" + ); + } +} + #[then("the dashboard reports live GPU telemetry")] async fn gpu_telemetry_displayed(world: &mut E2eWorld) { let tui = session(world); diff --git a/xtask/src/crate_edges.rs b/xtask/src/crate_edges.rs index 8c931ad66..fe7b1ec19 100644 --- a/xtask/src/crate_edges.rs +++ b/xtask/src/crate_edges.rs @@ -57,6 +57,7 @@ const ALLOWLIST: &[(&str, &str)] = &[ ("rocm-dash-collectors", "rocm-dash-core"), ("rocm-dash-daemon", "rocm-dash-core"), ("rocm-dash-daemon", "rocm-dash-collectors"), + ("rocm-dash-tui", "rocm-dash-collectors"), ("rocm-dash-tui", "rocm-dash-core"), ("rocm-dash-tui", "rocm-deps"), ("rocm-engine-lemonade", "rocm-core"),