kosich / rxjs-autorun

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

Add weak and normal strength to observables #10

Closed Jopie64 closed 3 years ago

Jopie64 commented 3 years ago

It will now only mark an observable 'tracked' when it was used as such in the last run of the expression. This would (maybe partly?) fix #8. Also it would unsubscribe an observable when it is not used any longer in the last run of the expression. This would (maybe partly?) fix #7.

We might want to update the semantics later to e.g., somehow mark observables as 'strong' so they will never be unsubscribed, and also won't 'suffer' from 'late subscription' and 'midflight interrupt'. This however requires API changes that are under discussion in #7.

kosich commented 3 years ago
run(() => {
  const n = $(o) % 2 ? $(o3) : -1;
  return n + $(o2);
})

@Jopie64 , I think, you're right saying that: while we don't yet have a value for o3, the changes of o2 are irrelevant. That's a good point!

My concern is that we mark o2 as unused, unsubscribe and drop cached value, although o2 is not unused, it's just waiting o3 to emit!

So imagine there's a time-consuming operation behind o2, say a request to a server that takes 1s. That means that every other time the o3 is dropped and re-used — we would also drop and re-use o2. Which makes another 1s request to the server each time. (not mentioning possible side-effects of a re-subscription)

Also note an inconsistency: if o3 would resolve synchronously — then o2 is marked as used. If o3 resolves asynchronously then o2 is marked unused & dropped/recreated. And that might be under no control of our user if it's not their Observable, but taken from outside.

I think, in the example above I would expect o2 to be kept as used for the lifetime of the expression.

This is hard 🤕. LMKWYT

kosich commented 3 years ago

I was answering your comment on a commit and then it somehow disappeared. Sorry if I've missed something: I'm a bit confused by this UX.

Jopie64 commented 3 years ago

I'm thinking of it this way: In my mind, there're 2 values at play the user could care about.

  1. It's costly to resubscribe an observable
  2. It's costly to keep a subscription open

Value 1 pleads for not closing an observable once it has been subscribed. -> strong subscription Value 2 pleads for closing an observable as soon as possible when it is not needed. -> weak subscription

We decided that weak subscription is currently the default. One reason for that is that by default you cannot know what observables are used in the expression in advance. I'd argue that, keeping value 2 in mind, it should unsubscribe an observable once it's value becomes irrelevant, even when that is because we are all the sudden waiting for a value of a different observable. It could take ages for that value to arrive, or it even could never arrive at all. I see what you mean by this inconsistency. If that different observable had fired a value immediately, it wouldn't have unsubscribed at all. But do you also see that a user could want this temporary unsubscription to happen because of value 2?

On the other side, if for a certain observable a resubscribe cycle is costly, hence value 1 becomes important, the user wants to make sure the observable keeps subscribed even when it is temporarily waiting for another observable that has not emitted yet but probably will soon. But this is leading to strong subscription. I agree that there should be an API for it to support that too. And a user can already make sure, although it being a workaround, an observable stays subscribed by marking it untracked at the beginning of the expression. I currently can't think of a workaround that would do it the other way around...

And yes, I agree it is hard... But at least I have a use-case that pleads for weak subscription, so that makes it a little bit more clear for me :P

Jopie64 commented 3 years ago

I was answering your comment on a commit and then it somehow disappeared. Sorry if I've missed something: I'm a bit confused by this UX.

What disappeared? My comment or yours? If yours: I hope you didn't loose too much...

And maybe mine is lost because I updated the PR? I can't find my comment on my own old code either... weird...

kosich commented 3 years ago

Yeah, I think your original comment I was replying to was bound to a commit. Nvm :)

Thanks, now I better understand your point! I'm not rolling back on the weak thing, I just saw weak is to be dropped only when it's explicitly not used (falling off a condition), while you want some very weak kind of weakness 😅 "eager weakness"?

Need to think about it: I'm worried that if not explicitly marked by the user, this behavior is unexpected (especially in inconsistent cases).

Can you share your theoretical use scenarios? E.g: what kind of observables you want to drop (why they are expensive) and what calculations you want to do in run. If possible, surely. I'd like to explore what options we might have

it should unsubscribe an observable once it's value becomes irrelevant, even when that is because we are all the sudden waiting for a value of a different observable. It could take ages for that value to arrive, or it even could never arrive at all

You make a good point here that the emission delay is undetermined.

It lead me to think: do you care about following kind of scenarios

run(() => $(a) ? $(b) : 0)
// or
run(() => $(a) + _(b))

b is that expensive connection you want to drop asap a emits only once a truthy value and then never emits again Therefore we use b once and then keep it alive for eternity. Maybe b should be dropped based on time passed since last use? (configurable) WDYT?

Edit: also, I'd like to understand whether your user would know if b is expensive when using it? E.g so they could mark it for that eager unsubscription.

Jopie64 commented 3 years ago

Can you share your theoretical use scenarios? E.g: what kind of observables you want to drop (why they are expensive) and what calculations you want to do in run. If possible, surely. I'd like to explore what options we might have

I currently can't really think of a realistic case what triggers this scenario, so a bit theoretical this one... Consider this presence case:

// alowedForAll$ -> emits boolean that says whether presence is allowed for all users
const presenceForUser = (userId: string): Observable<PresenceState> => {
  // (Lazy) request to slow unreliable server
  const allowedForUser$ = ajax(`.../isPresenceAllowedForUser/?userId=${userId}`).pipe(
    retryWhen(e => timer(10000)));
  const presence$ = monitorPresence(userId); // Costly when observed
  return run(() => $(allowedForAll$) || $(allowedForUser$) ? $(presence$) : PresenceState.NoPresence);
}

(Note that $(allowedForUser$) is only evaluated once $(allowedForAll$) is false)

First presence monitoring is allowed for all. But it turns out server resources are overloaded by all users monitoring presence of all other users. Hence the administrator disables presence for all and only allows it when someone is in the buddy list. To reduce server load quickly it's important that clients react quickly on the trigger of the administrator.

It lead me to think: do you care about following kind of scenarios

run(() => $(a) ? $(b) : 0)
// or
run(() => $(a) + _(b))

b is that expensive connection you want to drop asap a emits only once a truthy value and then never emits again Therefore we use b once and then keep it alive for eternity. Maybe b should be dropped based on time passed since last use? (configurable) WDYT?

First of all, IMHO I think timeouts are outside of this library boundaries. Other operators can be used for that. Second of all, a emits once and then completes? In that case, the first expression would start to emit values of b for eternity. This should be clear for the user. If he/she doesn't want that, he should write it differently using timeout operators IMHO :) In the second expression, it would emit only one value of b and then complete. (Currently it might not, but that was something I was going to propose next :) )

Edit: also, I'd like to understand whether your user would know if b is expensive when using it? E.g so they could mark it for that eager unsubscription.

I think this lib must assume users know what they are doing (they should know what the specs are of the observables they use, and what is costly to do with them). And it should also be clear for the user what to expect from this lib. I think, when the spec says dependent observables will only be subscribed when the value is needed to evaluate the expression, and will be unsubscribed/not subscribed at all, otherwise, I think it would be consistent when an observable is (maybe temporarily) unsubscribed because part of the expression is not reached anymore, no matter that is because of branching with or without midflight interrupt.

I might sometimes express my opinions a bit strongly. I hope you don't mind about that :)

kosich commented 3 years ago

Ah, the example is very good, thanks!

I might sometimes express my opinions a bit strongly. I hope you don't mind about that :)

Please continue expressing your opinions freely, Johan! After all, they say the truth is born in dispute 🙂 I hope I'm not a big push-back in return 😊

The case your suggesting is totally valid and I think we'll make it happen relatively easily! While it's so specific and tricky that:

  1. I would not dare to explain it to anyone who hasn't participated in this thread (us two 😅)
  2. I'd like everyone using this to be 100% sure what they are doing

So I think we should make this behavior consciously applied by user, not by default. For that we could split our weakness levels into three categories:

  1. strong — once visited, we create subscription and keep it
  2. normal — once visited, we create subscription and drop it if it falls off latest run (default)
  3. weak — once visited, we create subscription and drop it eagerly

API could look something like: $(…) for normal, and $.weak(…) $.strong(…). With similar thing on _. Therefore your example would be written as:

run(() => $(allowedForAll$) || $(allowedForUser$) ? $.weak(presence$) : PresenceState.NoPresence)

I suppose to implement this we could have temporary state, that we'll apply and clean at the end of try { … } phase (all cases). While in catch { … } we'll clean up all the weak cases and reset the temp state.

Tell me if this all makes sense

👋

Jopie64 commented 3 years ago

I'm happy you tolerate my behavior, and I hope you are patient with me once more :)

I like the $.xxx api to supply extra options to trackers!

But, bare with me, for me it still doesn't feel correct to (by default) not to do an unsubscribe when the expression didn't reach an observable due to a midflight interrupt... Thinking more of why I feel this way, I thought of this:

So it feels like, when you allow late subscription due to midflight interrupt, but not allow early unsubscription due to midflight interrupt, you are breaking the symmetry.

I wonder what you think of this.

Anyway, at least I can try to come up with code to not do unsubscribe on midflight interrupt. Then we can decide after that what will be the default. And of course, if we stay in disagreement about this, it's your decision that will be decisive, cause it's your lib/idea :)

kosich commented 3 years ago

Ha! Well, I thought if it were ever published — it would serve for simple calculations. That's you who gave it this extra weak/strong spin 🙂 Which is great! So I'm here not as a driving force, but rather as a constraining 🤷‍♂️ And I'm sorry for that.

Regarding the symmetry — I feel like it's a matter viewpoint now 😅 And I think understand your point.

While subscription can be side-effect-full and it's really hard to argue when weak unsubscription could happen (due to sync/async inconsistency), I'd still say that by default we should stay in the safe water. BTW, please note that you've already convinced me toward normal behavior symmetry as initially I thought connections should always be strong! 😄

So: if it's not going to blow up your code with $.weak everywhere (but only in special cases) and for the sake of moving forward a release and some practical experience (+feedback)

I suggest we go forward with $ as normal (meaning late subscriptions, dropping only when falling off logic tree) and $.strong / $.weak to be used for special cases.

If that won't work for us — we'll get back to that in later versions. We're in alpha, it's okay to be in flux!

--

Sidenote: beware that for weak observables there's also a case when the the mid-flight interruption happens not before, but after that weak observable usage. How to handle this case I'll leave totally up to you. I feel like I wont be able to handle this argument 😆

--

UPD: and it goes without saying: it's totally cool to push on this back if you think it's a critical difference!

Jopie64 commented 3 years ago

Ok, I think you've heard all my arguments now for going with my version of 'weak' :) So I'll try to come up with code for the 'normal' weak as default. (And hopefully with a test scenario that shows it really is inconsistent evil grin haha )

Regarding the sidenote case: When a midflight interruption happens after the weak observable usage, it remains subscribed. Technically because it is touched, and functionally because a change in that observable might cause a different branch to happen which doesn't have that midflight interruption. So this value is still relevant for the expression. In the test cases it is the observable that causes the branch to happen :)

Jopie64 commented 3 years ago

I pushed an update where 'normal' is now the default. Let me know what you think.

Jopie64 commented 3 years ago

Hmmm... I couldn't find a way to make the new test scenarios work without storing strength-restoration data... :(

Also I renamed trackType to strength, I think that better reflects what it is for :)

kosich commented 3 years ago

Great Job, Johan! 😅 👍 🍰

I let it close #7 and #8 — although in #7 we partially discuss eager subscriptions — we can create another issue for that later.

Merged! Thank you!

Jopie64 commented 3 years ago

Thnx for merging!

I also want to add strong subscription. Probably easy to implement now but requires more test scenarios. So I'll do that later. I think we don't need an issue for that, or should we still create one?

Anyway, could you also please check #13 which would (in my view) fix #11?

kosich commented 3 years ago

🎉

Yeah, I think we're fine w/o additional issue. Sure, checking 🙂

kosich commented 3 years ago

I've sent you an invitation to become a collaborator on this project 🙂 https://github.com/kosich/rxjs-autorun/invitations