1

I have a library that is doing bulk insert.
Library is EfCore extension made in .NetStandard(1.4) so it can be used in ASP.NET Core projects targeting both .NetCore(1.0+) or full NetFramework(4.6.1+)
One of the functions is:

public static BulkInsert<T>(this DbContext context, IList<T> entities)
{
    SqlBulkOperation.InsertAsync<T>(context, entities);
}

internal static class SqlBulkOperation
{
    public static void Insert<T>(DbContext context, IList<T> entities)
    {
        ....
        sqlBulkCopy.WriteToServer(reader);
        ....
    }
}

Next I have added the same method with async support

public static async Task BulkInsertAsync<T>(this DbContext context, IList<T> entities)
{
    await SqlBulkOperation.InsertAsync<T>(context, entities, null, null);
}

internal static class SqlBulkOperation
{
    public static async Task InsertAsync<T>(DbContext context, IList<T> entities)
    {
        ....
        await sqlBulkCopy.WriteToServer(reader);
        ....
    }
}

And now I was advised to change the async method in a way to add ConfigureAwait(false) to internal method and also to optimize out simple async by removing explicit async keyword from exposed method like this:

public static Task BulkInsertAsync<T>(this DbContext context, IList<T> entities)
{
    return SqlBulkOperation.InsertAsync<T>(context, entities, null, null, true);
}

internal static class SqlBulkOperation
{
    public static async Task InsertAsync<T>(DbContext context, IList<T> entities)
    {
        await sqlBulkCopy.WriteToServerAsync(reader).ConfigureAwait(false);
    }
}

So the question being:
In this situation is it better to use ConfigureAwait(false)?
And secondly is removing async keyword from exposed methods for scenario in example advisable?

PS I have already read few blogs and several questions here regarding these issue but still haven't come to conclusive answer. I read that ASP.NET Core no longer has a "context" so taking that into the consideration what would be the best practice here?

borisdj
  • 2,201
  • 3
  • 24
  • 31
  • This has been asked 100 of times before, the answer is always the same: In class libraries always use `.ConfigureAwait(false)`, in WPF/Console/ASP.NET/ASP.NET Core projects, **never** use it – Tseng Aug 11 '17 at 09:43
  • And what about second issue, removing async ? – borisdj Aug 11 '17 at 09:48
  • 2
    @borisdj You use await only if you need continuation otherwise just return the task. Put another way, if you do not need the result of the task after calling it then then there is no need to wait for it. Just return the task. – Nkosi Aug 11 '17 at 09:50
  • async in `BulkInsertAsync` is not necessary, so it will work fine. Also no `.ConfigureAwait` necessary since you are passing the task from `InsertAsync`. And w/o the `async` keyword on it you get a (minimal) better performance, as no async state machine needs to be invoked and no new Task needs to be instantiated. – Tseng Aug 11 '17 at 09:55
  • @Nkosi Let's say I don't need it. Out of curiosity in what general case would I need the result of the task or can you specify some example. – borisdj Aug 11 '17 at 10:04
  • @TsengI it was initially suggested to put `ConfigureAwait` in internal method and not in `BulkInsertAsync`, like it is done in the example. – borisdj Aug 11 '17 at 10:05

1 Answers1

4

In this situation is it better to use ConfigureAwait(false)?

I generally recommend ConfigureAwait(false) for library code. The same advice from nearly half a decade ago still applies today. However, there are some mitigating factors here:

  • Your only target platforms do not provide a SynchronizationContext. Thus, as long as your code only runs on those platforms, ConfigureAwait(false) isn't necessary.
  • Both the ASP.NET Core and EF Core teams no longer use ConfigureAwait(false). However, note that the EF Core team does provide synchronous APIs, so the idea there is that people won't be blocking on their asynchronous code in the first place, so a lack of ConfigureAwait(false) won't cause a deadlock when used on platforms with a SynchronizationContext.

So, in your case, you can choose not to include ConfigureAwait(false). I'd say that's a valid approach, since your library is an extension to EFCore and since you are supplying synchronous as well as asynchronous APIs (like EFCore does). Do bear in mind that this opens up deadlocks in the scenario where a user installs onto a platform with a SynchronizationContext (e.g., classic ASP.NET) and blocks on your asynchronous code. But EFCore has the same limitation.

On a side note, try out the boolean argument hack to prevent code duplication in your synchronous/asynchronous method pairs.

And secondly is removing async keyword from exposed methods for scenario in example advisable?

No. I'm not sure why this would be recommended. It prevents end-users from detecting whether you're using the async keyword (there's an attribute the compiler places on async methods), but who cares?

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • My understanding that there async is not required, since there is only one call and there are no *using* nor *try* blocks, while the benefit would be slightly better performance. – borisdj Aug 15 '17 at 10:28
  • @borisdj: If you're just calling another method, then yes, [eliding async is fine](http://blog.stephencleary.com/2016/12/eliding-async-await.html). The sample code in your question looks like it's specifically adding an internal-only `async` method just so that the actual API wouldn't have `async` on it, which would only *add* overhead. – Stephen Cleary Aug 15 '17 at 14:37