Add GPU memory usage retrieval and update language strings for memory usage display#1053
Conversation
Coverage Report
File CoverageNo changed files found. |
There was a problem hiding this comment.
Pull request overview
This PR adds runtime GPU memory usage retrieval for macOS systems and updates the language strings to be more concise. The implementation fetches GPU memory usage via a new backend command and conditionally displays it when static memory size information is unavailable.
Changes:
- Added GPU memory usage fetching with 3-second polling interval
- Updated English and Japanese translation strings for GPU memory display labels (removing "Shared GPU" prefix and fixing trailing space)
- Enhanced GPU info display logic to show dynamic memory usage as fallback when static memory size is unavailable
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/lang/en.json | Updated translation key to simplify "memorySizeSharedUsage" label and fix trailing space |
| src/lang/ja.json | Updated Japanese translation to match simplified English label |
| src/features/hardware/dashboard/components/DashboardItems.tsx | Added GPU memory usage state, polling logic via useEffect, and conditional rendering based on available data |
Co-authored-by: Copilot <[email protected]>
| const fetchGpuMemoryUsage = async () => { | ||
| const result = await commands.getGpuMemoryUsage(); | ||
| if (isError(result)) { | ||
| console.error("Failed to fetch GPU memory usage:", result); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The error handling here is inconsistent with the established pattern in this codebase. Other similar command calls in hooks like useHardwareInfoAtom.ts (lines 22-25) and useProcessInfo.ts use the useTauriDialog().error() method to show user-facing error dialogs instead of just console.error(). This provides better user feedback when GPU memory usage cannot be retrieved.
Consider using the established error handling pattern by extracting error from useTauriDialog() and calling it when an error occurs.
| {(() => { | ||
| const hasMemorySize = gpu.memorySize !== "N/A"; | ||
| const hasMemoryUsage = gpuMemoryUsage?.inUseBytes != null; | ||
| const memorySizeDisplay = hasMemorySize | ||
| ? gpu.memorySize | ||
| : hasMemoryUsage | ||
| ? (gpuMemoryUsage?.inUseBytes ?? "N/A") | ||
| : "N/A"; | ||
| const memorySizeLabel = hasMemorySize | ||
| ? t("shared.memorySize") | ||
| : hasMemoryUsage | ||
| ? t("shared.memorySizeSharedUsage") | ||
| : t("shared.memorySize"); | ||
|
|
||
| const showCoreCount = | ||
| gpu.memorySizeDedicated === "N/A" && os === "macos"; | ||
| const dedicatedMemoryDisplay = showCoreCount | ||
| ? (gpu.coreCount ?? "N/A") | ||
| : gpu.memorySizeDedicated; | ||
| const dedicatedMemoryLabel = showCoreCount | ||
| ? t("shared.coreCount") | ||
| : t("shared.memorySizeDedicated"); | ||
|
|
||
| return ( | ||
| <InfoTable | ||
| data={{ | ||
| [t("shared.name")]: gpu.name, | ||
| [t("shared.vendor")]: gpu.vendorName, | ||
| [memorySizeLabel]: memorySizeDisplay, | ||
| [dedicatedMemoryLabel]: dedicatedMemoryDisplay, | ||
| }} | ||
| /> | ||
| ); | ||
| })()} |
There was a problem hiding this comment.
The GPU memory usage is fetched globally (once per component), but the component renders multiple GPUs in a loop. The getGpuMemoryUsage command documentation indicates it returns memory usage for the GPU, but when there are multiple GPUs (hardwareInfo.gpus is an array), this global gpuMemoryUsage state is being applied to each GPU individually in the map() function.
According to the backend documentation in bindings.ts, the command is best-effort and currently only implemented for macOS, so it may return data for only one GPU. This could lead to incorrect memory usage display when multiple GPUs are present. Consider either: (1) documenting that memory usage only applies to the first/primary GPU, or (2) matching the GPU memory usage to a specific GPU by index/ID if the backend supports it.
| const memorySizeDisplay = hasMemorySize | ||
| ? gpu.memorySize | ||
| : hasMemoryUsage | ||
| ? (gpuMemoryUsage?.inUseBytes ?? "N/A") |
There was a problem hiding this comment.
The nullish coalescing operator here is redundant. The expression gpuMemoryUsage?.inUseBytes ?? "N/A" will already handle null/undefined cases, so wrapping it in parentheses and applying another ?? "N/A" is unnecessary since the inner expression already guarantees a non-null value.
| ? (gpuMemoryUsage?.inUseBytes ?? "N/A") | |
| ? gpuMemoryUsage?.inUseBytes |
No description provided.