Skip to content
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: new computed variant that's "active" only when observed #1359

Merged
merged 4 commits into from
May 8, 2014

Conversation

mbest
Copy link
Member

@mbest mbest commented Apr 18, 2014

Currently, a computed observable, once activated, remains active until one of the following happen:

  1. It is manually disposed by calling its dispose method.
  2. The DOM node specified using disposeWhenNodeIsRemoved is removed or cleaned.
  3. When a dependency changes, the disposeWhen function returns true.
  4. When a dependency changes, the read function returns without accessing any observables.

To clarify, active means that the computed observable is subscribed to one or more other observables and is updated when any of those are changed. This is important to understand because it means that an active computed observable cannot be garbage collected until all references to both it and all of its dependencies are dropped in an application. This could result in a memory leak if, for example, a computed observable references observables outside its own view model.

I'm proposing a new computed observable mode in which it only maintains subscriptions to its dependencies when it also has subscribers. When it has subscribers, it will act just like a "normal," active computed observable, But as soon as it has no subscribers, it will "dispose" its subscriptions and enter an inactive, "sleeping" state.

These are the states that a computed observable can currently be in (not counting intermediate states while it is updating):

  1. Created, but waiting to fully initialize (only when using the deferEvaluation option).
  2. Initialized and waiting for changes to its dependencies (normal, active state).
  3. Scheduled to update after dependencies have changed (when using throttle or rateLimit extenders).
  4. Disposed, with no dependencies and no chance for future updates (also called "inactive").

This feature will add a new computed observable state:

  • "Sleeping", waiting for subscriptions, during which the computed observable will be re-calculated when it is accessed and will not subscribe to any observables. This will also always be the initial state of the computed observable.

@SteveSanderson
Copy link
Contributor

Sounds reasonable. This would be a new option you pass when creating the computed, would it?

BTW, what kind of scenarios would you expect this to be used in? I'm not totally sure under what circumstances a computed would have no subscribers - normally I see computeds used in one of these scenarios:

  • Bound to UI (which is a subscriber)
  • As a source of info for more computeds onto it (which are subscribers)
  • As something manually subscribed to
  • To trigger side-effects from the evaluator (in which case you wouldn't enable this option)

So I'm curious about the cases when this would be effective.

@AdamWillden
Copy link
Contributor

@SteveSanderson I wonder if I might have a use case for this and I think it would greatly improve performance for me. All the same I'm wondering if I'm missing the point here as my understanding of the inner-workings of a computed observable is somewhat incomplete...

I have an array of view models which represent a collection of database entries. I maintain this collection (and many other collections like it) in the background for use over my application. This means that many viewmodels are retained in memory and are not bound to the DOM (and have no subscribers), however data received from the server may cause their computed observables to re-evaluate.

So to clarify for myself, is the point of this feature request to only immediately re-evaluate a computed when it has subscribers (e.g. the DOM or other subscribers)? If an inactive computed was later subscribed to would this put it back into an active state (for immediate evaluation)? Furthermore could/would this allow for an inactive chain of computed observables:

comp1 <- comp2 <- DOM
comp1 <- comp3 <- DOM
  • Comp1 is only subscribed to by comp2 & comp3 and they are bound to the DOM
  • Say the DOM is modified such that the bindings are broken, comp2 & comp3 become inactive. Would/Could this also cause comp1 to become inactive?

Apologies if I've completely missed the point @mbest was trying to make!

@mbest
Copy link
Member Author

mbest commented Mar 19, 2014

@AdamWillden You are right on. In your example, comp1 would also become inactive once comp2 and comp3 were inactive.

@mbest
Copy link
Member Author

mbest commented Mar 19, 2014

@SteveSanderson As you mentioned, this feature would not be used for a computed observable that triggers side effects (equivalent to a manual subscription). But for most cases, this feature provides two benefits:

  1. Ensures memory is freed when a sub-viewmodel is no longer referenced (of course, manual subscriptions will need to be disposed manually).
  2. Reduces computation overhead by not re-calculating computed observables whose value isn't being observed.

For most applications, I imagine that this could be made the default option for computed observables.

@AdamWillden
Copy link
Contributor

Fantastic, I'm glad I understood correctly 👍

I had also wondered if it made sense for this to become to default behaviour. Are there any cases where this would break existing implementations?

Either way I look forward to this being implemented :) I think you all do a brilliant job and I find your responses to be always patient and polite. Keep up the good work, you're making my job easier 😄

@alvingonzales
Copy link

If this was made default, it would break:

I'd love this implemented too, but maybe save making it the default behaviour in a major version release and make it opt-in for 3.x

(Speaking up as I have quite a lot of code implemented based on the async observable case :) )

