WL#17260: Separating synchronous and asynchronous code paths

It is a well-established best practice in modern .NET development to maintain separate code paths and distinct API signatures for synchronous (sync) and asynchronous (async) methods to avoid potential deadlocks, improve performance, and ensure clarity.

This WL aims at creating separate code paths instead of constantly evaluating for a sync/async flag throughout the code. Code will be reused where possible.

Functional Requirements:

• FR.1: No API changes should be made, that is, changes should be transparent to the user in regards to classes, methods, etc. exposed by our API.

• FR.2: Performance of sync operations should increase significantly.

• FR.3: Performance of async should not be negatively impacted.

Non-functional Requirements:

• NFR.1: Best practices dictate that separate code paths must exist for sync and async methods.

• NFR.2: Remove any conditionals related to evaluating if code should execute sync or async.

As part of a previous WL code was refactored to correctly implement async functionality using the Flag Argument Hack described in this Microsoft article.

While this is a good accepted approach for most applications which didn't involve rewriting the whole code base, it resulted in performance degradation (specially on the sync side) due to the excessive use of flags to evaluate if code should execute sync or async. This is caused because the nature of Connector NET involves evaluating the flag for operations that execute many many times during a database operation such as when reading data from a table. The constant evaluation consumes a lot of time that wouldn't need to be used if from the beggining it was stated that code should run sync or async.

Current best practices state that using a flag to dictate if code should run sync or async is not the recommended approach, instead providing both sync and async implementations of API methods is better.

In our code base, in most cases a single async method exists that receives an execSync parameter to determine if code should execute sync or async. The execAsync parameter is then cascaded to lower level methods to continue identifying how code should be executed. The use of execAsync is embedded in a large ammount of classes and is constantly being evaluated instead of simply defining how the code should execute from the beggining. This execesive use has proven to greatly affect performance of sync operations.

Code will be refactored to improve the following aspects:

Separate sync and async implementations

In the existing code base it is common to find code that looks like this:

public override bool NextResult() => NextResultAsync(false, CancellationToken.None).GetAwaiter().GetResult();

public override Task<bool> NextResultAsync(CancellationToken cancellationToken) => NextResultAsync(true, cancellationToken);

internal async Task<bool> NextResultAsync(bool execAsync, CancellationToken cancellationToken)
{
  ...
}

Note that the internal async NextResultAsync method is being called both for sync and async operations. Best practices dictate that instead of a single entry point, there should be different methods for sync and async. The use of GetAwaiter().GetResult() is also not encouraged as it may cause deadlocks in certains scenarios. Code should be refactored similar to this:

public override bool NextResult() => NextResult(CancellationToken.None);

public override Task<bool> NextResultAsync(CancellationToken cancellationToken) => NextResultAsync(cancellationToken);

internal bool NextResult(CancellationToken cancellationToken)
{
  ...
}

internal async Task<bool> NextResultAsync(CancellationToken cancellationToken)
{
  ...
}

Remove the use of execAsync parameter

The execAsync flag is used all over the code base. It is meant to keep track if code is being executed sync or async and is also cascaded down from the API entry point up until the lowest level of the MySQL protocol. Implementing separate sync and async methods will remove the need of this flag.

It was identified that when reaching lower layers, constant evaluation of this flag caused great performance degradation for sync operations.

Re-use CPU-bound code

Since sync and async methods will be implemented for all operations, this will raise the need to identify any code that can be reused by both methods as to prevent code duplication as much as possible.

• Implement separate sync and async methods.

• Remove the use of the execAsync parameter.

• Reuse CPU-bound code (if any) to reduce code duplication as much as possible.