Skip to content

Spanify SqlDecimal internals #1155

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.

Sign up for GitHub

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

Merged
roji merged 3 commits into from
Jan 23, 2020
Merged

Spanify SqlDecimal internals #1155

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3be442d
spanify SqlDecimal
Wraith2 Dec 26, 2019
3f755f5
fixup span clear
Wraith2 Dec 26, 2019
5848fbe
fixup parameter direction comment
Wraith2 Jan 16, 2020
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
77 changes: 39 additions & 38 deletions src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLDecimal.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@ public SqlDecimal(double dVal) : this(false)
AssertValid();
}

private SqlDecimal(uint[] rglData, byte bLen, byte bPrec, byte bScale, bool fPositive)
private SqlDecimal(ReadOnlySpan<uint> rglData, byte bLen, byte bPrec, byte bScale, bool fPositive)
{
CheckValidPrecScale(bPrec, bScale);
Debug.Assert(rglData.Length >= 4);
Expand Down Expand Up @@ -924,11 +924,11 @@ public override string ToString()
AssertValid();

// Make local copy of data to avoid modifying input.
uint[] rgulNumeric = new uint[4] { _data1, _data2, _data3, _data4 };
Span<uint> rgulNumeric = stackalloc uint[4] { _data1, _data2, _data3, _data4 };
int culLen = _bLen;
char[] pszTmp = new char[s_NUMERIC_MAX_PRECISION + 1]; //Local Character buffer to hold
//the decimal digits, from the
//lowest significant to highest significant
Span<char> pszTmp = stackalloc char[s_NUMERIC_MAX_PRECISION + 1]; //Local Character buffer to hold
pszTmp.Clear(); //the decimal digits, from the
//lowest significant to highest significant

int iDigits = 0; //Number of significant digits
uint ulRem; //Remainder of a division by x_ulBase10, i.e.,least significant digit
Expand Down Expand Up @@ -1263,8 +1263,8 @@ public static explicit operator decimal(SqlDecimal x)
culOp1 = x._bLen;
culOp2 = y._bLen;

uint[] rglData1 = new uint[4] { x._data1, x._data2, x._data3, x._data4 };
uint[] rglData2 = new uint[4] { y._data1, y._data2, y._data3, y._data4 };
Span<uint> rglData1 = stackalloc uint[4] { x._data1, x._data2, x._data3, x._data4 };
Span<uint> rglData2 = stackalloc uint[4] { y._data1, y._data2, y._data3, y._data4 };

if (fOpSignPos)
{
Expand Down Expand Up @@ -1312,7 +1312,7 @@ public static explicit operator decimal(SqlDecimal x)
if (x.LAbsCmp(y) < 0)
{
fResSignPos = !fResSignPos;
uint[] rguiTemp = rglData2;
Span<uint> rguiTemp = rglData2;
rglData2 = rglData1;
rglData1 = rguiTemp;
culOp1 = culOp2;
Expand Down Expand Up @@ -1449,13 +1449,14 @@ public static explicit operator decimal(SqlDecimal x)

// II) Perform multiplication

uint[] rglData1 = new uint[4] { x._data1, x._data2, x._data3, x._data4 };
uint[] rglData2 = new uint[4] { y._data1, y._data2, y._data3, y._data4 };
ReadOnlySpan<uint> rglData1 = stackalloc uint[4] { x._data1, x._data2, x._data3, x._data4 };
ReadOnlySpan<uint> rglData2 = stackalloc uint[4] { y._data1, y._data2, y._data3, y._data4 };

//Local buffer to hold the result of multiplication.
//Longer than CReNumeBuf because full precision of multiplication is carried out
const int x_culNumeMultRes = 9; // Maximum # UI4s in result buffer in multiplication
uint[] rgulRes = new uint[x_culNumeMultRes]; //new [] are already initialized to zero
Span<uint> rgulRes = stackalloc uint[x_culNumeMultRes]; //new [] are already initialized to zero

@roji roji Dec 26, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think we need these comments - stackalloc'd memory simply has undefined contents according to the language specs.

@Wraith2 Wraith2 Dec 27, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added the second comment to complement the original. You're right that it's undefined per-spec but that's unusual because all other allocated memory is zeroed or initialized to stated values. I think it's worth leaving in given that it's a comment and has no runtime cost.

@roji roji Dec 27, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure, it's true there's no runtime costs, but the comments simply describe how spans work, and is fairly evident to anyone doing high-perf work. For someone new to the business, yeah, it may look weird - but I don't think that's a good reason to have a comment. But it's just a comment, if you feel strongly about it I don't mind that much.

@Wraith2 Wraith2 Dec 27, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure about strongly, but the reason I put it there was that I made the mistake of not clearing it and that broke a test.

You're right that anyone doing high performance work should be aware of these things but I think it can ease the cognitive load on even those people sometimes if you just explain a little about what's going on. I do a lot of code archaeology in my day job and a small comment explaining the why of something can make a big difference.

rgulRes.Clear(); // but spans in core libraries are not for performance reasons so clear it
int culRes; // # of UI4s in result
int idRes = 0;

Expand Down Expand Up @@ -1682,12 +1683,14 @@ public static explicit operator decimal(SqlDecimal x)

// Step2: Actual Computation

uint[] rgulData1 = new uint[4] { x._data1, x._data2, x._data3, x._data4 };
uint[] rgulData2 = new uint[4] { y._data1, y._data2, y._data3, y._data4 };
Span<uint> rgulData1 = stackalloc uint[4] { x._data1, x._data2, x._data3, x._data4 };
Span<uint> rgulData2 = stackalloc uint[4] { y._data1, y._data2, y._data3, y._data4 };

// Buffers for arbitrary precision divide
uint[] rgulR = new uint[s_cNumeMax + 1];
uint[] rgulQ = new uint[s_cNumeMax];
Span<uint> rgulR = stackalloc uint[s_cNumeMax + 1];
Span<uint> rgulQ = stackalloc uint[s_cNumeMax];
rgulR.Clear();
rgulQ.Clear();
// # of ULONGs in result
int culQ, culR;

Expand Down Expand Up @@ -1788,7 +1791,7 @@ private void AssertValid()
Debug.Assert(CLenFromPrec(_bPrec) >= _bLen, "CLenFromPrec(m_bPrec) >= m_bLen", "In AssertValid");
Debug.Assert(_bLen <= s_cNumeMax, "m_bLen <= x_cNumeMax", "In AssertValid");

uint[] rglData = new uint[4] { _data1, _data2, _data3, _data4 };
ReadOnlySpan<uint> rglData = stackalloc uint[4] { _data1, _data2, _data3, _data4 };

// highest UI4 is non-0 unless value "zero"
if (rglData[_bLen - 1] == 0)
Expand Down Expand Up @@ -1828,7 +1831,7 @@ private void Print(String s) {
}
*/
// Set all extra uints to zero
private static void ZeroToMaxLen(uint[] rgulData, int cUI4sCur)
private static void ZeroToMaxLen(Span<uint> rgulData, int cUI4sCur)
{
Debug.Assert(rgulData.Length == s_cNumeMax, "rgulData.Length == x_cNumeMax", "Invalid array length");

Expand Down Expand Up @@ -1905,7 +1908,7 @@ private bool FGt10_38()
(_data3 == 0x5a86c47aL) && (_data2 >= 0x098a2240L));
}

private bool FGt10_38(uint[] rglData)
private bool FGt10_38(Span<uint> rglData)
{
Debug.Assert(rglData.Length == 4, "rglData.Length == 4", "Wrong array length: " + rglData.Length.ToString(CultureInfo.InvariantCulture));

Expand Down Expand Up @@ -2062,7 +2065,7 @@ private byte BActualPrec()
}
else
{
uint[] rgulU = new uint[4] { _data1, _data2, _data3, _data4 };
Span<uint> rgulU = stackalloc uint[4] { _data1, _data2, _data3, _data4 };
Prec = 0;
do
{
Expand Down Expand Up @@ -2093,7 +2096,7 @@ private void AddULong(uint ulAdd)
int iData; // which UI4 in this we are on
int iDataMax = _bLen; // # of UI4s in this

uint[] rguiData = new uint[4] { _data1, _data2, _data3, _data4 };
Span<uint> rguiData = stackalloc uint[4] { _data1, _data2, _data3, _data4 };

// Add, starting at the LS UI4 until out of UI4s or no carry
iData = 0;
Expand Down Expand Up @@ -2138,7 +2141,7 @@ private void MultByULong(uint uiMultiplier)
ulong dwlNextAccum = 0; // accumulation past dwlAccum
int iData; // which UI4 in *This we are on.

uint[] rguiData = new uint[4] { _data1, _data2, _data3, _data4 };
Span<uint> rguiData = stackalloc uint[4] { _data1, _data2, _data3, _data4 };

for (iData = 0; iData < iDataMax; iData++)
{
Expand Down Expand Up @@ -2198,7 +2201,7 @@ private uint DivByULong(uint iDivisor)
throw new DivideByZeroException(SQLResource.DivideByZeroMessage);

// Copy into array, so that we can iterate through the data
uint[] rguiData = new uint[4] { _data1, _data2, _data3, _data4 };
Span<uint> rguiData = stackalloc uint[4] { _data1, _data2, _data3, _data4 };

// Start from the MS UI4 of quotient, divide by divisor, placing result
// in quotient and carrying the remainder.
Expand Down Expand Up @@ -2408,8 +2411,8 @@ private int LAbsCmp(SqlDecimal snumOp)
if (culOp != culThis)
return (culThis > culOp) ? 1 : -1;

uint[] rglData1 = new uint[4] { _data1, _data2, _data3, _data4 };
uint[] rglData2 = new uint[4] { snumOp._data1, snumOp._data2, snumOp._data3, snumOp._data4 };
ReadOnlySpan<uint> rglData1 = stackalloc uint[4] { _data1, _data2, _data3, _data4 };
ReadOnlySpan<uint> rglData2 = stackalloc uint[4] { snumOp._data1, snumOp._data2, snumOp._data3, snumOp._data4 };

// Loop through numeric value checking each byte for differences.
iData = culOp - 1;
Expand All @@ -2430,9 +2433,9 @@ private int LAbsCmp(SqlDecimal snumOp)
// Move multi-precision number
private static void MpMove
(
uint[] rgulS, // In | Source number
ReadOnlySpan<uint> rgulS, // In | Source number
int ciulS, // In | # of digits in S
uint[] rgulD, // Out | Destination number
Span<uint> rgulD, // Out | Destination number
out int ciulD // Out | # of digits in D
)
{
Expand All @@ -2448,7 +2451,7 @@ out int ciulD // Out | # of digits in D
// Set multi-precision number to one super-digit
private static void MpSet
(
uint[] rgulD, // Out | Number
Span<uint> rgulD, // Out | Number
out int ciulD, // Out | # of digits in D
uint iulN // In | ULONG to set
)
Expand All @@ -2460,7 +2463,7 @@ uint iulN // In | ULONG to set
// Normalize multi-precision number - remove leading zeroes
private static void MpNormalize
(
uint[] rgulU, // In | Number
ReadOnlySpan<uint> rgulU, // In | Number
ref int ciulU // InOut| # of digits
)
{
Expand All @@ -2473,7 +2476,7 @@ ref int ciulU // InOut| # of digits
// Length can increase
private static void MpMul1
(
uint[] piulD, // InOut| D
Span<uint> piulD, // InOut| D
ref int ciulD, // InOut| # of digits in D
uint iulX // In | X
)
Expand Down Expand Up @@ -2505,7 +2508,7 @@ uint iulX // In | X
// Length of U can decrease
private static void MpDiv1
(
uint[] rgulU, // InOut| U
Span<uint> rgulU, // InOut| U
ref int ciulU, // InOut| # of digits in U
uint iulD, // In | D
out uint iulR // Out | R
Expand Down Expand Up @@ -2567,13 +2570,13 @@ private static uint LO(ulong x)
//
private static void MpDiv
(
uint[] rgulU, // In | U
ReadOnlySpan<uint> rgulU, // In | U
int ciulU, // In | # of digits in U
uint[] rgulD, // In | D
Span<uint> rgulD, // InOut | D
int ciulD, // In | # of digits in D
uint[] rgulQ, // Out | Q
Span<uint> rgulQ, // Out | Q
out int ciulQ, // Out | # of digits in Q
uint[] rgulR, // Out | R
Span<uint> rgulR, // Out | R
out int ciulR // Out | # of digits in R
)
{
Expand Down Expand Up @@ -2998,7 +3001,7 @@ private static char ChFromDigit(uint uiDigit)
}

// Store data back from rguiData[] to m_data*
private void StoreFromWorkingArray(uint[] rguiData)
private void StoreFromWorkingArray(ReadOnlySpan<uint> rguiData)
{
Debug.Assert(rguiData.Length == 4);
_data1 = rguiData[0];
Expand Down Expand Up @@ -3254,10 +3257,8 @@ public static SqlDecimal Power(SqlDecimal n, double exp)
// If object is not of same type, this method throws an ArgumentException.
public int CompareTo(object value)
{
if (value is SqlDecimal)
if (value is SqlDecimal i)
{
SqlDecimal i = (SqlDecimal)value;

return CompareTo(i);
}
throw ADP.WrongType(value.GetType(), typeof(SqlDecimal));
Expand Down