speed up Uint160.CompareTo and Uint256.CompareTo function

[lightszero]

speed up bytes.CompareTo X3 times maybe more.

[igormcoelho]

@lightszero I didn't manage to merge with your local master branch... I think it's a permission problem, or perhaps your PR should be changed to another branch (other than lightszero:master). See: #553

Using that structure, four tests are done now:

1. UInt256.CompareTo (including class overheads)
2. UInt256.CompareTo (implemented as an independent function)
3. lights code
4. jeff proposal

RANKED RESULTS (less time for 1 million comparisons on Intel i5-4210U CPU @ 1.70GHz)

 2. Elapsed=00:00:00.0440732 Sum=-1354
 3. Elapsed=00:00:00.0489810 Sum=-1354
 1. Elapsed=00:00:00.0533977 Sum=-1354
 4. Elapsed=00:00:00.0650152 Sum=-1354

 2. Elapsed=00:00:00.0433505 Sum=892
 3. Elapsed=00:00:00.0515609 Sum=892
 1. Elapsed=00:00:00.0575372 Sum=892
 4. Elapsed=00:00:00.0638825 Sum=892

 2. Elapsed=00:00:00.0427061 Sum=-364
 3. Elapsed=00:00:00.0510161 Sum=-364
 1. Elapsed=00:00:00.0544293 Sum=-364
 4. Elapsed=00:00:00.0625798 Sum=-364

 3. Elapsed=00:00:00.0497432 Sum=566
 2. Elapsed=00:00:00.0523211 Sum=566
 4. Elapsed=00:00:00.0643599 Sum=566
 1. Elapsed=00:00:00.0662604 Sum=566

All seem correct. Timings are similar too... current winner is original approach (2 or 1), although in one case lights approach was actually faster.

[lightszero]

test case data has some problem. make some equal data will be good.

[shargon]

Could you add some UT for this change?

[lightszero]

@shargon
test case is here. thanks for igormcoelho.
#553

https://github.com/igormcoelho/neo/tree/benchmarks_uint

[lightszero]

now result is like this，with 50% equal data

Elapsed=00:00:01.1170521 Sum=-1244
Elapsed=00:00:01.0273095 Sum=-1244
Elapsed=00:00:00.4392158 Sum=-1244
Elapsed=00:00:00.3674855 Sum=-1244

[igormcoelho]

Congratulations @lightszero, you were right about 3 times faster:

UInt256
A. Elapsed=00:00:00.2419631 Sum=802  (class overhead, discard)
B. Elapsed=00:00:00.1837202 Sum=802 (real current time)
C. Elapsed=00:00:00.0850467 Sum=802
D. Elapsed=00:00:00.0655413 Sum=802  (around 3x faster than B)

UInt160
A. Elapsed=00:00:00.1320433 Sum=-974 (discard)
B. Elapsed=00:00:00.1214178 Sum=-974
C. Elapsed=00:00:00.0700265 Sum=-974
D. Elapsed=00:00:00.0590581 Sum=-974 (around 2 times faster than B)

So I guess we will keep ulong option? :)

[igormcoelho]

This is a man thinking ahead of its time xD @lightszero since 2017 thinking on this hahahah

[igormcoelho]

Congratulations Lights!

[jsolman]

@lightszero nice job, should we unroll the loop for the UInt160 as @igormcoelho suggested so it can take advantage of 64bit comparisons?

[igormcoelho]

Ok, tests are merged now: #553
So, please update the code and tests directly here now @lightszero.

[shargon]

I made this little optimization, lightszero#2 take a look please @lightszero

[lightszero]

unroll

totally agree.

[igormcoelho]

@lightszero can you propose a final version?
Please let's also reduce the experiments to 1k instead of 1 million... it's already taking too much processing on my computer.

we had talk about that.

[jsolman]

I thought we were going to unroll to use 1 uint comparison and 2 ulong comparisons instead of 5 uint comparisons?

[lightszero]

yes,can do more test to find a fast way.

[vncoelho]

Can we resolve this, @jsolman? Maybe we could add this change in another PR after another tests.

[jsolman]

It can be switched later

[jsolman]

I dismissed the review; this comment can be left open though while the PR can still be approved.

[jsolman]

Maybe in an effort to maintain compatibility in the future should the dotnetcore2.x get support for big endian architecture, we should use if (BitConverter.IsLittleEndian)

Change requested was just a suggested improvement.

[shargon]

If we make private byte[] data_bytes; protected, we can save two ToArray() calls.
What do you thik?

[igormcoelho]

I agree with you @shargon, all UInt should have internal byte[]

[shargon]

If everyone thinks it's better with the protected data, I can make the changes

[igormcoelho]

@shargon let's merge this now and continue improving this in other PR.