Add IAsyncEnumerable support for Consumer

[rlgordey]

Please, add IAsyncEnumerable to list of Consume targets.

[timcassell]

I'm guessing you mean when returning IAsyncEnumerable(<T>) from the benchmark method, it should await foreach on the result and measure all the awaits and yields, and pass the T yielded values to Consumer.Consume()? Otherwise, this ask wouldn't make too much sense for just Consumer.Consume(IAsyncEnumerable).

[rlgordey]

I'm guessing you mean when returning IAsyncEnumerable(<T>) from the benchmark method, it should await foreach on the result and measure all the awaits and yields, and pass the T yielded values to Consumer.Consume()?

Yes @timcassell you are correct.

[timcassell]

In that case, this looks like a great candidate for a follow-up to my suggestion for fixing ValueTasks. #1595 (comment) Which is refactoring the engine to use an IMeasurer interface instead of System.Action WorkloadAction delegates.

[Edit] Note that this feature cannot be implemented without fixing ValueTasks, as the API relies on ValueTasks.

[timcassell]

For completeness, this should also include IAsyncEnumerator<T>.

[timcassell]

I can take this if #2111 gets merged.

[sramekpete]

Any progress on this one? I am actually trying to compare IEnumerable<T> and IAsyncEnumerable<T> performance, but having hard times to understand how to do it properly as Consume for IAsyncEnumerable doesn't exist.

I am happy to give a hand with implementation if someone would guide me.

If this is not something of an interest anymore, can someone just clarify how to properly consume IAsyncEnumerable in benchmark method, please?

I am now consuming IAsyncEnumerable by having one line at the end of the method.

await foreach (var _ in result) { }

Would that do the measurement somehow relevant to Consumer.Consume() method?

Thanks for a response and great work on improvements!

[timcassell]

Unfortunately, this isn't possible to implement correctly without the async refactor. await foreach (var _ in result) { } is the best you can do for now (it still includes the overhead of the async Task).

IEnumerable<T> and IAsyncEnumerable<T> aren't really comparable, as they serve different purposes. What are you trying to achieve?

[sramekpete]

@timcassell Thanks for a quick reply!

I do not care about the overhead that much. I may say I do care about comparing cases when it is better to use one over another(IEnumerable<T> and IAsyncEnumerable<T>).

I am basically exploring Span<T>, Memory<T> and ReadOnlySequenceSegment<T>. By reading what you have wrote I am not sure why benchmark should not take the overhead into a count. I mean, it is there. I would say that consume should count for that. But I may be wrong.

If you are interested in the case, there is a branch develop/1.0. There are benchmarks and implementation of sync and async Google Polyline Algorithm. I am just playing around with it to compare how things work, what performs better in which case, to just understand things better.

Thanks for the previous reply, @timcassell!

[timcassell]

By reading what you have wrote I am not sure why benchmark should not take the overhead into a count. I mean, it is there. I would say that consume should count for that. But I may be wrong.

You're right, it should. Which is exactly what the refactor aims to achieve. However, it is not a high priority for the other maintainers, so it hasn't been reviewed so far.

To get the best apples-to-apples comparison today, you can configure your benchmarks like this:

[Benchmark]
public async Task Blocking()
{
    await Task.Yield();
    
    // Do synchronous work here.
}

[Benchmark]
public async Task NonBlocking()
{
    await Task.Yield();
    
    // Do async work here.
}

[sramekpete]

Those things are from 2022...I do understand the focus is elsewhere. The priorities too. I do understand how hard is to maintain things.

Can I help with anything? I mean, it is really hard to contribute to something in case you don't have clear understanding where it should go and there are issues for years.

Happy to help, but I need at least something.

[timcassell]

Yes, the PR is old, and I haven't updated it in a while since it had no interest from others.

I think you commenting here showing your interest is already helpful. Maybe the other maintainers will take notice. If you want to help more, you could try reviewing #2111 yourself (and maybe #2349, which shows how I have thought about implementing this).

[sramekpete]

Will take a look over the weekend. If you don't mind, I will tag you in my responses.

Thank you!

[sramekpete]

@timcassell Hey Tim. I took a look at those two PR of yours. I am afraid I am not knowledgeable enough to evaluate if that is something benchmarkdotnet should go with or not. There needs to be quite a lot of understanding of the internals which I do not have at the moment :/

I will stick for the solution I have now. Sorry I couldn't help with those PR. Let's hope it will become more relevant in the future and maintainers put higher priority to it.