-
Notifications
You must be signed in to change notification settings - Fork 433
Sysinfo: add support for AMD and NVIDIA GPUs #2238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis change set introduces GPU monitoring support across the system, spanning backend data collection, frontend visualization, documentation, and testing. A new documentation file details the GPU monitoring feature, including platform-specific command-line integrations for Linux, macOS, and Windows. The backend ( Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (7)
pkg/wshrpc/wshremote/sysinfo_test.go (1)
20-28
: Consider adding verification for tool availability tests.While logging the availability is useful, consider adding assertions to verify expected behavior in CI environments or when specific tools are known to be present.
pkg/wshrpc/wshremote/sysinfo.go (6)
189-189
: Consider using debug-level logging for GPU detection.This log statement will execute every second for each GPU, potentially flooding the logs. Consider using debug-level logging or removing it.
- log.Printf("Found macOS GPU: %s", name) + // Only log in debug mode or remove entirely
399-399
: Document the GPU memory estimation heuristic.The 10% estimation of system memory for GPU is arbitrary and could be misleading. Consider adding a comment explaining this is a rough fallback estimate.
// Estimate GPU memory as a fraction of system memory - // This is a rough estimate and varies by GPU + // This is a rough fallback estimate (10% of system RAM) when actual VRAM info is unavailable + // Actual GPU memory varies significantly and this should be treated as unreliable return totalGB * 0.1 // Assume 10% of system memory for GPU
463-463
: Consider using debug-level logging for Windows GPU detection.Similar to the macOS implementation, this will log every second for each GPU. Consider using debug-level logging or removing it.
- log.Printf("Found Windows GPU: %s (%.2f GB total, %.2f GB used)", gpu.Name, gpu.MemTotal, memUsed) + // Only log in debug mode or remove entirely
415-427
: Consider extracting PowerShell scripts to separate files or constants.Large embedded PowerShell scripts are difficult to maintain and test. Consider extracting them to constants or external files for better maintainability.
+const psGetGpuInfoScript = ` + $gpus = Get-WmiObject -Class Win32_VideoController | Where-Object { $_.Name -notlike "*Basic*" -and $_.Name -notlike "*Standard*" } + $gpuInfo = @() + foreach ($gpu in $gpus) { + $gpuInfo += [PSCustomObject]@{ + Name = $gpu.Name + AdapterRAM = $gpu.AdapterRAM + VideoProcessor = $gpu.VideoProcessor + DriverVersion = $gpu.DriverVersion + } + } + $gpuInfo | ConvertTo-Json -Compress +` func getWindowsGpuData() ([]GpuData, error) { // ... - psCommand := ` - $gpus = Get-WmiObject -Class Win32_VideoController | Where-Object { $_.Name -notlike "*Basic*" -and $_.Name -notlike "*Standard*" } - // ... rest of script - ` + psCommand := psGetGpuInfoScript
641-643
: Consider logging GPU detection failures for debugging.Silently ignoring GPU detection errors makes troubleshooting difficult. Consider logging the error at debug level.
if err != nil || len(gpus) == 0 { + if err != nil { + log.Printf("Failed to get GPU data: %v", err) + } return }
697-697
: Remove redundant inline comment.The function name
getGpuData
is self-documenting. The inline comment adds no value.- getGpuData(values) // Add this line to get GPU data + getGpuData(values)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/GPU_MONITORING.md
(1 hunks)frontend/app/theme.scss
(1 hunks)frontend/app/view/sysinfo/sysinfo.tsx
(2 hunks)pkg/wshrpc/wshremote/sysinfo.go
(3 hunks)pkg/wshrpc/wshremote/sysinfo_test.go
(1 hunks)pkg/wshrpc/wshremote/wshremote.go
(1 hunks)pkg/wshrpc/wshrpctypes.go
(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: the `copyremote` function in waveterm's file operations has proper error handling that prevents part...
Learnt from: esimkowitz
PR: wavetermdev/waveterm#1725
File: pkg/remote/fileshare/wavefs/wavefs.go:441-494
Timestamp: 2025-01-29T04:21:11.649Z
Learning: The `CopyRemote` function in WaveTerm's file operations has proper error handling that prevents partial writes by failing fast and using context cancellation. Each step (path cleaning, file operations, tar reading/writing) is guarded by error checks that prevent proceeding with writes on error.
Applied to files:
pkg/wshrpc/wshremote/wshremote.go
📚 Learning: the rpcclient in pkg/remote/fileshare/wshfs/wshfs.go is initialized and handled downstream by either...
Learnt from: esimkowitz
PR: wavetermdev/waveterm#1790
File: pkg/remote/fileshare/wshfs/wshfs.go:122-122
Timestamp: 2025-01-22T01:28:41.417Z
Learning: The RpcClient in pkg/remote/fileshare/wshfs/wshfs.go is initialized and handled downstream by either main-server or wshcmd-connserver, as documented in the package comment.
Applied to files:
pkg/wshrpc/wshremote/wshremote.go
pkg/wshrpc/wshremote/sysinfo.go
📚 Learning: the aws connection code path in pkg/remote/awsconn/awsconn.go is currently inactive, so temporary fi...
Learnt from: esimkowitz
PR: wavetermdev/waveterm#1790
File: pkg/remote/awsconn/awsconn.go:32-33
Timestamp: 2025-01-22T01:02:32.621Z
Learning: The AWS connection code path in pkg/remote/awsconn/awsconn.go is currently inactive, so temporary file cleanup is not a critical concern at this time.
Applied to files:
pkg/wshrpc/wshremote/wshremote.go
📚 Learning: the getrpccontext() method in wshutil package is guaranteed to never return nil due to type constrai...
Learnt from: esimkowitz
PR: wavetermdev/waveterm#1790
File: pkg/remote/connparse/connparse.go:76-82
Timestamp: 2025-01-22T22:27:25.739Z
Learning: The GetRpcContext() method in wshutil package is guaranteed to never return nil due to type constraints, making nil checks unnecessary.
Applied to files:
pkg/wshrpc/wshremote/sysinfo.go
🔇 Additional comments (11)
pkg/wshrpc/wshrpctypes.go (1)
583-583
: LGTM! Consistent constant addition.The new
TimeSeries_Gpu
constant follows the existing naming pattern and integrates well with the GPU monitoring functionality.frontend/app/theme.scss (1)
119-119
: LGTM! Consistent color variable addition.The new GPU color variable follows the existing pattern and provides good visual distinction from CPU and memory colors.
pkg/wshrpc/wshremote/wshremote.go (1)
813-815
: LGTM! Good cleanup of unused variables.Removing the unused byte count variables while preserving error handling improves code cleanliness without affecting functionality.
pkg/wshrpc/wshremote/sysinfo_test.go (4)
12-18
: LGTM! Good basic platform detection test.The test appropriately verifies that platform detection returns a non-empty string and logs the result for visibility.
30-46
: Good exploratory test for GPU data collection.The test properly handles the case where no GPU data is available, which is important for systems without GPU monitoring tools.
48-77
: Excellent test coverage for macOS GPU parsing.The test uses realistic sample data and covers multiple parsing functions. The logging provides good visibility into the parsing results.
79-93
: Good Windows GPU parsing test coverage.The test validates both GPU detection and memory usage parsing with realistic JSON sample data.
frontend/app/view/sysinfo/sysinfo.tsx (3)
69-95
: Excellent GPU plot type implementations.The new GPU plot types follow the existing patterns perfectly:
- Consistent filtering and sorting logic
- Proper key extraction for utilization and memory metrics
- Good integration with existing CPU + memory combinations
104-104
: Good addition of base GPU metadata.The GPU utilization metadata reuses the CPU percentage logic, which is appropriate since both represent utilization percentages.
109-121
: Comprehensive GPU metadata coverage.The loop creates metadata for up to 8 GPUs covering:
- Utilization (%), memory (GB), and temperature (°C)
- Consistent use of the new GPU color variable
- Appropriate value ranges and decimal places
This provides excellent support for multi-GPU systems.
pkg/wshrpc/wshremote/sysinfo.go (1)
128-129
: Verify regex correctness against real rocm-smi output
We weren’t able to runrocm-smi
in this environment, so it’s unclear whether the single-line regex at pkg/wshrpc/wshremote/sysinfo.go:128–129 will ever match the actual (multi-line) output.Please test on a machine with AMD ROCm installed and:
- Capture a sample of
rocm-smi --showproductname --showmeminfo vram --showtemp
output- Confirm the current regex matches each GPU block, or adjust it to account for line breaks/format changes
- Consider a more robust parsing approach (e.g., splitting on blank lines and using separate patterns per field) if necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (2)
pkg/wshrpc/wshremote/sysinfo.go (2)
50-61
: Platform detection correctly implemented!The function now properly uses
runtime.GOOS
for cross-platform detection, which resolves the previous issue withuname
not being available on Windows.
108-110
: Inconsistent error handling for parsing GPU metrics.While errors are handled for
index
andutil
parsing (lines 98-107), the parsing ofmemUsed
,memTotal
, andtemp
still ignores errors. This inconsistency could lead to confusion about which metrics are reliable.Apply consistent error handling:
-memUsed, _ := strconv.ParseFloat(strings.TrimSpace(fields[2]), 64) -memTotal, _ := strconv.ParseFloat(strings.TrimSpace(fields[3]), 64) -temp, _ := strconv.ParseFloat(strings.TrimSpace(fields[4]), 64) +memUsed, err := strconv.ParseFloat(strings.TrimSpace(fields[2]), 64) +if err != nil { + log.Printf("Error parsing nvidia-smi memory used: %v", err) + memUsed = 0 +} +memTotal, err := strconv.ParseFloat(strings.TrimSpace(fields[3]), 64) +if err != nil { + log.Printf("Error parsing nvidia-smi memory total: %v", err) + memTotal = 0 +} +temp, err := strconv.ParseFloat(strings.TrimSpace(fields[4]), 64) +if err != nil { + log.Printf("Error parsing nvidia-smi temperature: %v", err) + temp = 0 +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/GPU_MONITORING.md
(1 hunks)pkg/wshrpc/wshremote/sysinfo.go
(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: in go, using a single if statement with the `&&` operator (e.g. `if x != nil && x.field != nil`) is ...
Learnt from: esimkowitz
PR: wavetermdev/waveterm#1807
File: pkg/wshrpc/wshremote/wshremote.go:718-726
Timestamp: 2025-01-23T06:09:05.334Z
Learning: In Go, using a single if statement with the `&&` operator (e.g. `if x != nil && x.Field != nil`) is the idiomatic way to handle multiple nil checks. Avoid suggesting nested if statements as they make the code more verbose without adding value.
Applied to files:
pkg/wshrpc/wshremote/sysinfo.go
📚 Learning: the getrpccontext() method in wshutil package is guaranteed to never return nil due to type constrai...
Learnt from: esimkowitz
PR: wavetermdev/waveterm#1790
File: pkg/remote/connparse/connparse.go:76-82
Timestamp: 2025-01-22T22:27:25.739Z
Learning: The GetRpcContext() method in wshutil package is guaranteed to never return nil due to type constraints, making nil checks unnecessary.
Applied to files:
pkg/wshrpc/wshremote/sysinfo.go
📚 Learning: the rpcclient in pkg/remote/fileshare/wshfs/wshfs.go is initialized and handled downstream by either...
Learnt from: esimkowitz
PR: wavetermdev/waveterm#1790
File: pkg/remote/fileshare/wshfs/wshfs.go:122-122
Timestamp: 2025-01-22T01:28:41.417Z
Learning: The RpcClient in pkg/remote/fileshare/wshfs/wshfs.go is initialized and handled downstream by either main-server or wshcmd-connserver, as documented in the package comment.
Applied to files:
pkg/wshrpc/wshremote/sysinfo.go
🧬 Code Graph Analysis (1)
pkg/wshrpc/wshremote/sysinfo.go (1)
pkg/wshrpc/wshrpctypes.go (1)
TimeSeries_Gpu
(583-583)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pkg/wshrpc/wshremote/sysinfo.go (1)
49-61
: Platform detection correctly implemented.The function now properly uses
runtime.GOOS
for reliable cross-platform detection, addressing the previous Windows compatibility issue.
🧹 Nitpick comments (4)
pkg/wshrpc/wshremote/sysinfo.go (4)
108-110
: Complete error handling for remaining fields.While error handling was added for index and utilization parsing, the memory and temperature parsing still ignores errors. For consistency and robustness, consider adding error handling for these fields as well.
-memUsed, _ := strconv.ParseFloat(strings.TrimSpace(fields[2]), 64) -memTotal, _ := strconv.ParseFloat(strings.TrimSpace(fields[3]), 64) -temp, _ := strconv.ParseFloat(strings.TrimSpace(fields[4]), 64) +memUsed, err := strconv.ParseFloat(strings.TrimSpace(fields[2]), 64) +if err != nil { + log.Printf("Error parsing nvidia-smi memory used: %v", err) + memUsed = 0 +} +memTotal, err := strconv.ParseFloat(strings.TrimSpace(fields[3]), 64) +if err != nil { + log.Printf("Error parsing nvidia-smi memory total: %v", err) + memTotal = 0 +} +temp, err := strconv.ParseFloat(strings.TrimSpace(fields[4]), 64) +if err != nil { + log.Printf("Error parsing nvidia-smi temperature: %v", err) + temp = 0 +}
125-179
: Good improvements with error handling, but consider output format robustness.The error handling has been properly implemented as requested in previous reviews. However, the regex pattern for parsing rocm-smi output might be fragile if the output format changes between versions.
Consider adding more flexible parsing or fallback mechanisms for different rocm-smi output formats to improve robustness across AMD driver versions.
254-280
: Improve macOS GPU utilization detection reliability.The current approach to parse
iostat
output for GPU utilization is quite fragile. The parsing logic looks for lines containing "gpu" or "GPU" butiostat
typically doesn't provide GPU-specific information in most versions.Consider returning 0 or using a more reliable method for GPU utilization on macOS, as this parsing is likely to fail in most cases:
func getMacGpuUtilization() float64 { - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) - defer cancel() - - // Try to get GPU utilization using iostat - cmd := exec.CommandContext(ctx, "iostat", "-n", "1", "1") - output, err := cmd.Output() - if err != nil { - return 0 - } - - // Parse iostat output for GPU utilization - lines := strings.Split(string(output), "\n") - for _, line := range lines { - if strings.Contains(line, "gpu") || strings.Contains(line, "GPU") { - fields := strings.Fields(line) - if len(fields) >= 2 { - if util, err := strconv.ParseFloat(fields[1], 64); err == nil { - return util - } - } - } - } - - return 0 + // iostat doesn't typically provide GPU utilization on macOS + // Return 0 to indicate unavailable data + return 0 }
468-510
: Improve Windows GPU output parsing robustness.The
parseWindowsGpuOutput
function uses regex to parse JSON-like output, but this approach is fragile. If the PowerShell command actually returns valid JSON, consider using proper JSON parsing.+import "encoding/json" func parseWindowsGpuOutput(output string) []WindowsGpuInfo { var gpuList []WindowsGpuInfo - // Try to parse as JSON array if strings.TrimSpace(output) == "" { return gpuList } - // Simple JSON parsing for the PowerShell output - // The output should be a JSON array of GPU objects - lines := strings.Split(output, "\n") - for _, line := range lines { - line = strings.TrimSpace(line) - if line == "" { - continue - } - - // Extract GPU name and memory info using regex - nameRe := regexp.MustCompile(`"Name":\s*"([^"]+)"`) - adapterRamRe := regexp.MustCompile(`"AdapterRAM":\s*(\d+)`) - - nameMatches := nameRe.FindStringSubmatch(line) - ramMatches := adapterRamRe.FindStringSubmatch(line) - - if len(nameMatches) >= 2 && len(ramMatches) >= 2 { - name := nameMatches[1] - if ramSize, err := strconv.ParseInt(ramMatches[1], 10, 64); err == nil { - // Convert bytes to GB - memTotal := float64(ramSize) / (1024 * 1024 * 1024) - - gpuInfo := WindowsGpuInfo{ - Name: name, - MemTotal: memTotal, - MemUsed: 0, - } - gpuList = append(gpuList, gpuInfo) - } - } - } + // Try proper JSON parsing first + var rawGpus []map[string]interface{} + if err := json.Unmarshal([]byte(output), &rawGpus); err == nil { + for _, gpu := range rawGpus { + if name, ok := gpu["Name"].(string); ok { + if ramInterface, ok := gpu["AdapterRAM"]; ok { + var ramSize int64 + switch v := ramInterface.(type) { + case float64: + ramSize = int64(v) + case string: + if parsed, err := strconv.ParseInt(v, 10, 64); err == nil { + ramSize = parsed + } + } + + if ramSize > 0 { + memTotal := float64(ramSize) / (1024 * 1024 * 1024) + gpuInfo := WindowsGpuInfo{ + Name: name, + MemTotal: memTotal, + MemUsed: 0, + } + gpuList = append(gpuList, gpuInfo) + } + } + } + } + return gpuList + } + + // Fallback to regex parsing if JSON parsing fails + // ... existing regex logic ... return gpuList }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/GPU_MONITORING.md
(1 hunks)pkg/wshrpc/wshremote/sysinfo.go
(3 hunks)pkg/wshrpc/wshremote/sysinfo_test.go
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/wshrpc/wshremote/sysinfo_test.go
- docs/GPU_MONITORING.md
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: the getrpccontext() method in wshutil package is guaranteed to never return nil due to type constrai...
Learnt from: esimkowitz
PR: wavetermdev/waveterm#1790
File: pkg/remote/connparse/connparse.go:76-82
Timestamp: 2025-01-22T22:27:25.739Z
Learning: The GetRpcContext() method in wshutil package is guaranteed to never return nil due to type constraints, making nil checks unnecessary.
Applied to files:
pkg/wshrpc/wshremote/sysinfo.go
📚 Learning: the rpcclient in pkg/remote/fileshare/wshfs/wshfs.go is initialized and handled downstream by either...
Learnt from: esimkowitz
PR: wavetermdev/waveterm#1790
File: pkg/remote/fileshare/wshfs/wshfs.go:122-122
Timestamp: 2025-01-22T01:28:41.417Z
Learning: The RpcClient in pkg/remote/fileshare/wshfs/wshfs.go is initialized and handled downstream by either main-server or wshcmd-connserver, as documented in the package comment.
Applied to files:
pkg/wshrpc/wshremote/sysinfo.go
🧬 Code Graph Analysis (1)
pkg/wshrpc/wshremote/sysinfo.go (1)
pkg/wshrpc/wshrpctypes.go (1)
TimeSeries_Gpu
(583-583)
🔇 Additional comments (5)
pkg/wshrpc/wshremote/sysinfo.go (5)
6-38
: LGTM! Imports and constants are well-structured.The new imports are appropriate for the GPU monitoring functionality, and the PowerShell command constant is properly formatted for Windows GPU detection.
40-47
: LGTM! Well-designed GPU data structure.The
GpuData
struct appropriately captures essential GPU metrics with proper JSON tags for serialization.
63-79
: LGTM! Proper utility availability detection.Both functions correctly use context with timeout and appropriate commands to detect GPU utility availability.
604-646
: LGTM! Well-structured GPU data integration.The main
getGpuData
function properly orchestrates platform-specific GPU data collection and integrates it cleanly with the existing metrics system. The aggregate utilization calculation and error handling are appropriate.
681-681
: LGTM! Clean integration with existing metrics collection.The addition of
getGpuData(values)
follows the established pattern and integrates seamlessly with the existing CPU and memory data collection.
// Get memory pressure from vm_stat command | ||
func getMemoryPressureFromVMStat() float64 { | ||
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) | ||
defer cancel() | ||
|
||
cmd := exec.CommandContext(ctx, "vm_stat") | ||
output, err := cmd.Output() | ||
if err != nil { | ||
return 0 | ||
} | ||
|
||
lines := strings.Split(string(output), "\n") | ||
var pageSize int64 = 4096 // Default page size | ||
|
||
// Find page size | ||
for _, line := range lines { | ||
if strings.Contains(line, "Mach Virtual Memory Statistics") { | ||
re := regexp.MustCompile(`page size of (\d+)`) | ||
matches := re.FindStringSubmatch(line) | ||
if len(matches) >= 2 { | ||
if size, err := strconv.ParseInt(matches[1], 10, 64); err == nil { | ||
pageSize = size | ||
} | ||
} | ||
break | ||
} | ||
} | ||
|
||
// Parse memory statistics | ||
var activePages, inactivePages, wiredPages int64 | ||
|
||
for _, line := range lines { | ||
line = strings.TrimSpace(line) | ||
if strings.Contains(line, "Pages active:") { | ||
re := regexp.MustCompile(`Pages active:\s+(\d+)`) | ||
matches := re.FindStringSubmatch(line) | ||
if len(matches) >= 2 { | ||
if pages, err := strconv.ParseInt(matches[1], 10, 64); err == nil { | ||
activePages = pages | ||
} | ||
} | ||
} else if strings.Contains(line, "Pages inactive:") { | ||
re := regexp.MustCompile(`Pages inactive:\s+(\d+)`) | ||
matches := re.FindStringSubmatch(line) | ||
if len(matches) >= 2 { | ||
if pages, err := strconv.ParseInt(matches[1], 10, 64); err == nil { | ||
inactivePages = pages | ||
} | ||
} | ||
} else if strings.Contains(line, "Pages wired down:") { | ||
re := regexp.MustCompile(`Pages wired down:\s+(\d+)`) | ||
matches := re.FindStringSubmatch(line) | ||
if len(matches) >= 2 { | ||
if pages, err := strconv.ParseInt(matches[1], 10, 64); err == nil { | ||
wiredPages = pages | ||
} | ||
} | ||
} | ||
} | ||
|
||
// Calculate used memory in GB | ||
usedBytes := (activePages + inactivePages + wiredPages) * pageSize | ||
return float64(usedBytes) / (1024 * 1024 * 1024) // Convert to GB | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Misleading GPU memory usage calculation.
The getMemoryPressureFromVMStat
function calculates system memory pressure, not GPU memory usage. Using system memory statistics as a proxy for GPU memory usage is misleading and could confuse users.
Consider either:
- Returning 0 to indicate GPU memory usage is unavailable on macOS
- Clearly documenting that this represents system memory pressure, not GPU memory
- Finding a more accurate method to estimate GPU memory usage
func getMemoryPressureFromVMStat() float64 {
- // Complex vm_stat parsing logic...
+ // GPU memory usage is not directly available on macOS
+ // Return 0 to indicate unavailable data
+ return 0
}
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In pkg/wshrpc/wshremote/sysinfo.go lines 336 to 399, the function
getMemoryPressureFromVMStat currently calculates system memory pressure but is
misleadingly used or interpreted as GPU memory usage. To fix this, either modify
the function to return 0 to indicate GPU memory usage is unavailable on macOS,
or add clear documentation in the function comment stating it returns system
memory pressure, not GPU memory usage. Alternatively, if feasible, implement a
more accurate method to estimate GPU memory usage on macOS.
I added support for AMD and NVIDIA GPUs by using platform specific commands. Tested only on Linux, the implementations for Windows and MacOS require appropriate review.