Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ public override unsafe int Read(byte[] buffer, int offset, int count)
fixed (byte* pDst = buffer)
{
Debug.Assert(pSrc is not null);
Unsafe.CopyBlockUnaligned(pDst + offset, pSrc + bufferOffset, (uint)chunk);
Utilities.CopyWithAlignmentFallback(pDst + offset, pSrc + bufferOffset, (uint)chunk);
}
}

Expand Down Expand Up @@ -511,7 +511,7 @@ public override unsafe void Write(byte[] buffer, int offset, int count)
fixed (byte* pDst = dataBuffer)
{
Debug.Assert(pDst is not null);
Unsafe.CopyBlockUnaligned(pDst + bufferOffset, pSrc + offset, (uint)chunk);
Utilities.CopyWithAlignmentFallback(pDst + bufferOffset, pSrc + offset, (uint)chunk);
}
}

Expand Down
2 changes: 1 addition & 1 deletion sources/core/Stride.Core.Serialization/Storage/Blob.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ internal unsafe Blob(ObjectDatabase objectDatabase, ObjectId objectId, IntPtr co
Debug.Assert(size >= 0);
this.Size = size;
this.Content = Marshal.AllocHGlobal(size);
Unsafe.CopyBlockUnaligned((void*)this.Content, (void*)content, (uint)size);
Utilities.CopyWithAlignmentFallback((void*)this.Content, (void*)content, (uint)size);
}

internal unsafe Blob(ObjectDatabase objectDatabase, ObjectId objectId, Stream stream)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public unsafe IntPtr GetData(DatabaseFileProvider fileProvider)
var read = (uint)stream.Read(buffer, 0, (int)Math.Min(count, bufferCapacity));
if (read <= 0)
break;
Unsafe.CopyBlockUnaligned(chunkBytesPtr, bufferStart, read);
Utilities.CopyWithAlignmentFallback(chunkBytesPtr, bufferStart, read);
chunkBytesPtr += read;
count -= read;
} while (count > 0);
Expand Down
22 changes: 10 additions & 12 deletions sources/core/Stride.Core/UnmanagedArray.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,13 @@ namespace Stride.Core;
[Obsolete("Obtain Memory<T> using GC.Allocate*Array or a Stride-specific allocator mechanism.")]
public class UnmanagedArray<T> : IDisposable where T : struct
{
private readonly int sizeOfT;
private readonly bool isShared;

[Obsolete("Obtain Memory<T> using GC.Allocate*Array or a Stride-specific allocator mechanism.")]
public UnmanagedArray(int length)
{
Length = length;
sizeOfT = Unsafe.SizeOf<T>();
var finalSize = length * sizeOfT;
var finalSize = length * Unsafe.SizeOf<T>();
Pointer = Utilities.AllocateMemory(finalSize);
isShared = false;
}
Expand All @@ -43,8 +41,10 @@ public T this[int index]
unsafe
{
var bptr = (byte*)Pointer;
bptr += index * sizeOfT;
res = Unsafe.ReadUnaligned<T>(bptr);
bptr += index * Unsafe.SizeOf<T>();
// Pointer is aligned, we expect the struct to be aligned as well;
// If the user decides to Pack=1, this scope is the least of their worries
res = Unsafe.Read<T>(bptr);
}

return res;
Expand All @@ -60,8 +60,10 @@ public T this[int index]
unsafe
{
var bptr = (byte*)Pointer;
bptr += index * sizeOfT;
Unsafe.WriteUnaligned(bptr, value);
bptr += index * Unsafe.SizeOf<T>();
// Pointer is aligned, we expect the struct to be aligned as well;
// If the user decides to Pack=1, this scope is the least of their worries
Unsafe.Write(bptr, value);
}
}
}
Expand All @@ -72,7 +74,6 @@ public unsafe void Read(T[] destination, int offset = 0)
{
throw new ArgumentOutOfRangeException(nameof(offset));
}
// Interop.Read((void*)Pointer, destination, offset, destination.Length);
new Span<T>((void*)Pointer, destination.Length - offset).CopyTo(destination.AsSpan(offset));
}

