Skip to content

fix: performance improvements (#312) #356

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.

Sign up for GitHub

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

Merged
laurentiu021 merged 1 commit into from
May 14, 2026
Merged

fix: performance improvements (#312) #356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,19 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

## [0.48.5] - 2026-05-14

### Changed
- **DuplicateFileService** — ShouldSkipDir uses OrdinalIgnoreCase instead of
ToLowerInvariant allocation on every path (PERF-002).
- **LargeFileScanner** — same OrdinalIgnoreCase fix (PERF-002).
- **SpeedTestService** — SHA-256 hashing uses stream instead of
File.ReadAllBytes to avoid loading entire zip into memory (PERF-004).
- **ProcessManagerService** — MainModule accessed once per process instead of
twice, halving P/Invoke overhead (PERF-005).
- **AboutViewModel** — CopyEnvironmentInfo WMI queries now run on background
thread via Task.Run, preventing UI freeze (PERF-008).

## [0.48.4] - 2026-05-14

### Fixed
Expand Down
3 changes: 1 addition & 2 deletions SysManager/SysManager/Services/DuplicateFileService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,7 @@ private static string ComputePartialHash(string filePath, CancellationToken ct)

private static bool ShouldSkipDir(string path)
{
var lower = path.ToLowerInvariant();
return SkipSegments.Any(seg => lower.Contains(seg));
return SkipSegments.Any(seg => path.Contains(seg, StringComparison.OrdinalIgnoreCase));
}

private static bool ShouldSkipFile(string name)
Expand Down
3 changes: 1 addition & 2 deletions SysManager/SysManager/Services/LargeFileScanner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ private static IReadOnlyList<LargeFileEntry> Scan(

private static bool ShouldSkip(string path)
{
var lower = path.ToLowerInvariant();
return SkipSegments.Any(seg => lower.Contains(seg));
return SkipSegments.Any(seg => path.Contains(seg, StringComparison.OrdinalIgnoreCase));
}
}
10 changes: 6 additions & 4 deletions SysManager/SysManager/Services/ProcessManagerService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,12 @@ private static IReadOnlyList<ProcessEntry> Snapshot(CancellationToken ct)
catch (System.ComponentModel.Win32Exception) { /* process may have exited */ }
}

try { entry.Description = p.MainModule?.FileVersionInfo.FileDescription ?? ""; }
catch (InvalidOperationException) { /* access denied or process exited */ }
catch (System.ComponentModel.Win32Exception) { /* access denied or process exited */ }
try { entry.FilePath = p.MainModule?.FileName ?? ""; }
try
{
var module = p.MainModule;
entry.Description = module?.FileVersionInfo.FileDescription ?? "";
entry.FilePath = module?.FileName ?? "";
}
catch (InvalidOperationException) { /* access denied or process exited */ }
catch (System.ComponentModel.Win32Exception) { /* access denied or process exited */ }
entry.CanOpenFileLocation = !string.IsNullOrWhiteSpace(entry.FilePath)
Expand Down
3 changes: 2 additions & 1 deletion SysManager/SysManager/Services/SpeedTestService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,8 @@ private static async Task<string> EnsureOoklaAsync(
// extracted binary, not on pinned hashes (which break on Ookla updates).
await Task.Run(() =>
{
var hash = Convert.ToHexString(SHA256.HashData(File.ReadAllBytes(zipPath)));
using var stream = File.OpenRead(zipPath);
var hash = Convert.ToHexString(SHA256.HashData(stream));
Log.Information("Ookla CLI downloaded: {Url}, SHA256={Hash}, Size={Size}",
zipUrl, hash, new FileInfo(zipPath).Length);

Expand Down
29 changes: 23 additions & 6 deletions SysManager/SysManager/ViewModels/AboutViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,26 @@ private void OpenManualDownload()
/// Fully defensive — falls back gracefully on any WMI / registry miss.
/// </summary>
[RelayCommand]
private void CopyEnvironmentInfo()
private async Task CopyEnvironmentInfoAsync()
{
try
{
var text = await Task.Run(() => CollectEnvironmentInfo()).ConfigureAwait(true);
try { Clipboard.SetText(text); }
catch (System.Runtime.InteropServices.ExternalException ex) { Log.Debug("Clipboard locked: {Error}", ex.Message); }
UpdateStatus = "Environment info copied to clipboard.";
}
catch (System.Management.ManagementException ex)
{
UpdateStatus = $"Couldn't collect environment info: {ex.Message}";
}
catch (InvalidOperationException ex)
{
UpdateStatus = $"Couldn't collect environment info: {ex.Message}";
}
}
Comment on lines +220 to +237

@coderabbitai coderabbitai Bot May 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix false-success status when clipboard copy fails.

UpdateStatus is set to success even when Clipboard.SetText throws, so users can get a “copied” message without data actually copied. Also, because CollectEnvironmentInfo() returns error text instead of throwing, this method should explicitly handle that return path before claiming success.

💡 Suggested fix 
 private async Task CopyEnvironmentInfoAsync()
 {
-    try
-    {
-        var text = await Task.Run(() => CollectEnvironmentInfo()).ConfigureAwait(true);
-        try { Clipboard.SetText(text); }
-        catch (System.Runtime.InteropServices.ExternalException ex) { Log.Debug("Clipboard locked: {Error}", ex.Message); }
-        UpdateStatus = "Environment info copied to clipboard.";
-    }
-    catch (System.Management.ManagementException ex)
-    {
-        UpdateStatus = $"Couldn't collect environment info: {ex.Message}";
-    }
-    catch (InvalidOperationException ex)
-    {
-        UpdateStatus = $"Couldn't collect environment info: {ex.Message}";
-    }
+    var text = await Task.Run(CollectEnvironmentInfo).ConfigureAwait(true);
+    if (text.StartsWith("Couldn't collect environment info:", StringComparison.Ordinal))
+    {
+        UpdateStatus = text;
+        return;
+    }
+
+    try
+    {
+        Clipboard.SetText(text);
+        UpdateStatus = "Environment info copied to clipboard.";
+    }
+    catch (System.Runtime.InteropServices.ExternalException ex)
+    {
+        Log.Debug("Clipboard locked: {Error}", ex.Message);
+        UpdateStatus = "Couldn't copy environment info: clipboard is currently in use.";
+    }
 }
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@SysManager/SysManager/ViewModels/AboutViewModel.cs` around lines 220 - 237,
The method CopyEnvironmentInfoAsync currently sets UpdateStatus to success
unconditionally; update it to only set the success message after a confirmed
successful Clipboard.SetText call, and handle failure paths: call
CollectEnvironmentInfo() and if it returns null/empty or an error-indicating
string (handle the known error format your CollectEnvironmentInfo returns) set
UpdateStatus to an error message and skip Clipboard.SetText; wrap
Clipboard.SetText in the existing catch for
System.Runtime.InteropServices.ExternalException but set UpdateStatus to a
failure message (including ex.Message) instead of leaving the success text; keep
the existing catches for ManagementException and InvalidOperationException but
ensure they set appropriate UpdateStatus values as currently done.


private string CollectEnvironmentInfo()
{
try
{
Expand Down Expand Up @@ -315,17 +334,15 @@ private void CopyEnvironmentInfo()
catch (System.Management.ManagementException ex) { Log.Debug("Display info unavailable: {Error}", ex.Message); }

var text = sb.ToString();
try { Clipboard.SetText(text); }
catch (System.Runtime.InteropServices.ExternalException ex) { Log.Debug("Clipboard locked: {Error}", ex.Message); }
UpdateStatus = "Environment info copied to clipboard.";
return text;
}
catch (System.Management.ManagementException ex)
{
UpdateStatus = $"Couldn't collect environment info: {ex.Message}";
return $"Couldn't collect environment info: {ex.Message}";
}
catch (InvalidOperationException ex)
{
UpdateStatus = $"Couldn't collect environment info: {ex.Message}";
return $"Couldn't collect environment info: {ex.Message}";
}
}

Expand Down
Loading