Re-implemented the ecvt function. #12894

mazong1123 · 2017-07-19T07:20:06Z

Instead of leveraging snprintf, re-implement the ecvt function according to the paper: https://www.cs.indiana.edu/~dyb/pubs/FP-Printing-PLDI96.pdf

Note:

This commit won't fix any existing bug.
This is a raw implementation of the paper. The performance on Linux only gain 10%. We could tune the performance further.

UPDATE:

Now the new implementation is at least 2x faster than the old one.

Fix #10651

Instead of leveraging snprintf, re-implement the ecvt function according to the paper: https://www.cs.indiana.edu/~dyb/pubs/FP-Printing-PLDI96.pdf Note: 1. This commit won't fix any existing bug. 2. This is a raw implementation of the paper. The performance on Linux only gain 10%. We could tune the performance further. Fix #10651

mazong1123 · 2017-07-19T07:25:19Z

@tarekgh @jkotas This commit is a raw implementation of the paper https://www.cs.indiana.edu/~dyb/pubs/FP-Printing-PLDI96.pdf . The performance not gained so much (only 10%). I'll tune the performance further.

Currently, I want to go through the CI checks so that if there's any issue I can fix them earlier so I created the PR.

Note I did not change the "15 digit first, then 17 digit precision" round trip conversion rule. I think that should be done in another PR.

Fix #10651

mazong1123 · 2017-07-19T14:57:09Z

@tarekgh @jkotas PTAL.
I ran some tests on this PR. Following are the details:

Accuracy

I ran following code against this PR:

using System;

namespace dtost
{
    class Program
    {
        static void TestDoubleConversion(double number)
        {
            string str = number.ToString("R");
            double back = double.Parse(str);
            if (number != back)
            {
                Console.WriteLine("Test failed: converted str: {0}. expected double {1}", back, number);
            }
            else
            {
                Console.WriteLine("Pass");
            }
        }

        static void Main(string[] args)
        {
            TestDoubleConversion(104234.343);
            TestDoubleConversion(-1.7976931348623157E+308);
            TestDoubleConversion(1.7976931348623157e+308);
            TestDoubleConversion(1.7976931348623157e+308);
            TestDoubleConversion(70.9228162514264339123);
            TestDoubleConversion(3.1415926535897937784612345);
            TestDoubleConversion(29999999999999792458.0);
            TestDoubleConversion(1000.4999999999999999999);
            TestDoubleConversion(1000.9999999999999999999);
            TestDoubleConversion(999.99999999999999999999);
            TestDoubleConversion(0.84551240822557006);
            TestDoubleConversion(1.0);
            TestDoubleConversion(10.0);
            TestDoubleConversion(0);
            TestDoubleConversion(0.0);
            TestDoubleConversion(0.0000000000000199);
            TestDoubleConversion(-0.0000123);
        }
    }
}

The output is:

Pass
Pass
Pass
Pass
Pass
Pass
Pass
Pass
Pass
Pass
Test failed: converted str: 0.84551240822557. expected double 0.84551240822557
Pass
Pass
Pass
Pass
Pass
Pass

Ran the code under current .NET Core release got exactly the same result. The only failure is caused by a known issue https://stackoverflow.com/questions/24299692/why-is-a-round-trip-conversion-via-a-string-not-safe-for-a-double

So the accuracy of the new implementation should be the same as current .NET Core release.

Performance

I ran following micro-benchmark:

using System;
using System.Diagnostics;
using System.Globalization;

namespace hlold
{
    class Program
    {
        static void Main(string[] args)
        {
            double number = 104234.343;
            CultureInfo cultureInfo = new CultureInfo("fr");
            for (int i = 0; i < 20; i++)
            {
                Stopwatch watch = new Stopwatch();
                watch.Start();
                for (int j = 0; j < 10000; j++)
                {
                    number.ToString(cultureInfo); number.ToString(cultureInfo); number.ToString(cultureInfo);
                    number.ToString(cultureInfo); number.ToString(cultureInfo); number.ToString(cultureInfo);
                    number.ToString(cultureInfo); number.ToString(cultureInfo); number.ToString(cultureInfo);
                }

                Console.Write(watch.ElapsedMilliseconds);
                Console.WriteLine();
            }
        }
    }
}

