-
-
Notifications
You must be signed in to change notification settings - Fork 0
β‘ 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.
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
Changes from 5 commits
0ce5189
598759b
c1e8a80
ca30e4d
e7bf692
421b1b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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(span.Length); | ||
|
|
||
| foreach (var range in span.Split('\\')) | ||
|
|
||
| { | ||
| 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
π§° Toolsπͺ GitHub Check: CodeQL[warning] 150-150: Useless assignment to local variable π€ Prompt for AI AgentsSource: MCP tools |
||
| UncPath = fullUnc, | ||
| }; | ||
|
|
||
|
|
||
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.
π Maintainability & Code Quality | π‘ Minor | β‘ Quick win
Fix the Markdown heading structure.
markdownlintreportsMD041because the file starts with anh2heading. It also reportsMD022because the heading has no blank line after it. Change the first heading toh1and add a blank line.Proposed fix
π Committable suggestion
π§° Tools
πͺ LanguageTool
[style] ~3-~3: Consider using a different verb to strengthen your wording.
Context: ...s multiple array/string allocations and makes parsing faster.
(MAKE_XXX_FASTER)
πͺ markdownlint-cli2 (0.23.1)
[warning] 1-1: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
π€ Prompt for AI Agents
Source: Linters/SAST tools