Skip to content
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

Improve performance of JoinParts for spans #42

Merged
merged 6 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
4 changes: 2 additions & 2 deletions src/NexusMods.Paths.Benchmarks/Benchmarks/Pr42.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@ public Pr42()
[Benchmark]
public string Current()
{
return PathHelpers.JoinParts(Path1, Path2, _osInformation);
return PathHelpers.JoinParts(Path1.AsSpan(), Path2.AsSpan(), _osInformation);
}

[Benchmark]
public string OldMethod()
{
return JoinParts(Path1, Path2, _osInformation);
return JoinParts(Path1.AsSpan(), Path2.AsSpan(), _osInformation);
}

[SkipLocalsInit] // original method didn't have it, but it's still good for comparison.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net7.0</TargetFramework>
<TargetFramework>net8.0</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
Expand Down
50 changes: 30 additions & 20 deletions src/NexusMods.Paths/Utilities/PathHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Runtime.CompilerServices;
using JetBrains.Annotations;
using Reloaded.Memory.Extensions;
using Reloaded.Memory.Pointers;

namespace NexusMods.Paths.Utilities;

Expand Down Expand Up @@ -367,6 +368,7 @@ public static int JoinParts(Span<char> buffer, ReadOnlySpan<char> left, ReadOnly
return left.Length + DirectorySeparatorString.Length + right.Length;
}

#pragma warning disable CS8500 // This takes the address of, gets the size of, or declares a pointer to a managed type
/// <summary>
/// Joins two path parts together and returns the joined path as a string.
/// </summary>
Expand All @@ -388,30 +390,38 @@ public static string JoinParts(ReadOnlySpan<char> left, ReadOnlySpan<char> right
var spanLength = GetExactJoinedPartLength(left, right);
unsafe
{
// Note: We're deconstructing and reconstructing a span here, because we cannot
// pass ref-structs (Span) via a delegate (SpanAction). This is a workaround for
// said issue.

// This isn't the only way to pin, it's also possible to pin via refs.
// Just that this is the more readable approach to the problem.
fixed (char* leftPtr = left)
fixed (char* rightPtr = right)
// Note: The two Span objects are on the Stack. We access them inside
// string.Create, by dereferencing these items from the stack.

// If a GC happens, the pointers inside these referenced items will be
// moved, but our stack objects won't. Therefore, access like this without
// an explicit pin is safe.
// A similar trick also exists out there known as 'ref pinning'.

// Don't believe me? Go crazy with `DOTNET_GCStress` 😉 - Sewer
var @params = new JoinPartsParams
{
return string.Create(spanLength, (((IntPtr)leftPtr, left.Length), ((IntPtr)rightPtr, right.Length), os), (span, tuple) =>
{
// ReSharper disable InconsistentNaming
var (left_, right_, os_) = tuple;
// ReSharper restore InconsistentNaming

var leftSpan = new ReadOnlySpan<char>((void*)left_.Item1, left_.Length);
var rightSpan = new ReadOnlySpan<char>((void*)right_.Item1, right_.Length);
var count = JoinParts(span, leftSpan, rightSpan, os_);
Debug.Assert(count == spanLength, $"Calculated span length '{spanLength}' doesn't match actual span length '{count}'");
});
}
Left = &left,
Right = &right,
Os = os
};

return string.Create(spanLength, @params, (span, tuple) =>
{
var count = JoinParts(span, *tuple.Left, *tuple.Right, tuple.Os);
Debug.Assert(count == spanLength, $"Calculated span length '{spanLength}' doesn't match actual span length '{count}'");
});
}
}

unsafe struct JoinPartsParams
{
internal ReadOnlySpan<char>* Left;
internal ReadOnlySpan<char>* Right;
internal IOSInformation Os;
}
Copy link
Member

Choose a reason for hiding this comment

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

The struct should be marked as private as well as readonly.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll do it when I get back to my machine, dw

#pragma warning restore CS8500 // This takes the address of, gets the size of, or declares a pointer to a managed type

/// <summary>
/// Joins two path parts together and returns the joined path as a string.
/// </summary>
Expand Down
Loading