The new implementation is 3x faster than the old one:

Output of new implementation:

Output of old implementation:

If we change the number to double.MinValue / 2, the new implementation is still 2x faster than the old one:

Output of new implementation:

Output of old implementation:

So the new implementation is at least 2x faster than the old one. I think we have proved that the algorithm is much more efficient than the old one.

tarekgh · 2017-07-19T16:22:15Z

@mazong1123 this is fantastic. the results look promising. I have enabled the whole corefx repo to run against your changes to know if we get any regression case. Also I am not seeing we used a big static data which is a good news too. if corefx goes fine we can move to the next step to check with the legal about using the algorithm and do official code review then we can finalize the work.

Thanks a lot for getting this done. good work!

tarekgh · 2017-07-19T17:17:58Z

@safern could you please take a look at the CI falure in ComponentModel?

Ubuntu x64 Checked Build and Test (Jit - CoreFx)

https://ci.dot.net/job/dotnet_coreclr/job/master/job/jitstress/job/x64_checked_ubuntu_corefx_baseline_prtest/62/consoleFull#-21281798079fe3b83-f408-404c-b9e7-9207d232e5fc

this is not urgent but it'll be good if you have a look when you have a chance.

@mazong1123 you may ignore the failure in "Ubuntu x64 Checked Build and Test (Jit - CoreFx)" for now as it looks unrelated to your changes.

safern · 2017-07-19T17:19:41Z

@safern could you please take a look at the CI falure in ComponentModel?

Will do later :)

tannergooding · 2017-07-19T21:08:21Z

I'm going to give the implementation a review pass sometime later this evening.

tarekgh · 2017-07-19T21:45:44Z

src/classlibnative/bcltype/bignum.cpp

+    const UINT32* pRhsEnd = pRhsCurrent + length;
+
+    while (pRhsCurrent != pRhsEnd)
+    {


can we just memcpy this part instead of looping?

It may be best to measure to see if there's any performance difference.

memcpy should do the right thing, regardless of size. That being said, the current max size is only 35 bytes so the perf difference is barely measurable. But, this size is way too small and should be well over 138 bytes (with exact size varying based on the maximum number of digits we consider). The current max size is 35 dwords (140 bytes).

why we don't just memcpy considering m_len even if you think the perf is barely measurable?

In reply to: 128531411 [](ancestors = 128531411)

tarekgh · 2017-07-19T21:59:19Z

src/classlibnative/bcltype/bignum.cpp

+
+    while (true)
+    {
+        if (*pCurrentLeftBlock > *pCurrentRightBlock)


nit: maybe we can do it as

INT64 diff = (INT64) (*pCurrentLeftBlock) - (INT64) (*pCurrentRightBlock);
if (diff != 0)
return diff;

👍, but we should validate that this doesn't perform any more poorly on 32-bit platforms

tarekgh · 2017-07-19T22:00:19Z

src/classlibnative/bcltype/bignum.cpp

+    const UINT32* pCurrentLeftBlock = lhs.m_blocks + lastIndex;
+    const UINT32* pCurrentRightBlock = rhs.m_blocks + lastIndex;
+
+    while (true)


true) [](start = 11, length = 5)

nit: while (pCurrentLeftBlock <= pLeftStartBlock) is better here and remove if (pCurrentLeftBlock == pLeftStartBlock) from the loop

I think you meant while (pCurrentLeftBlock >= pLeftStartBlock) :). I'll take this optimization.

tarekgh · 2017-07-19T22:02:12Z

src/classlibnative/bcltype/bignum.cpp

