Rewrite DbBuffer Read/Write methods with stackalloc #88395
Conversation
ghost
added
the
community-contribution
|
Tagging subscribers to this area: @roji, @ajcvickers Issue DetailsA memory optimisation for DbBuffer class, avoid GC allocation when possible
|
src/libraries/System.Data.Odbc/src/Common/System/Data/ProviderBase/DbBuffer.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Data.Odbc/src/Common/System/Data/ProviderBase/DbBuffer.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Data.Odbc/src/Common/System/Data/ProviderBase/DbBuffer.cs
Show resolved
Hide resolved
- rewrite WriteGuid
src/libraries/System.Data.Odbc/src/Common/System/Data/ProviderBase/DbBuffer.cs
Outdated
Show resolved
Hide resolved
byte* addr = (byte*)DangerousGetHandle() + (uint)offset;
*addr = value; |
src/libraries/System.Data.Odbc/src/Common/System/Data/ProviderBase/DbBuffer.cs
Show resolved
Hide resolved
src/libraries/System.Data.Odbc/src/Common/System/Data/ProviderBase/DbBuffer.cs
Outdated
Show resolved
Hide resolved
|
On a code style note, it would be preferable to use the |
src/libraries/System.Data.Odbc/src/Common/System/Data/ProviderBase/DbBuffer.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Data.Odbc/src/Common/System/Data/ProviderBase/DbBuffer.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Data.Odbc/src/Common/System/Data/ProviderBase/DbBuffer.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Data.Odbc/src/Common/System/Data/ProviderBase/DbBuffer.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Data.Odbc/src/Common/System/Data/ProviderBase/DbBuffer.cs
Outdated
Show resolved
Hide resolved
|
|
||
| Validate(offset, sizeof(Guid)); | ||
|
|
||
| Debug.Assert(0 == offset % /* alignof(Guid) */ sizeof(int), "invalid alignment"); |
There was a problem hiding this comment.
@roji, @ajcvickers Note for code owners, this change relaxes the alignment requirement from ADP.PtrSize. I believe this is acceptable, but it is still a change in behavior.
| try | ||
| { | ||
| DangerousAddRef(ref mustRelease); | ||
| *((Guid*)DangerousGetHandle() + (uint)offset) = value; |
There was a problem hiding this comment.
@roji, @ajcvickers Note for code owners, please check whether this cast meets alignment requirements, or whether Unsafe.WriteUnaligned should be used instead. Notably, the value offset is only validated in the debug build.
| Span<short> spanBuffer = stackalloc short[3]; | ||
| ReadInt16Array(offset, spanBuffer.Slice(0, 3)); | ||
| return new DateTime( | ||
| unchecked((ushort)buffer[0]), // Year | ||
| unchecked((ushort)buffer[1]), // Month | ||
| unchecked((ushort)buffer[2])); // Day | ||
| unchecked((ushort)spanBuffer[0]), // Year | ||
| unchecked((ushort)spanBuffer[1]), // Month | ||
| unchecked((ushort)spanBuffer[2])); // Day |
There was a problem hiding this comment.
This could also be rewritten as unsafe method:
[StructLayout(LayoutKind.Sequential)]
public unsafe readonly struct DateData
{
public readonly ushort Year;
public readonly ushort Month;
public readonly ushort Day;
}
DateData data = *(DateData*)addr;
return new DateTime(data.Year, data.Month, data.Day);
|
@Poppyto thanks for the contribution, sorry for the delay in responding. @roji @ajcvickers do we want to accept this kind of change into Odbc? It does not have a readme indicating we only take necessary changes (like eg codedom has). If we aren't taking changes like this, we should create a readme; if we are, hopefuly we can review the change. |
|
@SamMonoRT, what would you like to see happen with this PR? Thanks. |
Following up |
|
We are not taking changes to System.Data.Odbc. We will add a ReadMe for the same. cc @roji |
A memory optimisation for DbBuffer class, avoid GC allocation when possible