Open-NET-Libraries / Open.ChannelExtensions

A set of extensions for optimizing/simplifying System.Threading.Channels usage.
https://open-net-libraries.github.io/Open.ChannelExtensions/api/Open.ChannelExtensions.Extensions.html#methods
MIT License
416 stars 25 forks source link

Support disposable messages #11

Closed JKamsker closed 3 years ago

JKamsker commented 4 years ago

Hey, My producer creates disposable messages that are consumed by readallasync.

Is it possible for you to dispose it automatically without needing me to do it manually?

Thanks!

electricessence commented 4 years ago

I'm a little confused about the question. Dispose what? Channels are not disposable objects. Once it's empty and complete, that's it. If you await channelReader.ReadAllAsync(e=>/*do something*/), then once it's done, it's done.

Could you clarify more?

electricessence commented 4 years ago

Are you suggesting making a disposable extension? It would probably be a .DisposeAsync() method and you'd have to use await using.

And so if the .DisposeAsync() is called, it would call .Complete() and then wait for all the results to be finished. There is already a .CompleteAsync() extension. https://github.com/Open-NET-Libraries/Open.ChannelExtensions/blob/master/Open.ChannelExtensions/Extensions._.cs#L127-L142

So you'd expect a .CreateDisposable() extension that would return an IAsyncDisposable struct?

JKamsker commented 4 years ago

No. In the channel, im reading from an tcpclient and push the data in a wrapped object to the queue. This data is essentially borrowed from ArrayPool and needs to be returned. Without that addition, my consumer needs to call message.dispose on itself, but thats something, ChannelExtensions could do for me i guess

JKamsker commented 4 years ago

And: The channel theoretically never completes as long as a client is connected to the socket.

electricessence commented 4 years ago

So aren't you doing this?

channelReader.ReadAllAsync(async e => {
  await e.Data;
  await e.DisposeAsync(); // or something like pool.Return(e.Data);
}):

I want to help. :)

JKamsker commented 4 years ago

This is exactly what i currently do. But cant it be integrated into the library?

If the message is disposable, it should be disposed?

electricessence commented 4 years ago

So it's not the intention of the library to get too far into the woods of individual implementations. The primary goal is to reduce the boiler plate for common scenarios. To simply 'automatically' dispose of objects is inadvisable.

That said, you can very easily write your own extension that does exactly that.

electricessence commented 4 years ago

If these objects require manual disposal, then it still seems like the above code example is the right way to go. It's explicit, and readable. If it's a common operation that you do, writing your own extension inside your code be a great improvement.

channelReader.ProcessMessages();

Another way to do this, is with .Pipe() or .PipeAsync(). Look into them. Basically this can happen: Producer writes to channel and .Pipe() reads the data and passes on the message to another channel that does the disposal. This can decouple the processing in a way that can (or won't [depending]) improve overall performance. You'd want to be sure to have a capacity value set on the pipe operation so you don't pile up disposal operations.

Now on to another point. If the objects don't require disposal, manual disposal can (but might not) degrade your overall performance as the GC might actually do a better job at it.

If you are using an ArrayPool, then it can be tricky to be sure to get the array back to that pool. But again, that's a custom implementation.

I'm sorry if this isn't helpful. I still might not understand the problem fully.

electricessence commented 4 years ago

Probably going offline for tonight. But I'll try to get back to you asap.