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

refactor(combineLatest): small efficiency changes #6021

Conversation

benlesh
Copy link
Member

@benlesh benlesh commented Feb 12, 2021

  • no longer use an array to track first value emission
  • inline a few things than can be inlined
  • adds more comments

* inner firehose observables can teardown in the event of a `take` or the like.
* @internal
*/
class CombineLatestSubscriber<T> extends Subscriber<T> {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need this, we can use OperatorSubscriber.

// The number of inner sources that still haven't emitted the first value
// We need to track this because all sources need to emit one value in order
// to start emitting values.
let remainingFirstValues = length;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I'm using a countdown, instead of an array, to track whether or not each source has emitted at least one value.

@benlesh benlesh requested a review from cartant February 12, 2021 15:42
- no longer use an array to track first value emission
- inline a few things than can be inlined
- adds more comments
@benlesh benlesh force-pushed the blesh/refactor-combineLatest-small-efficiency-improvements branch from c026e2d to 5c67d95 Compare February 12, 2021 20:50
@benlesh benlesh merged commit d362c38 into ReactiveX:master Feb 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants