-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Trampolining (or CurrentThread) Scheduler #30
Comments
I think I follow, but how does this differ from the current scheduler implementations in this library? If no scheduler is specified, I think we're currently defaulting to the currentThreadScheduler, and you're looping over an array in a @trxcllnt, can you review the schedulers for me and ensure that's the case? Perhaps I've stared at them too long. |
@Blesh the current-frame-scheduler as it exists today is analogous to RxJS's ImmediateScheduler. I've opened this issue to track creation of a scheduler that's analogous to RxJS's CurrentFrameScheduler. I have a prototype of this scheduler in this gist. The difference between the two is how they handle re-entrancy:
By Observable creation operators defaulting to synchronous iterative loops in the absence of a scheduler is an implicit implementation of the recursive scheduling strategy, which obviates the need for a recursive scheduler at all (i.e. the ImmediateScheduler). But we still need an implementation of the trampolining strategy (i.e. the CurrentThreadScheduler). My prototype uses a linked-list instead of an Array stack, to avoid the |
I see what you're saying. I think I whiffed the semantics with naming current-frame-scheduler as I did, it should be immediate-scheduler. :\ |
@Blesh tl;dr: If all the Observable creation operations default to iterative loops, we should only need the trampolining scheduler, which we still need to write. |
That's fair... However, I wonder what the performance implications of trampolining will be on the library. Is it worth the perf decrease to protect re-entrant behavior? Or is that more of an edge case? I had defaulted to immediate scheduling because I felt that trampolining had limited benefits the majority of the time. |
Trampolining is currently the default behavior in RxJS, and breadth-first vs. depth-first definitely have implications on functionality (esp. with nested flatMapLatest, expand, etc.). I don't mind switching to the recursive scheduling strategy by default, but it's important to include the trampolining scheduler. Since creation operators will default to iterative looping (recursive strategy), implementing a trampolining scheduler won't have any performance implications except when you buy into that scheduling strategy. |
@trxcllnt just had a conversation with @jhusain about this. I think that we're on the right course defaulting to the immediate scheduler, but you're right, we need to add the trampolining scheduler in as well. Jafar had a good suggestion, which is, that although I've already done some of the work of putting the schedulers together, we might just want to port what RxJS 2 already has to TypeScript and use those. They're battle-tested, after all. I'm okay with that, as long as we can look at the code and feel it's performance is up to what we want out of it. |
TODO:
|
@Blesh I talked with @mattpodwysocki about this last week. The existing CurrentThreadScheduler uses a priority queue, which isn't necessary in JS's single-threaded environment. An alternative is to use an Array stack, but that could non-deterministically thrash the GC if it grows large enough, and there's a cost to shifting actions off the front. I used a linked-list, which is three hash-lookups on insertion, and max two on execution or disposal. I can write a more formal LinkedList class if desired. We'll also need a scheduler that guarantees scheduled actions are executed on the next tick, and not in the current execution context. Maybe we should call it the ZalgoAgnosticScheduler? ;) I'll file a ticket to track what our story is on the return value from |
@trxcllnt The ZalgoAgnosticScheduler should be the As for the return value from Observable#subscribe ... I want to keep it to the current ES7 spec, which is now returning a Subscription object on it, with nothing more than an |
Awesome. Looks like the next-frame-scheduler is doing what I described. If perf is an issue, we could benchmark the micro task queue's Array against an LRU. We'll also need to ensure the schedulers are returning Subscription instances, so scheduled actions can be canceled. @jhusain Any reason Disposable got renamed to Subscription? Originally I remember the Subscription was supposed to be some sort of Generator-like value, but it seems it's been relegated back strictly to disposal. Since disposal is a more universal concept than subscription, it seems we should generalize Subscription back to Disposable (or Cancelable, as raix called it). |
Yes. It was renamed Subscription to correspond with the subscribe method we expect the majority of devs to use. Disposable is a .NET concept that is sufficiently general as to be non-ergonomic. We moved away from retuning a generator because we couldn't come up with any use cases for notify and error talkback. Use cases would be appreciated if you have any ideas. JH
|
@jhusain Are you implying schedulers should return Subscriptions, or that they return an inconsistent type? Narrowing the concept of disposability to the domain of Observable subscriptions seems non-ergonomic when the full spectrum of disposable behaviors is considered. |
I'm not trying to imply the scheduler should return subscriptions. The only change I was talking about was the fact that we have named disposables subscriptions, and we return a subscription not a generator from subscribe. Sent from my iPad
|
@jhusain so what should schedulers return?
We obviously need a type to encapsulate resource management. Subscriptions are resources, scheduled actions are resources. Shouldn't they be congruent? If people get hung up on the name "Disposable," let's call it something else. I don't want to bikeshed on the name, but I'm curious why the spec differs from the principles established and proven by Rx.NET, RxJS, RxJava, and ReactiveCocoa. |
Schedulers are not a concept being introduced for standardization. Implementations are free to do what they like. My preference would be to return a subscription. It's worth noting that we are in line with RxJava here: worker = Schedulers.newThread().createWorker(); worker.schedule(new Action0() { @OverRide public void call() { yourWork(); } }); // some time later... worker.unsubscribe(); It's my guess that returning a subscription from a scheduler will not blow any minds. However we can ask Ben Christensen if this introduced any confusion. JH
|
|
@jhusain @headinthebox I'm 100% on the Disposable -> Subscription rename. I was concerned the spec is defining a type that isn't compatible with more use-cases than just subscription. |
cc/ @benjchristensen |
As an aside: @jhusain: @trxcllnt and I had a conversation just yesterday in which I postulated that there should be native schedulers built into JavaScript, given that people keep reinventing the same solutions to the problem. They seem like a solid add to any event driven system or event loop architecture. |
@trxcllnt can this issue be closed? Are we all satisfied with the direction we've chosen with Scheduling? |
Need an implementation of a trampolining scheduler (Rx2's
currentThreadScheduler
). Don't need an implementation of the recursive scheduler (Rx2'simmediateScheduler
) if all theObservable
creation functions (of
,fromArray
,returnValue
, etc.) fallback to imperative loops in the absence of an external scheduler. This is possible since observers now return iteration results from onNext, onError, and onCompleted.Example:
@Blesh @jeffbcross
The text was updated successfully, but these errors were encountered: