kosich / rxjs-autorun

Re-evaluate an expression whenever Observable in it emits
MIT License
34 stars 2 forks source link

Release (unsubscribe) unused subscriptions #7

Closed Jopie64 closed 3 years ago

Jopie64 commented 4 years ago

I figured it might be better to discuss this in a new issue :)

Here's a use-case that advocates for unsubscribing unused observables.

Suppose there's an observable userPresence$ which, when observed, starts a resourceful backend subscription on this users presence. There's also a button modelled by another observable that enables showing the user presence, e.g. presenceEnabled$. Out of those two you want to make a new observable that is used to display the current presence state. Following could be a naive implementation:

const displayPresence$ = combineLatest([
    userPresence$, presenceEnabled$
  ]).pipe(
    map(([presence, enabled]) => enabled ? presence : Presence.Disabled));

But this will not release the subscription resource when presence is disabled. So then you might want to write it like this:

const displayPresence$ = presenceEnabled$.pipe(
  switchMap(enabled => enabled ? userPresence$ : of(Presence.Disabled)));

This has the correct behavior. But I think it would be nicer when you could use this lib.

const displayPresence$ = run(() => $(presenceEnabled$) ? $(userPresence$) : Presence.Disabled);

Now it becomes a one liner that is easy to read and does exactly what I want which I think is very cool! But in this scenario, currently it leaves the userPresence$ subscription open once presenceEnabled$ became true once (even after it becomes false again later). That means it never releases the costly presence subscription resource anymore.

So I would love to propose a feature that does not only 'late subscription', but also 'early unsubscription' :)

Do you think it is OK if I write a PR for this?

Originally posted by @Jopie64 in https://github.com/kosich/rxjs-autorun/issues/3#issuecomment-703878927

Jopie64 commented 4 years ago

Also consider this scenario:

// s emits true, then false and then does nothing forever.
// I know currently the runner will not see 'true' because 'false' is also emitted synchronously, but
// let's assume the runner will see it anyway.
const s = concat(of(true, false), NEVER);
// Not sure it will, but lets suppose t emits it's first value synchronously
const t = interval(100);
const r1 = run(() => _(s) ? $(t) : -1);
const r2 = run(() => _(s) ? 1 : -1);
const r3 = run(() => _(s) ? 1 : $(t));

Suppose you subscribe r2. What would you expect? Since it is only observing s untracked, it will emit only once a 1 and then complete, because there's no way that r2 could ever emit something else.

Suppose you subscribe r3. What would you expect? It will never reach the point where it starts to observe t. So this would also emit 1 and then complete because the runner couldn't know about t.

Now suppose you subscribe r1. What would you expect? It subscribes t which emits 0 immediately. But when it wants to emit it's next value, s has changed to false and will never become true again. But now t is still observed and tracked. And even when it's value is never used, it keeps waiting for that observable to complete. So r1 currently never completes.

I argue that r1 should complete. Because since s is not tracked and t is not used any longer, there should be no way that the run expression is run again.

kosich commented 4 years ago

First of all I'd like to emphasize that this library is an ongoing experiment, rather then a know-how solution. It's whole usefulness is still under question and applications and their best practices are not clear. I think, it's good to keep this in mind discussing this and other issues.


To the matter: Initially the library (experiment) was intended to be used with either side-effect-less subscriptions or even better with BehaviorSubjects (Subjects that always have a sync value). E.g. you have Subject a and b and you want c to be computed based on a+b. No side-effects. I think, it's an analogue of computed in MobX or Vue ā€” it should've combined stream updates (preferably with sync value always available), not to manage streams.

But Rx is different: the subscription & unsubscription might cause a side-effect. That's why I don't encourage conditional usage of tracked observables: late subscription might lead to late side-effect. Not mentioning conditional re-subscription. These are really hard to argue about!

So conservative me would strongly suggest that the solution with operators is more transparent and therefore preferable, e.g. a$.switchMap(a => a ? b$ : null) (as in your example).

Yet experimenter in me thinks that it's an interesting idea! šŸ™‚

I'd say that both cases with keeping subscription and resetting it might be needed: re-establishing subscription might lead to unwanted or expensive side-effects, while sometimes, as in your case, one needs that unsubscribe-subscribe cycle.

So to meet the two scenarios, I suggest adding a pair of "weak" trackers: weak$ & weak_ ā€” these would drop the subscription once it's been detected unused, e.g:

const s = concat(of(true, false), NEVER);
const t = interval(100).startWith(0); // startWith is sync
const r1 = run(() => _(s) ? weak$(t) : -1);
const r2 = run(() => _(s) ? 1 : -1);
const r3 = run(() => _(s) ? 1 : weak$(t));

