Skip to content

⚡ Bolt: [Performance optimization] WslPathResolver allocation reduction #144

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

Open
google-labs-jules wants to merge 3 commits into
base: master
Choose a base branch
from
+30 −4
Open

⚡ Bolt: [Performance optimization] WslPathResolver allocation reduction #144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0ce5189
⚡ Bolt: [Performance optimization] Avoid string array and string allo…
google-labs-jules[bot] Jul 31, 2026
598759b
fix: apply CodeRabbit auto-fixes
coderabbitai[bot] Jul 31, 2026
c1e8a80
⚡ Bolt: [Performance optimization] Avoid string array and string allo…
google-labs-jules[bot] Jul 31, 2026
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
3 changes: 3 additions & 0 deletions .jules/bolt.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## 2024-05-24 - WslPathResolver string splitting
**Learning:** `WslPathResolver.TryParseUncRemainder` used `string.Split()`, `string.Join()`, and `Enumerable.Skip()` which leads to unnecessary GC pressure and multiple string/array allocations for hot path code.
**Action:** Replace `string.Split` with `ReadOnlySpan<char>.Split` and use `StringBuilder` to append portions sequentially when parsing and reconstructing path parts. This saves multiple array/string allocations and makes parsing faster.
31 changes: 27 additions & 4 deletions QuickShell.Core/Services/WslPathResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,16 +142,39 @@
private static bool TryParseUncRemainder(string remainder, string fullUnc, out WslLocation location)
{
location = null!;
var parts = remainder.Split('\\', StringSplitOptions.RemoveEmptyEntries);
if (parts.Length < 2)
// Bolt: Performance optimization - avoid string.Split(), Linq Skip(), and string.Join() allocations.
var span = remainder.AsSpan();
string? distro = null;
var linuxPathBuilder = new System.Text.StringBuilder();
Comment on lines +145 to +148

@coderabbitai coderabbitai Bot Jul 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚀 Performance & Scalability | 🔵 Trivial

Make the performance claim reproducible.

The PR reports allocation and runtime reductions, but the repository does not contain a dedicated benchmark for the old and new implementations. Add a checked-in benchmark that measures allocations, Gen0 collections, and runtime for both paths. Treat the reported measurements as unverified until the benchmark produces them.

🤖 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 `@QuickShell.Core/Services/WslPathResolver.cs` around lines 145 - 148, Add a
dedicated benchmark that measures both the old implementation path (using
string.Split, Linq Skip, and string.Join) and the new optimized implementation
path (using AsSpan and StringBuilder) in the WslPathResolver code. The benchmark
should measure allocations, Gen0 collections, and runtime for both
implementations to verify the claimed performance improvements, and commit this
benchmark to the repository so the measurements are reproducible and verifiable.

Source: MCP tools

Comment on lines +146 to +148

@greptile-apps greptile-apps Bot Jul 31, 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.

P2 The StringBuilder is created unconditionally on every call, even for early-return cases (e.g. a path with only a distro name and no segments). Initialising it with a capacity derived from remainder.Length would also avoid internal reallocations for longer paths, and using the using alias using System.Text; at the top of the file is cleaner than the fully-qualified name inline.

Suggested change
var linuxPathBuilder = new System.Text.StringBuilder();
var linuxPathBuilder = new System.Text.StringBuilder(remainder.Length);
Prompt To Fix With AI 
This is a comment left during a code review.
Path: QuickShell.Core/Services/WslPathResolver.cs
Line: 148

Comment:
The `StringBuilder` is created unconditionally on every call, even for early-return cases (e.g. a path with only a distro name and no segments). Initialising it with a capacity derived from `remainder.Length` would also avoid internal reallocations for longer paths, and using the `using` alias `using System.Text;` at the top of the file is cleaner than the fully-qualified name inline.

```suggestion
        var linuxPathBuilder = new System.Text.StringBuilder(remainder.Length);
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Cursor Fix in Claude Code Fix in Codex


foreach (var range in span.Split('\\'))

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning

This assignment to
range
Loading
is useless, since its value is never read.
{
var part = span[range];
if (part.IsEmpty)
{
continue;
}

if (distro is null)
{
distro = part.ToString();
}
else
{
linuxPathBuilder.Append('/');
linuxPathBuilder.Append(part);
}
}

if (linuxPathBuilder.Length == 0 || distro is null)
{
return false;
}

location = new WslLocation
{
Distro = parts[0],
LinuxPath = "/" + string.Join('/', parts.Skip(1)),
Distro = distro,
LinuxPath = linuxPathBuilder.ToString(),
Comment on lines +150 to +177

@coderabbitai coderabbitai Bot Jul 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add direct tests for UNC remainder parsing.

WslPathResolverTests.cs does not directly exercise this parser. Add cases for both \\wsl$\ and \\wsl.localhost\, repeated separators, trailing separators, a missing distro, and a missing Linux path. Assert Distro, LinuxPath, UncPath, and rejected inputs. This protects the span iteration and validation contract.

🧰 Tools
🪛 GitHub Check: CodeQL

[warning] 150-150: Useless assignment to local variable
This assignment to range is useless, since its value is never read.

🤖 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 `@QuickShell.Core/Services/WslPathResolver.cs` around lines 150 - 177, Add
direct tests in WslPathResolverTests for UNC remainder parsing, covering both
\\wsl$\ and \\wsl.localhost\ prefixes, repeated and trailing separators, missing
distro, and missing Linux path. Assert the resulting Distro, LinuxPath, and
UncPath values for valid inputs, and verify invalid inputs are rejected,
targeting the parser behavior implemented around the span iteration and
validation checks.

Source: MCP tools

UncPath = fullUnc,
};

Expand Down
Loading