Bug #114567 mysql connector got almost twice slower than previous relases (from 8.0.33)
Submitted: 7 Apr 13:39 Modified: 13 May 12:58
Reporter: József Mikla Email Updates:
Status: Verified Impact on me:
None 
Category:Connector / NET Severity:S5 (Performance)
Version:8.0.33-8.3 OS:Windows
Assigned to: CPU Architecture:Any

[7 Apr 13:39] József Mikla
Description:
Hey,

I was using version 6.6.7.0 for a long time with connector since it worked stable with a lot of database versions. In past weeks i was taking a look at different connectors.. not just oracle made for .net. And made a lot of testing with same databases at a time with the different versions of connectors and mesured the time it takes for same operations.

Your own connectors started to get slower and slower as the version was going up, especially after 8.x which were about 5-10% tops at first, but there is a very significant change from 8.0.32 to 8.0.33. The time it takes to complete the reading from the database almost doubles.

I did test many versions as i was saying and u have the same issues in every newer versions till 8.3. So its very safe to say that it is where u changed something for the worse for my scenarios. And reading the documentation my guess is that the bug fixes for the async issues are the ones what made this change happen.
My assumption is based partially on the fact that my code uses syncronous reading functions only. (lets note that the same happens if i run the syncronous methods on a thread other than UI, so it has nothing to do with beign threaded or not).

How to repeat:
I did wrote own codes.. i mean i am not using EF and stuff like those to handle reads writes.

So what i am doing is, is that i send multiple SELECT * FROM tables for every table i wanna read from all at once in 1 command, and i handle the results as they come in. And thats it. Nothing more special than this.
Just use a few tables which have at least 1-2 queries where u get 250k rows back and u will see the issue clearly.

Use a code similair to this one to produce the issue.
Made some more testing right now, and it seems that the reader.read part is what is taking like twice as long.

using (DbConnection connection = this.GetConnection())
            {
                connection.ConnectionString = this.connectionString;
                connection.Open();
                DbCommand command = connection.CreateCommand();
                command.CommandText = sql;
                command.Parameters.AddRange(parameters);
                var reader = command.ExecuteReader();

                int i = 0;
                do
                {
                    while (reader.Read())
                    {
                    }

                    i++;
                }
                while (reader.NextResult());

                connection.Close();
            }

Suggested fix:
I am not sure..

but the issue arised after the async fixes!
-> reader.Read takes twice as much time almost.
[15 Apr 12:16] József Mikla
Hello,

i took the liberty to download both source codes for 8.0.22 and 8.0.33.
After some comparing.. i saw that u simply call the async methods for sync methods, so my suspisions were accurate about the probable issue.

Anyways. To be sure, and also i was really curious, i did change the code of 8.0.33 with the original 8.0.32 codes help. Meaning that i did only change or add stuff mostly what u guys made.
The change was ONLY for the datareaders Read().. i commented out the () => async part of it.. and copied back the original Read all while preserving EVERYthing async in the 8.0.33.
I went down the chain to every part recopied the necessary bits of non async calls down to every MySqlType like MySqlDecima.cs -> IMySqlValue IMySqlValue.ReadValue(MySqlPacket packet, long length, bool nullVal).
I readded the above codes while still having all the async versions untouched.
I made sure so to speak that the Read will be pure sync code, and i left the async version as it was. The only bits i did change in the Read was that on error handling i did leave a slightly modified async versions of it in the code since on errors it gets only called one time i guess.

Anyways.. after compiling to DLL. I did add the original and my modified version of 8.0.33 in my software for testing, mesured the times, and the modified version is nearly as fast as the 8.0.32 version was. With only changing the Read method to sync version.

I belive that the issue with the performance/speed with 8.0.33 and above is that u changed your code with an extra boolean -> bool execAsync, and this is tested like every time on every column of every row in an extreme amount of times so it decides to call a sync or async version of parts of your code. It might have other reasons while its so much slower, but i have a feeling that this is a huge part of it.
I mean previously it never went left or right on every value it tried to access, but now it does. Even your async calls will be much faster if u simply write a sync and async version for everything instead of this "lazy" coding for something which has to be obviously as fast as possible, meaning as optimized as it can be.
Or the very least for the codes what are called extensively like Read and calls from these methods/functions.

Anyways, i hope i was not offending u with "lazy" :) It wasnt my intention.

I hope u try to rewrite the next version of your connector with the suggestions i had in mind. Have a nice day!
[15 Apr 12:59] József Mikla
One more thing.

Every async method gets translated to a class with extra methods and variables and switch statement.. when the code is actually running.

Which means that even a sync call will go trough this since u call your async methods from sync methods. And these new generated classes are used in reality every time the code goes from a to b.. like your async decimal calls another async method down the line and so on.. so who knows how many times it calls these extra things instead of being a straightforward sync method. Which means that due to being called so many times it also generates extra performance issues, while in reality it does not give any benefits at all. It just makes it so that a programmer who just calls readasync does not have to write too many things to be async.. thats all what happens.. all while he could write his own async solutions which call the sync method :) Which would run a lot faster -> half time.

This guy checked whats generated from async by the compiler..
https://youtu.be/GQYd6MWKiLI?t=292
[27 Apr 6:40] József Mikla
I did create later a project just for testing, to make life easier :)
I add my test results for different cases, so u can have some visual estimates of the performance issues.

Times are in seconds, and the amount of data is not relevant that much, but u can see it clearly how things compare to each other in speed.
8022 orig sync - 16,22
8033 mod sync - 16,66
8033 orig sync - 37,64
8033 mod async - 53,43
8033 orig async - 53,62

- orig = original dll
- mod = the Sync Read() got replaced fully by 8022 sync code down the line every method call was added back into the code. And the async ReadAsync() i changed, that i added copy pasted versions of all the methods it touches, and made sure to eliminate execAsync bool so it was just going straightforward with async calls.

Notes:
- the modified 8033 runs maybe slightly slower, due to still calling async methods everywhere else, like nextresult, and executereader.
- the original 8033 sync is slower than the 8032 and modified 8033 due to that the read is called millions of times AND every method is async type which is translated into a very slow code compared to a sync method.
- the original 8033 sync is faster than any of the async test results due to the fact that at the end of every chain of method calls it does end in an IF where due to execAsync being false it goes onwards with a pure sync code, and the async methods go forward with who knows how much more async method calls.
For example:
int read = execAsync
    ? await stream.ReadAsync(buffer, offset + numRead, numToRead, CancellationToken.None).ConfigureAwait(false)
    : stream.Read(buffer, offset + numRead, numToRead);
- the difference btw the modified async and original async may be either nothing, but since the modifed version is even more optimal it might had been the elemination of the execAsync bool code.
[13 May 12:58] MySQL Verification Team
Hello József Mikla,

Thank you for the bug report.
Verified as described.

Regards,
Ashwini Patil