@AdamWillden
Copy link
Contributor

Yep, in fact I do that at least a couple of places in my code and I can see how it might be used extensively in others! Thanks for the additional clarification.

@mbest
Copy link
Member Author

mbest commented Mar 21, 2014

Thanks, @alvingonzales, for spelling that out. I am not planning to make this the default option for computed observables but merely suggesting that developers could make it the default setting for their own applications (for example, I use deferEvaluation as a default option for view-models in my own applications).

@mbest
Copy link
Member Author

mbest commented Apr 1, 2014

I've started to work on an implementation of this and would like some feedback on the API, specifically how things should be named.

So far, I have a ko.computed option, canSleep, and a method, isSleeping(). But the option isn't how I expect this feature to be used. I also have ko.functionalComputed that returns a computed observable with sleeping enabled, emphasizing the type of situation this feature is useful for.

@AdamWillden
Copy link
Contributor

What are you asking? Do you mean that you don't like the term 'sleep'? If so, you could name the option restful and use ko.restfulComputed perhaps.

@mbest
Copy link
Member Author

mbest commented Apr 1, 2014

@AdamWillden, I'm just looking for feedback. What's your opinion on the names?

@AdamWillden
Copy link
Contributor

functionalComputed does't mean anything to me. Perhaps you could explain why you think it suits? I think restful or even lazy better describes it to emphasise how it only does work when it needs to. Lazy fits in with the sleep option ;-) I'm not overly fussed of course but I dont think functionalComputed separates it in behaviour - is the standard ko.computed not functional?!

@mbest
Copy link
Member Author

mbest commented Apr 1, 2014

I mean "functional" as in functional programming; specifically relating to avoiding state and mutable data within the computed's function.

@AdamWillden
Copy link
Contributor

Thanks @mbest. While I do think myself an experienced programmer, I probably wouldn't be considered seasoned, either way I'm far from new and even after skimming that WIKI I don't feel the name is memorable (in relation to it's perceivable behaviour) or succinct (dependantObservable -> computed). I understand it may be a fitting name with regards to its inner workings but this is an 'outward facing' name.

I, like many newcomers, read the documentation (which is brilliant) and I believe names should make sense or at least be explainable to non-programmers. If it could be explained to someone who's not a programmer than it's likely to make more sense to someone who is and therefore be memorable making the framework more accessible.

Would be nice if someone else reading this were to chime in with their thoughts however - cue fellow commenter

Not to mention you can save me 6 characters of typing 😉
lazyComputed
functionalComputed

@SteveSanderson
Copy link
Contributor

How does the implementation look? Is it a small addition to the existing ko.computed logic, or does it involve major refactoring?

About naming, that's always a challenge :) If you want to emphasise the "functional" use case, which I do agree makes sense, then I guess a natural candidate term would be pure. People should use the new behavior if and only if their evaluator is a pure function, right?

For side-effect-free evaluators, it's always preferable to suppress evaluation if the result isn't being used, and for side-effectful ones, it's (virtually) always necessary to trigger the side-effect regardless of whether the return value is being used by something else.

This looks quite neat and natural to me:

this.something = ko.computed({
    read: function() { ... },
    pure: true
});

@SteveSanderson
Copy link
Contributor

Actually I recognise that the kind of purity implied is only about lack of side-effects, and not lack of dependence on external mutable state. The exact definition of "pure function" given on wikipedia wouldn't be applicable here, since computed evaluators never accept parameters. Instead we would regard "all other state in the system" as being an implicit parameter to the function, and by that definition a side-effect-free function does qualify as pure.

Of course, this might confuse and offend people, so maybe it's not the best choice of name :) If we wanted to be really explicit,

this.something = ko.computed({
    read: function() { ... },
    hasSideEffects: false
});

But that is a lot of typing.

@SteveSanderson
Copy link
Contributor

Other possible, non-pejorative shorthands for "no side effects":

@mbest
Copy link
Member Author

mbest commented Apr 9, 2014

Steve, thanks for the suggestions. The implementation so far is pretty small. I'm still working on the tests and making adjustments.

@gilesbradshaw
Copy link
Contributor

Pretty sure this is related to this issue I've just raised: #1390

I extend observables to give me events when they are subscribed to and unsubscribed to like this:
(forgive coffeescript)

ko.observable().extend
    listener:
       subscribeActions:->
            onSubscribe:-> 
            onDispose:->

This lets me do things like getting data with ajax and subscribing and unsubscribing to signalR endpoint when observables are subscribed to and unsubscribed from. This is quite good because you just have to look at stuff from the view for data to get fetched from the server.

however when I have observables extended like this and referenced by a computed which then gets subscribed to:

obs = ko.observable()
    listener:
       subscribeActions:->
            onSubscribe:-> 
                dosomeajax.then (data)-> obs data
            onDispose:->

comp = ko.computed
    deferEvaluation:true
    read:()->'compute from ' + obs()

and I have some markup:

<!-- ko if:lookAtComp -->
    <span data-bind='text:comp;></span>
<!-- /ko -->

then when lookAtComp goes false comp stays subscribed to obs

I was quite surprised by this behavior actually

In the code appended to my issue 1390 above there is a hack I made to get round this. Hopefully what @mbest is proposing will do it properly

cheers

…o subscribers. Add ko.pureComputed to encapsulate that interface, emphasizing that this approach works best if the computed doesn't modify any external state. Update specs to use ko.computed instead of ko.dependentObservable.
@mbest
Copy link
Member Author

mbest commented Apr 18, 2014

I've uploaded the implementation to 1359-pure-computed

@gilesbradshaw
Copy link
Contributor

Thank you very much - I can confirm that resolves my issue. I think I might always be using pure computeds from now on as this is how I had assumed they worked.

@gilesbradshaw
Copy link
Contributor

I wonder also if pure computeds should default to deferEvaluation:true?

@mbest
Copy link
Member Author

mbest commented Apr 19, 2014

A pureComputed already includes the deferEvaluation behavior; that is, it defers evaluation until the computed is accessed or subscribed to. It ignores the deferEvaluation option.

@mbest
Copy link
Member Author

mbest commented Apr 30, 2014

There are a couple of issues with this that I think should be changed.

  1. ko.computedContext.isInitial() allows a pure computed to return a different value for the first evaluation, but this really shouldn't be allowed. From Wikipedia: "The function result value cannot depend on any hidden information or state that may change as program execution proceeds..." So I think isInitial() should always return the same value for a pure computed (either false or undefined).
  2. Knockout silently blocks computed observables from being evaluated recursively. For a pure computed, recursion is only possible if the computed accesses itself, which is an error. I feel that rather than silently preventing recursion, we should throw an error (or just allow the browser to do so--"stack overflow").

…ort on initial evaluation for pure computeds.
@SteveSanderson
Copy link
Contributor

Generally this looks great. Thanks for designing and implementing it! A few questions:

  • What's the use case for exposing isSleeping? That seems like internal state - under what circumstances would a developer rightfully vary their logic depending on it?
    • If we are keeping this, what does isSleeping return after disposal? Sorry if that's covered in specs somewhere and I missed it. I'm not sure what it should return, since the computed is kind of sleeping, but it's definitely never going to wake up again. Maybe false would be the most useful result.
  • Why don't pure computeds auto-dispose themselves? I would have expected that, if some evaluation during sleep detects no dependencies, then the pure computed would behave like any other computed and become disposed. Otherwise we introduce a new scenario whereby computeds may sometimes change value even if they don't depend on any observables, which was previously not possible.
  • See also my comment on dependentObservable.js line 330 - I think we should avoid mutating parameter objects where possible, as doing so exposes an implementation detail.

Also, I realised that my earlier statement Developers should use pureComputed if and only if the evaluator has no side effects is wrong. I think the truth is: Developers should use pureComputed if and only if both of the following hold:

  1. The evaluator has no side-effects
  2. The evaluator is cheap, so you don't need to cache its output.

We should be clear about this in docs.

@mbest
Copy link
Member Author

mbest commented May 8, 2014

Thanks for the feedback, Steve. I've made changes based on your suggestions. Here's what I was thinking:

  1. I had thought it might be useful to expose isSleeping, but I agree that it's better not to do so until we discover a specific need.
  2. I had disabled auto-dispose for pure computeds because I was thinking that the original purpose of the feature wouldn't apply to a pure computed--to not add a disposal callback to nodes. But you brought up a good point--that it keeps the computed behavior consistent.

SteveSanderson added a commit that referenced this pull request May 8, 2014
Proposal: new computed variant that's "active" only when observed
@SteveSanderson SteveSanderson merged commit 224f8e9 into master May 8, 2014
@SteveSanderson
Copy link
Contributor

Fantastic, thanks! The changes make sense and I think this feature will be really valuable.

@gilesbradshaw
Copy link
Contributor

@mbest could I possibly ask you to make me v3.1.0 script with this added to it due to my lack of building competance

many thanks

@mbest
Copy link
Member Author

mbest commented May 15, 2014

@gilesbradshaw:

It's available here: https://github.com/knockout/knockout/tree/v3.2.0-alpha/dist

Or through Bower: bower install knockout#3.2.0-alpha

@gilesbradshaw
Copy link
Contributor

Many thanks

@mbest mbest deleted the 1359-pure-computed branch May 26, 2015 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants