Bug fix & Performance Optimization

[BruceChenQAQ]

fix #2118 , fix #2082, fix #2129, fix #2119, fix #2094, fix #2132, fix #2111, fix #2150

Some fixes have been submitted to ConsoleInteractive.

I will record some of the before and after optimization comparisons below.

[BruceChenQAQ]

Check Vs WithoutCheck

Code

using System.Diagnostics;

const string Format = "{0,7:0.000} ";
const int TotalPasses = 25000;
const int Size = 50;
Stopwatch timer = new();

var functionList = new List<Action> { WithCheck, WithoutCheck };

Console.WriteLine("{0,5}{1,20}{2,20}{3,20}{4,20}", "Run", "Ticks", "ms", "Ticks/Instance", "ms/Instance");

foreach (var item in functionList)
{
    var warmup = Test(item);
    var run = Test(item);

    Console.WriteLine($"{item.Method.Name}:");
    PrintResult("warmup", warmup);
    PrintResult("run", run);
    Console.WriteLine();
}

static void PrintResult(string name, long ticks)
{
    Console.WriteLine("{0,10}{1,20}{2,20}{3,20}{4,20}", name, ticks, string.Format(Format, (decimal)ticks / TimeSpan.TicksPerMillisecond), (decimal)ticks / TotalPasses, (decimal)ticks / TotalPasses / TimeSpan.TicksPerMillisecond);
}

long Test(Action func)
{
    timer.Restart();
    func();
    timer.Stop();
    return timer.ElapsedTicks;
}

static void WithCheck()
{
    ushort[,,] blocks = new ushort[16, 16, 16];

    for (var passes = 0; passes < TotalPasses; passes++)
    {
        for (int chunkY = 0; chunkY < 24; chunkY++)
        {
            for (int blockY = 0; blockY < 16; blockY++)
            {
                for (int blockZ = 0; blockZ < 16; blockZ++)
                {
                    for (int blockX = 0; blockX < 16; blockX++)
                    {
                        if (blockX < 0 || blockX >= 16)
                            throw new ArgumentOutOfRangeException("blockX", "Must be between 0 and " + (16 - 1) + " (inclusive)");
                        if (blockY < 0 || blockY >= 16)
                            throw new ArgumentOutOfRangeException("blockY", "Must be between 0 and " + (16 - 1) + " (inclusive)");
                        if (blockZ < 0 || blockZ >= 16)
                            throw new ArgumentOutOfRangeException("blockZ", "Must be between 0 and " + (16 - 1) + " (inclusive)");

                        blocks[blockX, blockY, blockZ] = (ushort)(blockY * blockZ * blockX);
                    }
                }
            }
        }
    }
}

static void WithoutCheck()
{
    ushort[,,] blocks = new ushort[16, 16, 16];

    for (var passes = 0; passes < TotalPasses; passes++)
    {
        for (int chunkY = 0; chunkY < 24; chunkY++)
        {
            for (int blockY = 0; blockY < 16; blockY++)
            {
                for (int blockZ = 0; blockZ < 16; blockZ++)
                {
                    for (int blockX = 0; blockX < 16; blockX++)
                    {
                        blocks[blockX, blockY, blockZ] = (ushort)(blockY * blockZ * blockX);
                    }
                }
            }
        }
    }
}

[BruceChenQAQ]

• new ReadNextVarInt

public int ReadNextVarInt(Queue<byte> cache)
{
    int i = 0;
    int j = 0;
    byte b;
    do
    {
        b = cache.Dequeue();
        i |= (b & 127) << j++ * 7;
        if (j > 5) throw new OverflowException("VarInt too big");
    } while ((b & 128) == 128);
    return i;
}

---

• old ReadNextVarInt

string rawData = BitConverter.ToString(cache.ToArray());
int i = 0;
int j = 0;
int k = 0;
while (true)
{
    k = ReadNextByte(cache);
    i |= (k & 0x7F) << j++ * 7;
    if (j > 5) throw new OverflowException("VarInt too big " + rawData);
    if ((k & 0x80) != 128) break;
}
return i;

[BruceChenQAQ]

Before: 65 seconds until chunks is fully loaded. Memory usage: 260MiB

---

After: 17 seconds until chunks is fully loaded. Memory usage: 138MiB

[BruceChenQAQ]

I removed the locks from Chunk.cs and ChunkColumn.cs and now they should still be thread safe. There may be some oversight, so if you find something that might be wrong, please point it out in the comments.
:)

[BruceChenQAQ]

9.7 seconds until chunks is fully loaded. Memory usage: 73MiB

---

Before joining a server:

After joining a server:

[BruceChenQAQ]

5.1 seconds until chunks fully loaded.

[BruceChenQAQ]

2.8 seconds & 58MiB

Now using the AES-NI instruction set for greater throughput and lower CPU usage than the previous version using BouncyCastle.

On devices that do not support this instruction set (SSE2 and AES-NI), the AES implementation from the standard library will be used and parallel processing will be enabled.

public override int Read(byte[] buffer, int outOffset, int required)
{
    if (inStreamEnded)
        return 0;

    Span<byte> blockOutput = FastAes != null ? stackalloc byte[blockSize] : null;

    byte[] inputBuf = new byte[blockSize + required];
    Array.Copy(ReadStreamIV, inputBuf, blockSize);

    for (int readed = 0, curRead; readed < required; readed += curRead)
    {
        curRead = BaseStream.Read(inputBuf, blockSize + readed, required - readed);
        if (curRead == 0)
        {
            inStreamEnded = true;
            return readed;
        }

        int processEnd = readed + curRead;
        if (FastAes != null)
        {
            for (int idx = readed; idx < processEnd; idx++)
            {
                ReadOnlySpan<byte> blockInput = new(inputBuf, idx, blockSize);
                FastAes.EncryptEcb(blockInput, blockOutput);
                buffer[outOffset + idx] = (byte)(blockOutput[0] ^ inputBuf[idx + blockSize]);
            }
        }
        else
        {
            OrderablePartitioner<Tuple<int, int>> rangePartitioner = curRead <= 256 ?
                Partitioner.Create(readed, processEnd, 32) : Partitioner.Create(readed, processEnd);
            Parallel.ForEach(rangePartitioner, (range, loopState) =>
            {
                Span<byte> blockOutput = stackalloc byte[blockSize];
                for (int idx = range.Item1; idx < range.Item2; idx++)
                {
                    ReadOnlySpan<byte> blockInput = new(inputBuf, idx, blockSize);
                    Aes!.EncryptEcb(blockInput, blockOutput, PaddingMode.None);
                    buffer[outOffset + idx] = (byte)(blockOutput[0] ^ inputBuf[idx + blockSize]);
                }
            });
        }
    }

    Array.Copy(inputBuf, required, ReadStreamIV, 0, blockSize);

    return required;
}

---

Before joining a server:

After joining a server:

[milutinke]

Can you rename the protocolversion to protocolVersion so it matches the master, so there is no conflicts?

[BruceChenQAQ]

Okay, renamed.

[ORelio]

@BruceChenQAQ Thanks for your work on this. You seems to coordinate well with @milutinke so I'm adding you as well as maintainer so you can work more efficiently on the code and merge this PR when appropriate 👍

[milutinke]

Pull from the master the web UI can't work with this much conflics, I'll do testing again on multiple servers, especially with terrain handling.
Btw, awesome job, this is going to improve MCC so much.

[BruceChenQAQ]

Conflict resolution has been completed, I'll do some testing later.
There are still some issue found with the handling of MessageHeader in 1.19.2
Edit: Fixed.

[BruceChenQAQ]

I tested terrain handling in an official server and a paper server respectively, and they worked fine.

@milutinke Let's merge it if all functions are working properly.

[milutinke]

Is this Console Interactive bug?

Edit:
The only remaining things are the bug above and not being able to path-find when in 2x1 tunnel.
Everything else works.

[BruceChenQAQ]

Is this Console Interactive bug?

Partially yes, this should be fixed in b26949e and breadbyte/ConsoleInteractive#14 and breadbyte/ConsoleInteractive#12.

---

Edit: The only remaining things are the bug above and not being able to path-find when in 2x1 tunnel. Everything else works.

I tested that it is pathfinding in the tunnel, could it be that the coordinates are wrong?
The coordinates of the Block row should be used, or the coordinates of the XYZ row should be rounded down (-42.1 -> -43, +43.9 -> +43)
Maybe it should be reminded in the documentation.

[milutinke]

Is this Console Interactive bug?

Partially yes, this should be fixed in b26949e and breadbyte/ConsoleInteractive#14 and breadbyte/ConsoleInteractive#12.

Edit: The only remaining things are the bug above and not being able to path-find when in 2x1 tunnel. Everything else works.

I tested that it is pathfinding in the tunnel, could it be that the coordinates are wrong? The coordinates of the Block row should be used, or the coordinates of the XYZ row should be rounded down (-42.1 -> -43, +43.9 -> +43) Maybe it should be reminded in the documentation.

[BruceChenQAQ]

@milutinke Try using /move 35 71 -35

[milutinke]

/move 35 71 -35

That works.
It's not really intuitive.

[BruceChenQAQ]

/move 35 71 -35

That works. It's not really intuitive.

Yes, I also encountered this problem when I started testing.
Because people are used to rounding to zero, rather than strictly rounding down.

[BruceChenQAQ]

Perhaps it would be better to upgrade the move command to accept floating point and relative coordinate representations.

[milutinke]

Perhaps it would be better to upgrade the move command to accept floating point and relative coordinate representations.

The message appeared after the merge xD
That could be done in a separate PR.

[ORelio]

Good job 🎉

[breadbyte]

Updated the ConsoleInteractive submodule as well, as it wasn't included in this PR (BruceChenQAQ added in fixes too)

[The recent build is failing because there seems to be a network issue with the github action runner, will try again later.]

[BruceChenQAQ]

The message appeared after the merge xD
That could be done in a separate PR.

Implemented in #2154, it will now move to the exact coordinates entered.
:)