Expand All @@ -87,8 +88,7 @@ public void Read(T[] destination, int pointerByteOffset, int arrayOffset, int ar
{
var ptr = (byte*)Pointer;
ptr += pointerByteOffset;
// Interop.Read(ptr, destination, arrayOffset, arrayLen);
new Span<T>(ptr, sizeOfT * arrayLen)
new Span<T>(ptr, Unsafe.SizeOf<T>() * arrayLen)
.CopyTo(destination.AsSpan(arrayOffset, arrayLen));
}
}
Expand All @@ -99,7 +99,6 @@ public unsafe void Write(T[] source, int offset = 0)
{
throw new ArgumentOutOfRangeException();
}
// Interop.Write((void*)Pointer, source, offset, source.Length);
source.AsSpan(offset).CopyTo(new Span<T>((void*)Pointer, source.Length - offset));
}

Expand All @@ -112,7 +111,6 @@ public unsafe void Write(T[] source, int pointerByteOffset, int arrayOffset, int

var ptr = (byte*)Pointer;
ptr += pointerByteOffset;
// Interop.Write(ptr, source, arrayOffset, arrayLen);
source.AsSpan(arrayOffset, arrayLen).CopyTo(new Span<T>(ptr, arrayLen));
}

Expand Down
44 changes: 44 additions & 0 deletions sources/core/Stride.Core/Utilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,50 @@ namespace Stride.Core;
/// </summary>
public static class Utilities
{
// MUST BE A METHOD AND AGGRESSIVELY INLINED, OTHERWISE THE JIT WILL NOT ELIMINATE THE BRANCH
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static bool IsUnalignedSafe() =>

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.

What if it is a static readonly field? Theoretically, doesn't the JIT evaluate those only once and uses them like a const later on?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

You would think, but nope, here's disasm:

; Method C:CopyWithAlignmentFallback2(byref,byref,uint):float (FullOpts)
G_M000_IG01:                ;; offset=0x0000
       sub      rsp, 40

G_M000_IG02:                ;; offset=0x0004
       test     byte  ptr [(reloc 0x7ffb0f9b41d8)], 1
       je       SHORT G_M000_IG08

G_M000_IG03:                ;; offset=0x000D
       cmp      byte  ptr [(reloc 0x7ffb0f6fb090)], 0
       jne      SHORT G_M000_IG06

G_M000_IG04:                ;; offset=0x0016
       vmovss   xmm0, dword ptr [reloc @RWD00]

G_M000_IG05:                ;; offset=0x001E
       add      rsp, 40
       ret      

G_M000_IG06:                ;; offset=0x0023
       vxorps   xmm0, xmm0, xmm0

G_M000_IG07:                ;; offset=0x0027
       add      rsp, 40
       ret      

G_M000_IG08:                ;; offset=0x002C
       mov      rcx, 0x7FFB0F9B4168
       call     CORINFO_HELP_GET_NONGCSTATIC_BASE
       jmp      SHORT G_M000_IG03
RWD00  	dd	3F800000h		;         1

While this one is:

; Method C:CopyWithAlignmentFallback2(byref,byref,uint):float (FullOpts)
G_M000_IG01:                ;; offset=0x0000

G_M000_IG02:                ;; offset=0x0000
       vxorps   xmm0, xmm0, xmm0

G_M000_IG03:                ;; offset=0x0004
       ret      
; Total bytes of code: 5

Godbolt shows similar overhead, so this is not a config issue.

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.

Maybe one of those things that it does when doing PGO and tiering?
Anyway, not that important.

RuntimeInformation.ProcessArchitecture is Architecture.X64 or Architecture.X86 or Architecture.Arm64;

/// <inheritdoc cref="AlignmentFallbackDoc"/>
/// <inheritdoc cref="Unsafe.CopyBlock(ref byte, ref readonly byte, uint)"/>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static unsafe void CopyWithAlignmentFallback(ref byte destination, ref readonly byte source, uint byteCount)
{
if (IsUnalignedSafe())
fixed (void* src = &source, dst = &destination)
Buffer.MemoryCopy(src, dst, byteCount, byteCount);
Comment on lines +48 to +49

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.

Why not use Span<T>.CopyTo() in these cases. It seems it does a Buffer.Memmove internally, the same as Buffer.MemoryCopy (link).

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.

Also, for cases where we are sure the memory is aligned (like those where we control the type and ensure proper layouts), you also have the option of Unsafe.CopyBlock

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I chose Buffer.MemoryCopy because it's more straightforward than creating two spans and using Span.CopyTo between them. But as you said, they map to the same method, so if span does not provide anything else compared to buffer I think that this is kind of bikeshedd-y ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

While Unsafe.CopyBlock is an intrinsic, it is not optimized for the wide array of ranges one might have when making copies

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.

For these utility methods, it's enough to use Buffer.MemoryCopy, yes.
My point is at those call sites where we could be using a Span. Maybe we have more of those in the future, as more of the API is converted to use/accept spans.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yep

else
Unsafe.CopyBlockUnaligned(ref destination, in source, byteCount);
}

/// <inheritdoc cref="AlignmentFallbackDoc"/>
/// <inheritdoc cref="Unsafe.CopyBlock(ref byte, ref readonly byte, uint)"/>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static unsafe void CopyWithAlignmentFallback(void* destination, void* source, uint byteCount)
{
if (IsUnalignedSafe())
Buffer.MemoryCopy(source, destination, byteCount, byteCount);
else
Unsafe.CopyBlockUnaligned(destination, source, byteCount);
}

/// <summary>
/// Zero out memory at the address provided
/// </summary>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static unsafe void Clear(void* startAddress, uint byteCount)
{
// Span swaps between InitBlockUnaligned and _ZeroMemory depending on the size
new Span<byte>(startAddress, (int)byteCount).Clear();
}

/// <remarks>
/// Some of the architecture dotnet runs on do not support arbitrary unaligned reads or writes,
/// use this instead of other memcopy if you aren't sure whether the pointers you passed in are aligned.
/// </remarks>
static void AlignmentFallbackDoc() { }

/// <summary>
/// Allocate an aligned memory buffer.
/// </summary>
Expand Down
10 changes: 5 additions & 5 deletions sources/engine/Stride.Engine/Animations/AnimationBlender.cs
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,15 @@ public static unsafe void Blend(CoreAnimationOperation blendOperation, float ble
if (factorLeft > 0.0f && factorRight == 0.0f)
{
*resultData++ = 1.0f;
Unsafe.CopyBlockUnaligned(resultData, sourceLeftData, (uint)channel.Size);
Utilities.CopyWithAlignmentFallback(resultData, sourceLeftData, (uint)channel.Size);
continue;
}

// Use right value
if (factorRight > 0.0f && factorLeft == 0.0f)
{
*resultData++ = 1.0f;
Unsafe.CopyBlockUnaligned(resultData, sourceRightData, (uint)channel.Size);
Utilities.CopyWithAlignmentFallback(resultData, sourceRightData, (uint)channel.Size);
continue;
}

Expand All @@ -202,7 +202,7 @@ public static unsafe void Blend(CoreAnimationOperation blendOperation, float ble
switch (channel.BlendType)
{
case BlendType.Blit:
Unsafe.CopyBlockUnaligned(
Utilities.CopyWithAlignmentFallback(
resultData,
blendFactor < 0.5f ? sourceLeftData : sourceRightData,
(uint)channel.Size);
Expand All @@ -229,7 +229,7 @@ public static unsafe void Blend(CoreAnimationOperation blendOperation, float ble
switch (channel.BlendType)
{
case BlendType.Blit:
Unsafe.CopyBlockUnaligned(resultData, sourceLeftData, (uint)channel.Size);
Utilities.CopyWithAlignmentFallback(resultData, sourceLeftData, (uint)channel.Size);
break;
case BlendType.Float2:
Vector2 rightValue2;
Expand Down Expand Up @@ -257,7 +257,7 @@ public static unsafe void Blend(CoreAnimationOperation blendOperation, float ble
switch (channel.BlendType)
{
case BlendType.Blit:
Unsafe.CopyBlockUnaligned(resultData, sourceLeftData, (uint)channel.Size);
Utilities.CopyWithAlignmentFallback(resultData, sourceLeftData, (uint)channel.Size);
break;
case BlendType.Float2:
Vector2 rightValue2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public unsafe void AddCurveValues(CompressedTimeSpan newTime, AnimationClipResul
var channel = Channels.Items[index];

// For now, objects are not supported, so treat everything as a blittable struct.
channel.Curve?.AddValue(newTime, (IntPtr)(structures + channel.Offset + sizeof(float)));
channel.Curve?.AddValue(newTime, (structures + channel.Offset + sizeof(float)));
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions sources/engine/Stride.Engine/Animations/AnimationCurve.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public abstract class AnimationCurve
/// </summary>
/// <param name="newTime">The new time.</param>
/// <param name="location">The location.</param>
public abstract void AddValue(CompressedTimeSpan newTime, IntPtr location);
public abstract unsafe void AddValue(CompressedTimeSpan newTime, byte* location);

/// <summary>
/// Meant for internal use, to call AnimationData{T}.FromAnimationChannels() without knowing the generic type.
Expand Down Expand Up @@ -142,9 +142,9 @@ public int FindKeyIndex(CompressedTimeSpan time)
}

/// <inheritdoc/>
public override unsafe void AddValue(CompressedTimeSpan newTime, nint location)
public override unsafe void AddValue(CompressedTimeSpan newTime, byte* location)
{
var value = Unsafe.ReadUnaligned<T>((void*)location);
var value = Unsafe.ReadUnaligned<T>(location);
KeyFrames.Add(new(newTime, value));
}

Expand Down
2 changes: 1 addition & 1 deletion sources/engine/Stride.Engine/Updater/UpdatableField.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ ldarg data
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public unsafe void SetBlittable(IntPtr obj, IntPtr data)
{
Unsafe.CopyBlockUnaligned((void*)obj, (void*)data, (uint)Size);
Utilities.CopyWithAlignmentFallback((void*)obj, (void*)data, (uint)Size);
}

/// <summary>
Expand Down
4 changes: 2 additions & 2 deletions sources/engine/Stride.Graphics/Buffer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ public unsafe void GetData<T>(CommandList commandList, Buffer stagingTexture, Sp
// Map the staging resource to a CPU accessible memory
var mappedResource = commandList.MapSubresource(stagingTexture, 0, MapMode.Read);
fixed (void* pointer = toData)
Unsafe.CopyBlockUnaligned(pointer, (void*)mappedResource.DataBox.DataPointer, (uint)toDataInBytes);
Utilities.CopyWithAlignmentFallback(pointer, (void*)mappedResource.DataBox.DataPointer, (uint)toDataInBytes);
// Make sure that we unmap the resource in case of an exception
commandList.UnmapSubresource(mappedResource);
}
Expand Down Expand Up @@ -386,7 +386,7 @@ public unsafe void SetData<T>(CommandList commandList, ReadOnlySpan<T> fromData,
throw new ArgumentException("offset is only supported for textured declared with ResourceUsage.Default", "offsetInBytes");

var mappedResource = commandList.MapSubresource(this, 0, Usage == GraphicsResourceUsage.Staging ? MapMode.Write : MapMode.WriteDiscard);
Unsafe.CopyBlockUnaligned((void*)mappedResource.DataBox.DataPointer, pointer, (uint)sizeInBytes);
Utilities.CopyWithAlignmentFallback((void*)mappedResource.DataBox.DataPointer, pointer, (uint)sizeInBytes);
commandList.UnmapSubresource(mappedResource);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ private static unsafe void DeserializeImage(ContentManager contentManager, Image
if (!chunk.IsLoaded)
throw new ContentStreamingException("Data chunk is not loaded.", storage);

Unsafe.CopyBlockUnaligned((void*)bufferPtr, (void*)data, (uint)chunk.Size);
Utilities.CopyWithAlignmentFallback((void*)bufferPtr, (void*)data, (uint)chunk.Size);
bufferPtr += chunk.Size;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public unsafe void Recreate(IntPtr dataPointer)
if (heapType == HeapType.Upload)
{
var uploadMemory = NativeResource.Map(0);
Unsafe.CopyBlockUnaligned((void*) uploadMemory, (void*) dataPointer, (uint) SizeInBytes);
Core.Utilities.CopyWithAlignmentFallback((void*) uploadMemory, (void*) dataPointer, (uint) SizeInBytes);
NativeResource.Unmap(0);
}
else
Expand All @@ -132,7 +132,7 @@ public unsafe void Recreate(IntPtr dataPointer)
SharpDX.Direct3D12.Resource uploadResource;
int uploadOffset;
var uploadMemory = GraphicsDevice.AllocateUploadBuffer(SizeInBytes, out uploadResource, out uploadOffset);
Unsafe.CopyBlockUnaligned((void*) uploadMemory, (void*) dataPointer, (uint) SizeInBytes);
Core.Utilities.CopyWithAlignmentFallback((void*) uploadMemory, (void*) dataPointer, (uint) SizeInBytes);

// TODO D3D12 lock NativeCopyCommandList usages
var commandList = GraphicsDevice.NativeCopyCommandList;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@ public unsafe void Copy(GraphicsResource source, GraphicsResource destination)
var destinationMapped = destinationTexture.NativeResource.Map(0);
var sourceMapped = sourceTexture.NativeResource.Map(0, new SharpDX.Direct3D12.Range { Begin = 0, End = size });

Unsafe.CopyBlockUnaligned((void*) destinationMapped, (void*) sourceMapped, (uint) size);
Core.Utilities.CopyWithAlignmentFallback((void*) destinationMapped, (void*) sourceMapped, (uint) size);

sourceTexture.NativeResource.Unmap(0);
destinationTexture.NativeResource.Unmap(0);
Expand Down Expand Up @@ -976,7 +976,7 @@ internal unsafe void UpdateSubresource(GraphicsResource resource, int subResourc
var uploadSize = region.Right - region.Left;
var uploadMemory = GraphicsDevice.AllocateUploadBuffer(region.Right - region.Left, out var uploadResource, out var uploadOffset);

Unsafe.CopyBlockUnaligned((void*) uploadMemory, (void*) databox.DataPointer, (uint) uploadSize);
Core.Utilities.CopyWithAlignmentFallback((void*) uploadMemory, (void*) databox.DataPointer, (uint) uploadSize);

ResourceBarrierTransition(resource, GraphicsResourceState.CopyDestination);
FlushResourceBarriers();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public unsafe bool TryGetData(long[] dataArray)
var mappedData = readbackBuffer.Map(0);
fixed (long* dataPointer = &dataArray[0])
{
Unsafe.CopyBlockUnaligned(dataPointer, (void*) mappedData, (uint) QueryCount * 8);
Core.Utilities.CopyWithAlignmentFallback(dataPointer, (void*) mappedData, (uint) QueryCount * 8);
}
readbackBuffer.Unmap(subresource: 0);
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ private unsafe void InitializeFromImpl(DataBox[] dataBoxes = null)
var dataPointerCurrent = databox.DataPointer;
for (int rowIndex = 0; rowIndex < mipHeight; rowIndex++)
{
Unsafe.CopyBlockUnaligned((void*) uploadMemoryCurrent, (void*) dataPointerCurrent, (uint) mipRowPitch);
Core.Utilities.CopyWithAlignmentFallback((void*) uploadMemoryCurrent, (void*) dataPointerCurrent, (uint) mipRowPitch);
uploadMemoryCurrent += mipRowPitch;
dataPointerCurrent += databox.RowPitch;
}
Expand Down Expand Up @@ -208,7 +208,7 @@ private unsafe void InitializeFromImpl(DataBox[] dataBoxes = null)
var dataPointerCurrent = dataPointer + z * databox.SlicePitch;
for (int y = 0; y < rowCount; ++y)
{
Unsafe.CopyBlockUnaligned((void*) uploadMemoryCurrent, (void*) dataPointerCurrent, (uint) rowSize);
Utilities.CopyWithAlignmentFallback((void*) uploadMemoryCurrent, (void*) dataPointerCurrent, (uint) rowSize);
uploadMemoryCurrent += destRowPitch;
dataPointerCurrent += databox.RowPitch;
}
Expand Down
Loading
Loading