The names are surely not final. And it's unclear whether default trackers should be weak or strong.

What do you think?

kosich commented 4 years ago

(I've described current behavior as a bug in #8, please take a look)

Jopie64 commented 4 years ago

Thanks for your answer! I thought about it for a little longer. Actually, I think the semantics of late subscription, naturally lead to early unsubscription as well... But that brought me to an idea. What if we allow a user to iterate observables which need strong subscription? I think that would provide the best of both worlds and fixes the late subscription problem as well!

The API could be something like this:

const r = run(
    () => $(o) % 2 ? $(o2) : $(o3),
    [o, o2, o3]);

So the (optional) second argument of the run function has an array of observables that always need to be subscribed, even when they temporarily don't play a role in the result. (And even at the very beginning didn't play a part in the result, -> no late subscription.)

This would also naturally lead to what you said in #8. When an observable has strong subscription but temporarily doesn't do anything in the expression, its value changes can be ignored because a rerun wouldn't change the outcome. But when they are not strongly subscribed, they can be released, so they surely won't emit.

And I agree about that in rx, subscription side-effects play a role. Resubscription could be what a user wants to avoid. But it also could very well be exactly what he wants. And I think with this API extension the user could specify how he wants it to behave.

Let me know what you think.

Jopie64 commented 4 years ago

I just thought about that it could prevent another 'pitfall' as well. You could make it so that the first run callback is executed only once all strongly subscribed observables have emitted at least once. So when a user doesn't like the 'abort on the fly' behavior, he just has to specify all observables that play a role in the function.

kosich commented 4 years ago

It is interesting. Looks somewhat similar to 1st alternative API from #3 and React's hooks.

Let me recap our vocabulary ( šŸ™‚ ):

at least we get some terms out of this

So the dependency list you suggest instantly marks those observables as strong and used so that run waits for all of them to emit before first expression execution, right?

To confirm that I understand it right: this approach doesn't solve #8 where we probably should mark unused as temporarily untracked, it's just somewhat related?

kosich commented 4 years ago

It's really hard for me to argue what API should look like now, while we don't have much practice using this whole autorun approach. So if this particular issue is not blocking you now, I suggest we keep it open, resolve #2 and publish the first version to give it a try. And return to in following releases?

Jopie64 commented 4 years ago

Good to have this definition list! A lot easier to discuss with it.

Indeed this proposal would not directly solve #8, but I think it would feel natural to tackle this problem when implementing a solution for #7...

I would tackle it like this:

I think this would fix #8, and my proposal for #7. Also, when we want to do #7 differently, I think it would be relatively easy to implement when this mechanism is in place. The only thing that changes would be the decision to mark an observable touched or not.

But it's OK I think to keep this issue open and return to it after a fix for #2 and a release... Although, for our purposes I'm not sure it can be used without it.

kosich commented 4 years ago

I'm wary to add this to API yet šŸ™‚ I like the idea of dep list marking observables as used (maybe "touched" is a better term) to delay first execution. Though I'm not 100% sure if it should mark em as strong (though it does make sense, just not 100% clear).

Say, would it be enough for now in your use-cases if we keep current API and make Observables weak by default? So that we just:

Or you need a switcher between strong and weak?

Jopie64 commented 4 years ago

Weak by default would definitely be enough for our use-cases. I came up with strong-marking only to help possible other use-cases where resubscribe cycles would cause issues. It's even possible to use current API to mark observables as 'strong', although a bit less elegant :)

const r = run(() => {
   // Touch o2 and o3 so they become strong (aka won't be unsubscribed)
   _(o2); _(o3);
   return $(o) ? $(o2) : $(o3);
});

Still a workaround though because it wouldn't solve the 'interrupt midflight' problem...

But for the use cases I'm thinking of this would be enough.

kosich commented 4 years ago

Yeah, I thought of that hack too šŸ˜… That could be even a one-liner: run(() => _(o2), _(o3), $(o) ? $(o2) : $(o3))

(though I wrongly imagined it using $ which introduced another issue. Your example w/ `that later sets it to$` is šŸ‘)_

It has its issues but it should be good enough for v0.0.1!

Lets start with weak by default and then figure out what to do with strong refs and dependencies. Sounds good?

I see you've already started working on it? Want to create a PR on this too? šŸ˜… If so you can share early-stage PR if you want me to do an early review. Thanks!

Jopie64 commented 4 years ago

You were being served as you spoke :D And nice hack! I forgot about the comma operator.