New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposal: Disposables in CoreFx: SerialDisposable, CompositeDisposable, etc #19822
Comments
👍 to this. Disposable implementations are useful on their own, outside of a RX setup. I have libraries where I have purposefully avoided a dependency on RX, but want the various disposables, and have ended up duplicating them internally. I think they should be "promoted". |
It may also be worth doing another round of optimization through the types. I noticed that often the disposable objects are a major source of memory consumption when using Rx extensively. Perhaps there's opportunities for internal improvements? E.g., coalescing CompositeDisposables to prevent multiple List instances floating around? Not sure what's actually possible here, but worth a look? |
I've created classes for this purpose is several projects before ever using RX and would readily use the proposed examples if they were made available. |
@onovotny it will be helpful if you can have the initial proposal so we can start the discussion on the deign. |
@tarekgh what needs to be there for the initial proposal? There are a few types already in existence in Rx.NET that have been well tested and complete from an API perspective. There may be room for internal optimizations as @RxDave suggests, but that's an implementation detail. The types are here currently: https://github.com/Reactive-Extensions/Rx.NET/tree/v3.1.1/Rx.NET/Source/System.Reactive.Core/Reactive/Disposables
Thanks |
thanks @onovotny
we need to list all types/APIs we need to expose, something like: public sealed class AnonymousDisposable : ICancelable
{
public AnonymousDisposable(Action dispose)
{
}
public bool IsDisposed
{
}
public void Dispose()
{
}
} and mention briefly the value having these types and how is going to be used. |
@tarekgh is there an easier way to do this given that this is already "approved" API in This is more about "promoting" the existing types/API from |
@terrajobst could you please advise how we can proceed with this request? @onovotny is there any other docs or pointers rather than the code for these types? |
The docs are on MSDN Does that help? |
In response to @tarekgh
The Although these types are first-class in Rx, where they're mostly used as composite subscriptions to observables, they're often used outside of Rx as well; e.g., for compositing a list of disposables together into a single disposable that can be disposed in a single call (especially useful when the order in which they are disposed matters, such as when releasing COM objects with dependencies, or when disposing has side effects), for exposing a disposable proxy before the object that actually needs to be disposed is created (such as the eager disposal of an object that will be created by awaiting an asynchronous operation), for disposing of one object and creating its replacement as an atomic operation (such as when an asynchronous request is canceled and replaced with another request, such that the result of the previous request, if any, will be discarded; e.g., when a user types in a search box, it cancels any previous search that may still be in progress and replaces it with the latest search operation instead), among other uses.
|
After considering this further it makes more sense to me to separate these Disposable types into their own library package instead of trying to pull them into a core framework. My original reason for agreeing with the proposal was to be able to use these disposable patterns without requiring a dependency Reactive Extensions because they are useful on their own. Are there any other strong benefits to making these part of CoreFx? |
This is a great issue. We need to approach this space more holistically. I don't think we want to add all functionality like this (area that is large enough to warrant its own library but doesn't need to be part of the platform layer) to CoreFX. On the other hand, we don't want to a bunch of random extensions with inconsistent names on top either. It would be nice if had some sort of agreed up and still reviewed set of platform extensions/community projects that can sit above CoreFX. I'll take a stab at a proposal and share it later. |
@terrajobst looks like this one has been stale for a while. Is corefxlab a reasonable place for these kind of APIs? If not, would you suggest creating its own repo under dotnet, or now that we are getting some extensions in runtime repo perhaps we can find a good fit for these? |
I've been beginning to use Reactive Extensions recently and was pleasantly surprised to see the various xxxxDisposable types as I've previously had to write my own (albeit much simpler) when simplifying objects that use a number of IDisposable objects (e.g. 3D graphics in engineering applications). I came here to see if there were any plans to build these APIs into the system .NET libraries. What would need to happen for this to become reality? Is there anything I can help out with? |
I am forever creating these two disposables in varius projects - and it's frustrating that I typically copy paste them around. I could create a nuget package of course but for just two classes.. and it's usually always at a point where I just don't have time, as I hit the need for these whils tin the middle of trying to glue stuff together - so dont have time to drop what I am doing and go create a dedicated nuget package projject and solve this properly - so the copy paste continues!
I am not sure about the other RX disposables mentioned and how general purpose they are. I could see the cancellation token one pootentially being useful but -- With an I also note that react's Finally there is now also |
Moving here from dotnet/reactive#309
Disposbles have a lot of uses in many libraries. Many libraries have created their own management and disposable aggregates to combine multiple disposable's. There might be an opportunity to move some of this logic in to CoreFX as a common library.
A few questions:
1. Does this belong in CoreFX at all?
2. Should it be in a new library,
System.Disposables
or does it fit into an existing library?/cc @ghuntley @mattpodwysocki @RxDave
The text was updated successfully, but these errors were encountered: