Convert pointers to span in System.Net.Primitives#121016
Conversation
|
Tagging subscribers to this area: @dotnet/ncl |
|
(When I previously experimented with this it was quite measurable, but maybe things have improved since then.) |
EgorBo
force-pushed
the
safe-ipaddress
branch
from
d7aebf1 to
4b7bc1b
Compare
This comment was marked as resolved.
This comment was marked as resolved.
EgorBo
force-pushed
the
safe-ipaddress
branch
from
4b7bc1b to
3aca4e7
Compare
|
@EgorBot -intel -arm -amd using BenchmarkDotNet.Attributes;
public class UriParsingBenchmarks
{
public static IEnumerable<string> GetTestData() => new[]
{
"https://alice:pa%24%24@例子.com/テスト",
"https://bob%40dev:%F0%9F%92%[email protected]/💾/ファイル",
"https://user:pwd%40key@пример.тест/данные/測試?mode=%E6%A4%9C%E8%A8%BC",
"https://user:pa%24%[email protected]/路径/данные/資料",
"https://data:tok3n@服务.cn/文件/📊/統計?state=%7B%22ok%22%3Atrue%2C%22v%22%3A%22%25done%25%22%7D",
"https://token%3Auser:secr%25et@ドメイン.example/分析/データ/данные",
"https://üser:päss@пример.com/страница/テスト?lang=日本語",
"https://john:doe%21pass@δοκιμή.gr/δοκιμή/資料?ref=abc%25xyz",
"https://账号:密码@例え.テスト/画像/файлы?タグ=猫%20写真",
"https://x:y%25z@データ.jp/経路/данные/🧠/研究?city=Z%C3%BCrich&lang=%E6%97%A5%E6%9C%AC%E8%AA%9E"
};
// Baseline: construct a Uri
[Benchmark(Baseline = true)]
[ArgumentsSource(nameof(GetTestData))]
public Uri NewUri(string uri) => new (uri);
} |
There was a problem hiding this comment.
Pull Request Overview
This PR converts pointer-based string operations to ReadOnlySpan<char> in the System.Private.Uri library to improve code safety and make bounds checking more explicit. The change addresses concerns about raw pointer arithmetic that makes it difficult to validate array access safety.
Key Changes:
- Removed
unsafekeyword and pointer parameters fromTestForSubPathandUnescapePercentEncodedUTF8Sequencemethods - Replaced pointer arithmetic with span indexing and slicing operations
- Updated call sites to pass spans instead of fixed pointers
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| UriHelper.cs | Converted TestForSubPath from unsafe pointer-based to span-based implementation; updated call to UnescapePercentEncodedUTF8Sequence |
| UriExt.cs | Removed unsafe block and fixed statements in IsBaseOfHelper, now directly passes strings to TestForSubPath |
| PercentEncodingHelper.cs | Converted UnescapePercentEncodedUTF8Sequence signature from pointer to span; replaced pointer arithmetic with span slicing |
| IriHelper.cs | Updated call site to pass ReadOnlySpan<char> instead of pointer to UnescapePercentEncodedUTF8Sequence |
|
@stephentoub @MihaZupan I've limited changes to Iri parsing and it seems like my benchmarks (which utilize the code that I changed) didn't regress (-/+2% noise). Unless I'm missing something. I also ran |
For reference, my comment about having previously tested it and having seen regressions were about the IPAddress changes that were in the original commit (d7aebf1) |
Co-authored-by: Stephen Toub <[email protected]>
|
/ba-g deadletter |
IPAddress and Uri parsing are heavily relying on raw pointers making code extremely difficult to validate we never go out of bounds. I wonder what is the performance impact if we convert them all to Span.. Obviously, in most cases it will lead to redundant bounds checks that JIT is unlikely to remove due to complex loop shapes, implicit/IPA assumptions, etc..