Speed up multisig ut

[shargon]

No description provided.

[codecov-io]

Codecov Report

Merging #908 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #908   +/-   ##
=======================================
  Coverage   45.16%   45.16%           
=======================================
  Files         178      178           
  Lines       12608    12608           
=======================================
  Hits         5695     5695           
  Misses       6913     6913

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1d949c8...6b6c6ab. Read the comment docs.

[erikzhang]

Can this really speed up multisig ut?

[vncoelho]

Sounds good, Shargon.
Good idea.

[erikzhang]

@vncoelho Did you test it?

[vncoelho]

Yes, worked normally.
But I did not benchmark...

I am having problems with #905
There was a failed test that passed yesterday.
I am trying to double check it.

[erikzhang]

Worked normally only, or faster?

[vncoelho]

I did not benchmark, @erikzhang.
It passed the tests. But makes sense to be faster.

For being sure about that we would need to run a loop and count the time. Do you think that we should do that for being sure before merging, @shargon?

[erikzhang]

We should do that for being sure before approving. @vncoelho

[vncoelho]

Let's check it out.
Results are coming.

[vncoelho]

For replications of the following loop:

                double avgTime = 0;
                int nIter = 10;
                for (int i = 0; i < nIter; i++)
                {
                    Stopwatch sw0 = new Stopwatch();
                    sw0.Start();

                    var aa = walletA.CreateAccount();
                    var bb = walletB.CreateAccount();

                    sw0.Stop();
                    TimeSpan elapsed = sw0.Elapsed;
                    avgTime += elapsed.Seconds;
                }
                Console.WriteLine($"Elapsed={avgTime / nIter}");
                avgTime = 0;
                for (int i = 0; i < nIter; i++)
                {
                    Stopwatch sw0 = new Stopwatch();
                    sw0.Start();

                    var ta = new Task<WalletAccount>(() => walletA.CreateAccount());
                    var tb = new Task<WalletAccount>(() => walletA.CreateAccount());
                    ta.Start();
                    tb.Start();
                    Task.WaitAll(ta, tb);

                    var aa = ta.Result;
                    var bb = tb.Result;
                    sw0.Stop();
                    TimeSpan elapsed = sw0.Elapsed;
                    avgTime += elapsed.Seconds;
                }
                Console.WriteLine($"ElapsedNew={avgTime/nIter}");

For nIter = 10

 Elapsed=6.2
 ElapsedNew=3.1

For nIter = 100

 Elapsed=61
 ElapsedNew=32

I also learned to run with a filter, now I am dominating these dotnet tests! ahueahueaea
dotnet test --verbosity n --filter "FullyQualifiedName~Neo.UnitTests.UT_Transaction" ./neo.UnitTests/neo.UnitTests.csproj
Ui Flai...average 0,6s gain with this PR... :D

[shargon]

from 8 seconds to 6 in my laptop

[shargon]

You should take into account that ut culture run this in several times, so the optimization is better

[i359]

@vncoelho,Pls double check the below code,and re-test, thanks!

var tb = new Task(() => walletA.CreateAccount());
maybe is var tb = new Task(() => walletB.CreateAccount());

[vncoelho]

That is true, @i359, you are right. Thanks for taking a careful look!

But results will be similar. I think that I will not re-run this, because the result matched the theoretical expectation...aheuahuea

[i359]

@vncoelho, The following test results are amazing, and it is recommended to use the modified code to
re-test,that's scientific rigour, right? thanks！

For nIter = 10
Elapsed=6.2
ElapsedNew=3.1

[vncoelho]

Sometimes science is made of empirical tests...aheuahuea
Thanks for contributing, @i359.

[i359]

@vncoelho, the below code only create walletA, no create walletB, so save about half time,
it is recommended to use the modified code to re-test,thanks!

var ta = new Task(() => walletA.CreateAccount());
var tb = new Task(() => walletA.CreateAccount());

[vncoelho]

You are blowing my mind now, it is 1am here.
I am going to re-run for nIter=10 and go to sleep...aheuahea

[vncoelho]

I run the tests 2 times until remember that we need to fail for printing messages...aheuahea

Here it is, @i359. The same result... O.o 💃

 Elapsed=6
 ElapsedNew=3

Anyway, it is always good to wonder.
Thanks for the insight.
@i359, it would be great if you could help the investigation for finding other places in the UT that a similar strategy could be applied.

[shargon]

i have a mistake...

[shargon]

Please if some mistake are found, remove the approve 🙏

[vncoelho]

@i359, I thought the mistake was just on my loop for testing...