1

I've got a toolbar where each tool can be disabled/hidden independently. The tools depend on other system events and emit individual events configuring their availability. The toolbar uses combineLatest to pull all the tools together and emit a toolbar config.

The combineLatest is listening to 40+ events.

Will this be a performance problem? Is there a practical limit to how many events combineLatest can consume?

NDavis
  • 1,127
  • 2
  • 14
  • 23
  • https://stackoverflow.com/q/22747068/6680611 – cartant Jul 10 '18 at 00:12
  • 1
    you mean how many **streams** can be combined ? there is no hard limit, and Angular reactive form easily handles hundreds of observable streams (from the form controls in a tree), so you should ve ok in theory. On a practical side, if you will be combining them in naive / brute-force way (as args to same function), I'm pretty sure after 10 you will loose the type-safety. – c69 Jul 10 '18 at 00:59
  • Where do you suspect a bottleneck? – a better oliver Jul 10 '18 at 14:23
  • I'm wondering about the amount of overhead in using combineLatest for multiple events. From what I'm hearing, I think I can assume it's negligible and the real cost is in how often those events fire – NDavis Jul 10 '18 at 18:06

1 Answers1

0

Hard to say just like that.

I think that having a huge number of streams combined is not a problem on it's own.

What could be:
- having those streams emitting a value very very often
- having those streams triggering Angular change detection (might be worth running them outside ng zone if possible)

That said, I think that the performance problem here is hiding a conception problem eventually. It really feels like you might need a "single source of truth". Having a look into Redux and eventually Ngrx might be a great help for you.

Then from the unique store, you could easily retrieve the availability of your tools.

The tools depend on other system events and emit individual events

The Redux pattern is generally playing very well with that kind of challenges:
- Async
- State

It really sounds like it might be a perfect fit here.

If you don't know where to start, I'd advise you to first read the Redux documentation. It's one of the best I've ever read: https://redux.js.org

Once you understand how Redux works and whether it's a good fit for you or not, if the answer is yes then take a look into Ngrx. As you seem to be working with streams a lot, if you take the time to learn Redux first then Ngrx will definitely not be a problem: https://redux.js.org

If you decide to go this way, good luck into this amazing journey of reactive and functional programming :)

EDIT 11/07: If you think that Redux is overkill then maybe you could build a minimal solution that acts a bit like it. The following is completely type safe and you can update multiple properties without firing the final stream as many times as you update properties. Once is enough:

import { BehaviorSubject } from 'rxjs';
import { tap } from 'rxjs/operators';

type YourDataType = {
  prop1: string,
  prop2: string,
  prop3: string,
  prop4: string,
  // ...
  prop40: string,
};

const properties$: BehaviorSubject<YourDataType> = new BehaviorSubject({
  prop1: '',
  prop2: '',
  prop3: '',
  prop4: '',
  // ...
  prop40: '',
});

const patchProperties = (updatedProperties: Partial<YourDataType>) =>
  properties$.next({
    ...properties$.getValue(),
    ...updatedProperties
  });

properties$
  .pipe(
    tap(x => console.log(JSON.stringify(x, null, 2)))
  )
  .subscribe()

patchProperties({
  prop3: 'new prop 3'
});

patchProperties({
  prop1: 'new prop 1',
  prop2: 'new prop 2',
  prop3: 'final prop 3',
  prop40: 'new prop 40',
});

Produces the following output:

{
  "prop1": "",
  "prop2": "",
  "prop3": "",
  "prop4": "",
  "prop40": ""
}

{
  "prop1": "",
  "prop2": "",
  "prop3": "new prop 3",
  "prop4": "",
  "prop40": ""
}

{
  "prop1": "new prop 1",
  "prop2": "new prop 2",
  "prop3": "final prop 3",
  "prop4": "",
  "prop40": "new prop 40"
}

Here's a Stackblitz demo:
https://stackblitz.com/edit/typescript-zafsnk?file=index.ts

maxime1992
  • 22,502
  • 10
  • 80
  • 121
  • 1
    how is this answer related to RxJS (or the OP question) ? – c69 Jul 10 '18 at 00:53
  • I did tell him about the number of values emitted by a stream and also talked about angular CD. Then I told him that what he's doing is probably going into a way where his app is going to be complicated to manage and gave him an idea to improve it: Do not use 40 streams but rather one with the 40 data into it. Which is going to be easier to manage, easier to debug, easier to test and won't have type safety issues. – maxime1992 Jul 10 '18 at 07:28
  • _"Do not use 40 streams but rather one with the 40 data into it."_ That's what `combineLatest` is for. The result of using Redux would be the same, but the code would get unnecessarily bloated. Please note that the question is not about about Angular at all. – a better oliver Jul 10 '18 at 14:23
  • True, a redux pattern would be another way to solve this problem. Not a bad suggestion but probably not an option for me at this point – NDavis Jul 10 '18 at 18:07
  • "That's what combineLatest is for" @abetteroliver I strongly disagree. Rxjs is not here to fix a design flaw. If you've got to ship 40 box, would you rather use 40 cars, each with 1 box or one van containing them all, well packed? – maxime1992 Jul 10 '18 at 21:34
  • If all the 40 options are separated, I don't know if it's a problem in this case of not but it might end up in some inconsistency too. For example let say you toggle/update 30 of them, with a solution like redux you could apply all the changes at once in the store, which would trigger only one update within your stream. With 40 different streams, you'll end up with in this case 30 "notifications" eventually computing and computing some costly operation instead of doing that once. So regarding: "The result of using Redux would be the same", so yes but at which cost? Readability, testing, ... – maxime1992 Jul 10 '18 at 21:37
  • "Please note that the question is not about about Angular at all." fair enough. Forget about the term "ngrx" and just use Redux or Redux observable. – maxime1992 Jul 10 '18 at 21:39
  • 1
    "Not a bad suggestion but probably not an option for me at this point" @NDavis you can totally adopt Redux in an incremental way. You don't have to use for the whole app and could totally only use it to manage only this part. "not an option for me at this point" sometimes it's better to refactor some code than just keep something "rotten". But yeah I understand that you probably have a limited time too. I'm just giving ideas to makes things in a simpler way IMO :) Maybe you will not take that into account now but some other day :) – maxime1992 Jul 10 '18 at 21:43
  • There seems too be a misunderstanding. I'm not necessarily arguing against Redux. In this case(!) `combineLatest` does exactly the same as Redux: Creating a new state based on an event. It acts as reducer. There is no _"costly operation"_ involved. Yes, emitting an event to a store directly could(!) be a better choice designwise, but introducing Redux / ngrx just for this use case seems highly exaggerated. – a better oliver Jul 11 '18 at 08:26
  • "combineLatest does exactly the same as Redux" once again I disagree. Change 10 properties and your output stream (the one after the combineLatest) will be fired 10 times whereas you could do that only once (not only with Redux) but at least with one source of truth regarding those properties. "There is no costly operation involved": Do you know the app NDavis is working on? I'm not talking about rxjs cost, I'm talking a potential costly operation when those streams are updated. And if there's that might fired it 10 times instead of one. – maxime1992 Jul 11 '18 at 09:17
  • @abetteroliver take a look into the post, I've just edited it and added a "redux" like solution which doesn't seem "highly exaggerated" – maxime1992 Jul 11 '18 at 09:22
  • Ah, I see. You assume that properties can be changed at once which is not the case in the OP's example. – a better oliver Jul 11 '18 at 09:36
  • Who can do more can do less right? But even if you want to update one property at a time I'd still go this way instead of creating X streams. – maxime1992 Jul 11 '18 at 09:38
  • Sure, but out of interest: what are your arguments against streams (provided that we can update only one property at a time)? Please bear in mind that _"The tools depend on other system events"_, meaning that there will be streams anyway. Best case scenario: The OP leverages those streams to emit events, so there would be little overhead. – a better oliver Jul 11 '18 at 10:14
  • Easier to reason about a single source of truth, easily reusable: what if you need to access all those data again, importing all of them again and merging another time all of them? Also easier to test and mock. "The tools depend on other system events" that's fine. Just update the "store" with the good values. But "The tools depend on other system events" is making me think that the system is probably not that simple and redux is probably not overkill here either. – maxime1992 Jul 11 '18 at 10:25