+
+void BigNum::ShiftLeft(UINT64 input, int shift, BigNum& output)
+{
+    int shiftBlocks = shift / 32;


int shiftBlocks = shift / 32; [](start = 4, length = 29)

shift >> 5

👍, although the compiler should also be doing this optimization for us.

Actually, we should probably just leave this. The compiler will optimize the next statement as well (into either a single idiv or into a couple of shifts and a subtract)

Using / and % is fine, no native compiler will generate actual div instructions for this code. However, I would suggest making shift unsigned because unsigned division by constant is cheaper than signed division. It may be the native compiler will figure this one too but it requires interprocedural optimizations so it's somewhat less certain.

@mikedn Good catch. I think make shift unsigned should be more reasonable here.

tannergooding · 2017-07-19T22:55:40Z

src/classlibnative/bcltype/number.cpp

+        realExponent = -1074;
+        mantissaHighBitIdx = BigNum::LogBase2(realMantissa);
+    }
+


You can shortcut Zero, Infinity, Indeterminate, and NaN by this point

These special cases has already been handled before passing to the method: https://github.com/dotnet/coreclr/blob/master/src/classlibnative/bcltype/number.cpp#L146

I think adding asserts is worth though.

It looks like zero and negative zero are not handled by the outer code (just NaN/Inf).

Yes, we should shortcut zeros here. Thanks. I'd like to borrow the code from https://github.com/dotnet/coreclr/blob/master/src/pal/src/cruntime/misctls.cpp#L185-L193

However, I do not quite understand the comment at https://github.com/dotnet/coreclr/blob/master/src/pal/src/cruntime/misctls.cpp#L184

// we have issue #10290 tracking fixing the sign of 0.0 across the platforms

Did this code introduce issue #10290 ?

@mazong1123, no. #10290 is tracking a future improvement (and possible breaking change) that -0.0 and 0.0 should be treated differently when converting to a string.

The code you referenced is fine and is matching the current expected behavior.

tannergooding · 2017-07-19T22:58:05Z

src/classlibnative/bcltype/number.cpp

+    UINT64 realMantissa = 0;
+    int realExponent = 0;
+    UINT32 mantissaHighBitIdx = 0;
+    if (((FPDOUBLE*)&value)->exp > 0)


I would expect you to be special casing denormal numbers, since the implicit bit is zero instead of one.

I think I have included the denormal numbers in the else condition. According to https://en.wikipedia.org/wiki/Double-precision_floating-point_format

In the case of subnormals (e=0) the double-precision number is described by:
(-1)^sign * 2^(1 - 1023) * 0.fraction = (-1)^sign * 2^(-1022) * 0.fraction

2^(-1022) * 0.fraction = 2^(-1022) * mantissa / 2 ^52 = mantissa * 2^(-1074) . So you can see in the else block, realExponent = -1074 , realMantissa = mantissa.

I will add comments here.

tannergooding · 2017-07-19T23:03:52Z

src/classlibnative/bcltype/bignum.h

+        }
+    } m_initializer;
+
+    UINT8 m_len;


nit: Either make this UINT32 or place it after m_blocks (larger fields should come first). The resulting struct is going to be 8-bytes in either case...

I'll place m_len after m_blocks. Thanks.

You really should make it UINT32 rather than placing it after m_blocks. In general, the only advantage of using small integral types is storage size. In this case you can't actually save any storage due to alignment requirements. Instead your risk making the code larger because the compiler may need to insert widening conversion in some cases.

@mikedn Sounds reasonable. But that means I need to extend the static data as well. Something like BIGSIZE and BIGPOWER10NUM should be extended to UINT32.

Anyway, I'll change m_len and related static data to UINT32 to see if there's any big impact. I don't want to rely on the compiler for the alignment but the size of static data should not be increased so much as well.

Something like BIGSIZE and BIGPOWER10NUM should be extended to UINT32.

Yes, but only for consistency. I would expect the compiler to treat those as constants so the impact on the generated code should be exactly 0. In fact, they should be constexpr, provided that the version(s) of VC++ used to build this code doesn't barf on it.

tannergooding · 2017-07-19T23:05:06Z

src/classlibnative/bcltype/bignum.h

+    static const UINT8 BIGSIZE = 35;
+    static const UINT8 UINT32POWER10NUM = 8;
+    static const UINT8 BIGPOWER10NUM = 6;
+    static UINT32 m_power10UInt32Table[UINT32POWER10NUM];


static UINT32 const m_power10UInt32Table[] =
{
// Declare data inline
};

I have to use constexpr to accomplish this. The code can be compiled on my current Ubuntu 16.04 box but I'm not sure if it is able to be compiled across platform. I'll do the change and to see if it can pass the CI later.

Using constexpr would likely work, but it shouldn't be required.

This should just be a UINT32 constant for the length and then a sequence of bytes representing the data. You likely need a second lookup table that contains the starting index for each entry, but it should just be a cast from UINT32* to BigNum* at that point.

tannergooding · 2017-07-19T23:08:25Z

src/classlibnative/bcltype/bignum.cpp

+
+BigNum::BigNum()
+    :m_len(0)
+{


a comment about explicitly not zeroing the memory due to perf reasons might be nice

tannergooding · 2017-07-19T23:11:45Z

src/classlibnative/bcltype/bignum.cpp

+            break;
+        }
+
+        --pCurrentLeftBlock;


nit: Lengths of both are the same, you can use a single index to do the access

tannergooding · 2017-07-19T23:13:05Z

src/classlibnative/bcltype/bignum.cpp

+    return *this;
+}
+
+int BigNum::Compare(const BigNum& lhs, UINT32 value)


Is having a compare really better than individual ==, !=, <, <=, >, and >= operators?

I can overload these operators but I think we should do this enhancement in another PR because a simple compare method should be enough to implement the algorithm.

I was mostly asking from a perf perspective. It seems like we are only actually reusing the Compare value in one place, every other place we are just doing a single comparison, which is likely cheaper than the full Compare check.

I'll remove BigNum::Compare(const BigNum& lhs, UINT32 value) since it is not used now.

BigNum::Compare(const BigNum& lhs, const BigNum& rhs) is still useful when comparing numerator and denominator so I'll keep it.

tannergooding · 2017-07-19T23:16:50Z

src/classlibnative/bcltype/bignum.cpp

+    int shiftBlocks = shift / 32;
+    int remaningToShiftBits = shift % 32;
+
+    for (int i = 0; i < shiftBlocks; ++i)


memset(0) would be much faster

(memset over the entire range that is)

Might be, it really depends on how large these bignums usually get. I suspect not very large.

As per the memcpy comment. memset should do the right thing, regardless of size. ~~The current max size is only 35 bytes, but I also believe this is considerably too small.~~ The current max size is 35 dwords (140 bytes).

I'll change the loop to memset.

tannergooding · 2017-07-19T23:23:52Z

src/classlibnative/bcltype/bignum.cpp

+        return true;
+    }
+
+    for (UINT8 i = 0; i < m_len; ++i)


it seems like we could trivially detect all the cases where the value would become zero and only rely on m_len == 0

Did you mean we should ensure that whenever m_len > 0, we at least have 1 block != 0 so that we could only check m_len == 0 inside IsZero()? I think IsZero() should not depend on this assumption which is like to depend on an implementation outside of it.

IsZero is part of the BigNum class, so it would only be dependent on a convention set for its own class.

This probably isn't too important so we can investigate later.

tannergooding · 2017-07-19T23:26:42Z

src/classlibnative/bcltype/bignum.cpp

+
+void BigNum::SetUInt64(UINT64 value)
+{
+    m_len = 0;


m_blocks[0] = value & 0xFFFFFFFF;
m_blocks[1] = value >> 32;
m_len = (m_blocks[1] == 0) ? 1 : 2;

Nice! Thanks!

tannergooding · 2017-07-19T23:28:01Z

src/classlibnative/bcltype/bignum.cpp

+
+void BigNum::ExtendBlock(UINT32 newBlock)
+{
+    m_blocks[m_len] = newBlock;


There are a few cases where an ExtendBlock(UINT32 blockValue, UINT32 numBlocks) would be useful. It can memset and increment m_len the appropriate count all at once.

I tried this change but it slightly slows down the performance. So I'd like to keep this method and use memset when there're bulk blocks need to be extended.

Did you create a second ExtendBlock method or just replace this one entirely? I was suggesting that we create a second method and call it if the number of blocks being set (we loop the current method in a couple places) is larger than some threshold.

tannergooding · 2017-08-12T15:51:01Z

src/classlibnative/bcltype/bignum.cpp

+    UINT32 lastIndex = lhs.m_len - 1;
+    INT32 currentIndex = lastIndex;
+
+    while (currentIndex >= 0)


for (UINT32 currentIndex = lhs.m_len - 1; currentIndex >= 0; --currentIndex) might be more readable?

tannergooding · 2017-08-12T16:03:00Z

src/classlibnative/bcltype/number.cpp

+    // ========================================================================================================================================
+    // This implementation is based on the paper: https://www.cs.indiana.edu/~dyb/pubs/FP-Printing-PLDI96.pdf
+    // Besides the paper, some of the code and ideas are modified from http://www.ryanjuckett.com/programming/printing-floating-point-numbers/
+    // You must read these two marterials to fully understand the code.


materials

tannergooding · 2017-08-12T16:04:44Z

src/classlibnative/bcltype/number.cpp

+    //
+    // 0.30102999566398119521373889472449 = log10V2
+    // 0.69 = 1 - log10V2 - epsilon (a small number account for drift of floating point multiplication)
+    int k = (int)(ceil(double((int)mantissaHighBitIdx + e) * 0.30102999566398119521373889472449 - 0.69));


nit, it would be more readable to declare log10v2 and 0.69` as constants.

tannergooding

A few minor nits, but overall 👍

Fix #10651

mazong1123 · 2017-08-13T06:31:09Z

@tannergooding I've updated code according to the feedback. Please take a look. Thanks!

mazong1123 · 2017-08-13T08:44:47Z

@dotnet-bot test Ubuntu x64 corefx_baseline
@dotnet-bot test Windows_NT x86 corefx_baseline
@dotnet-bot test Tizen armel Cross Release Build

mazong1123 · 2017-08-15T05:18:39Z

Do I need to put the license mentioned in #12894 (comment) anywhere?
At the beginning of number.cpp or DoubleToNumberWorker?
I am asking and will get back to you.

@tarekgh Do we have any update?

tarekgh · 2017-08-15T16:15:05Z

Do we have any update?

We are still following up here. looks there is some process we need to follow. we'll let you know what needs to be done when having more details. Thanks for your patience.

tarekgh · 2017-08-25T22:07:07Z

We are still following up here. we'll update as soon as we have news.

tarekgh · 2017-09-12T19:32:31Z

#13920

tarekgh · 2017-09-13T01:31:56Z

@mazong1123 thanks a lot for your effort and getting this ready. finally, I am going to merge it :-)

jkotas · 2017-09-13T03:58:36Z

This introduced build break on Windows ARM (this config does not build by default in CI):

src\classlibnative\bcltype\bignum.cpp(595): error C3861: 'BitScanReverse64': identifier not found [D:\j\workspace\arm_cross_che---9fe37e18\bin\obj\Windows_NT.arm.Checked\src\classlibnative\bcltype\bcltype.vcxproj]

jkotas · 2017-09-13T04:07:48Z

Fix: #13932

mazong1123 · 2017-09-13T05:03:23Z

@jkotas how did you find the build break? Is there any way to check arm build in CI?

jkotas · 2017-09-13T06:46:51Z

No worries. I happened to notice it on a different job. There are custom jobs that can be triggered manually based on the nature of the change.

@dotnet-bot help

prints the list. You will find the Windows ARM build somewhere in there. Obviously, it is not cost effective to run all of them for each job.

This reverts commit 602ebe9.

dnfclas added the cla-already-signed label Jul 19, 2017

mazong1123 changed the title ~~Re-implemented the ecvt function.~~ [WIP] Re-implemented the ecvt function. Jul 19, 2017

mazong1123 added 2 commits July 19, 2017 20:12

Resolve a cross platform header file issue.

448aca3

Fix #10651

Fixed a minor bug. Improved the performance.

b617b79

Fix #10651

mazong1123 changed the title ~~[WIP] Re-implemented the ecvt function.~~ Re-implemented the ecvt function. Jul 19, 2017

tarekgh added the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Jul 19, 2017

dotnet deleted a comment from dotnet-bot Jul 19, 2017

tarekgh reviewed Jul 19, 2017

View reviewed changes

tannergooding reviewed Jul 19, 2017

View reviewed changes

tannergooding reviewed Aug 12, 2017

View reviewed changes

tannergooding approved these changes Aug 12, 2017

View reviewed changes

Updated code according to the code review feedback.

61d940f

Fix #10651

karelz added the area-Meta-corelib label Aug 25, 2017

karelz assigned mazong1123 and tarekgh Aug 25, 2017

tarekgh added area-System.Runtime and removed area-Meta-corelib labels Aug 26, 2017

tarekgh assigned joperezr and unassigned tarekgh Aug 26, 2017

tarekgh merged commit 602ebe9 into dotnet:master Sep 13, 2017

mazong1123 deleted the fix-10651 branch September 13, 2017 01:41

jkotas added a commit to jkotas/coreclr that referenced this pull request Sep 13, 2017

Revert "Re-implemented the ecvt function. (dotnet#12894)"

4d7f650

This reverts commit 602ebe9.

mazong1123 mentioned this pull request Aug 18, 2018

Consider using Grisu3 + Dragon4 for double.ToString() conversion. chakra-core/ChakraCore#5609

Open

This was referenced Jan 31, 2020

Consider implementing Grisu3 algorithm for double.ToString() dotnet/runtime#9122

Closed

Add more performance tests for double.ToString() dotnet/runtime#22963

Closed

aloraman mentioned this pull request Sep 14, 2022

Get different result when using ToString("F2", new CultureInfo("de-DE") in .NET6 and .Framework4.7.2 dotnet/runtime#75588

Closed

Re-implemented the ecvt function. #12894

Re-implemented the ecvt function. #12894

Conversation

mazong1123 commented Jul 19, 2017 • edited Loading

mazong1123 commented Jul 19, 2017 • edited Loading

mazong1123 commented Jul 19, 2017 • edited Loading

Accuracy

Performance

Output of new implementation:

Output of old implementation:

Output of new implementation:

Output of old implementation:

tarekgh commented Jul 19, 2017

tarekgh commented Jul 19, 2017

safern commented Jul 19, 2017

tannergooding commented Jul 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tannergooding Jul 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarekgh Jul 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mazong1123 Jul 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mazong1123 Jul 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mazong1123 Jul 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mazong1123 Jul 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tannergooding Jul 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mazong1123 Jul 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tannergooding left a comment

Choose a reason for hiding this comment

mazong1123 commented Aug 13, 2017

mazong1123 commented Aug 13, 2017

mazong1123 commented Aug 15, 2017

tarekgh commented Aug 15, 2017

mazong1123 commented Jul 19, 2017 •

edited

Loading

mazong1123 commented Jul 19, 2017 •

edited

Loading

mazong1123 commented Jul 19, 2017 •

edited

Loading

tannergooding Jul 20, 2017 •

edited

Loading

tarekgh Jul 19, 2017 •

edited

Loading

mazong1123 Jul 20, 2017 •

edited

Loading

mazong1123 Jul 21, 2017 •

edited

Loading

mazong1123 Jul 23, 2017 •

edited

Loading

mazong1123 Jul 23, 2017 •

edited

Loading

tannergooding Jul 20, 2017 •

edited

Loading

mazong1123 Jul 22, 2017 •

edited

Loading

jkotas commented Sep 13, 2017 •

edited

Loading

jkotas commented Sep 13, 2017 •

edited

Loading