Many logics causes deep call stacks

[riston]

Issue description

Application with many logics(example 100) would cause deep RxJS "next" call stacks.

I have recreated the issue with one of the example, added 100 dummy logics which are not triggered (https://github.com/riston/redux-logic/blob/perf-logic/examples/async-fetch-proc-options/src/users/logic.js). Dispatching the "fetch" call(could be any other as well) which now causes really long call stack.

Here are the Javascript profiler screenshots to give a better overview:

Start and dispatch part, function with names would make debugging easier:

Fulfilled http request promise handling, calls actions and causes another "next" cascade.

Possible solution

• Reduce the number of logics in application, dynamic add/remove logics.
• Inner changes in redux-logic code, to reduce the "next" cascade?

Why is this problem?

Mobile Safari has smaller execution stack which causes silent failures in stream handling, also possible freezing, crashing and other unexpected behaviours.

[jeffbski]

Thanks for identifying this @riston. I can see if there are ways to improve the situation internally or like you said via dynamically adding and removing logic. Maybe there are other strategies that could be done to segment for stacks. I can also discuss with the RxJS team.

I guess the next obvious question is: has there been any real world failures in a redux-logic app due to this (on Mobile Safari or otherwise)? or is are you just doing due dilligence to anticipate potential problems?

It's certainly worth giving some thought either way. I just wanted to know how to prioritize it along with everything.

[askot]

Hi Jeff, thank you for quick response. I am working together with @riston and we are seeing this right now in our app.

Our app has grown to having over 180 logics, and with recent release (after adding bunch of code), we started seeing issues on IOS browsers. Basically, when dispatching an action that dispatches a lot of secondary actions, app would stop responding, usually, without an error.

Debugging with the logics monitor$ showed that dispatching still starts, but code logics is not triggered any more. And same thing can be reproduced on desktop Chrome, when running with smaller stack size (for example for Chrome Canary - /Applications/Google\ Chrome\ Canary.app/Contents/MacOS/Google\ Chrome\ Canary --js-flags="--stack-size 400").

[jeffbski]

Thanks for the update @askot. I'll take a look and see what we can do to help this situation. And thanks for the tip about how to reproduce in canary that may help.

[riston]

As @askot already mentioned this is quite problematic for mobile devices. In the first example only single action gets dispatched but when multiple actions get dispatched the call stack gets really wide.

Here is the same example as before but added one additional dispatch after the http client is fulfilled:

The click handler takes ~650ms:

The part when XHR state change takes in total ~1.2s:

I am not a RxJs expert and it's always hard to spot or debug cases like this, but do you have ideas which might cause this issue?

[jeffbski]

I tried running with Canary with limited stack as you mentioned

/Applications/Google\ Chrome\ Canary.app/Contents/MacOS/Google\ Chrome\ Canary --js-flags="--stack-size 400"

and ran your example with the 100 logics (https://github.com/riston/redux-logic/blob/perf-logic/examples/async-fetch-proc-options/src/users/logic.js)

Should I expect it to fail? (or just to show the stack depth?)

Do you have any other examples that will fail if I use Canary with a limited stack size? I'd love to be able to reproduce the error condition you are seeing.

[jeffbski]

I was thinking about the issue and I wonder if it could be solved by swapping out the logic stack as you change to different parts of your app? That way actions are only running through a subset of logic in each part of your app. I think that would keep the stack size down.

We could build extra functionality in redux-logic to help with this if this turns out to be a good approach, but you can get started trying that now just using the replaceLogic.

So what you could do is instead of creating one array of logic.

You could create different arrays of logic that you can switch to in different parts of your app.

So you might create a foo group and a bar group of logic and then as you navigate to that part of the app, you could switch to that group of logic.

const fooLogicGroup = [
  logic1,
  logic2,
  logic3
];

const barLogicGroup = [
  logic10,
  logic11,
  logic12
];

You use the logicMiddleware instance that you created with createLogicMiddleware. I like to attach it to the store after I create it store.logicMiddleware = logicMiddleware so that I have access to it where I need it.

// assuming you have stored your logicMiddleware instance on store
store.logicMiddleware.replaceLogic(fooLogicGroup);

// elsewhere
store.logicMiddleware.replaceLogic(barLogicGroup);

If you need to have some logic in both groups, that is fine, just add them in as necessary to each group.

When you use replaceLogic, only that group will be running going forward (though existing actions being processed in the old stack will complete) so new actions will only hit this group of logic.

So I think that would allow you to really segment down your app and limit the number of logic that need to work on your actions at any one time.

See if this will work for your app and let me know the results. If you have any problems let me know. If this seems like a workable solution but we need some additional functionality to help manage it, we can brainstorm on some ideas. Right now I just want to see if this gets you past your issue.

[riston]

Hello Jeff, thank you for a quick response. I was mistaken the application does have more than 180 logics added to the application. Grouping logics and adding these dynamically sounds a good idea, although in our case one group would still have 160+ logics which would be in the same group.

The test with limited stack size would simulate similar issues occurring on iOS Safari. It does not need to be Canary, Chrome also supports following arguments --js-flags="--stack-size 400". With 180 logics, the following example(https://github.com/riston/redux-logic/blob/perf-logic/examples/async-fetch-proc-options/src/users/logic.js) should have silent failures, the monitoring stream seems to work but other actions dispatching starts failing:

Trigger users fetch, the fulfilled is triggered but never processed:

configureStore.js:33 > {action: {…}, op: "top"}
configureStore.js:33 > {action: {…}, name: "L(USERS_FETCH)-0", op: "begin"}
configureStore.js:33 > {action: {…}, nextAction: {…}, name: "L(USERS_FETCH)-0", shouldProcess: true, op: "next"}
configureStore.js:33 > {action: {…}, dispAction: {…}, op: "dispatch"}
configureStore.js:33 > {action: {…}, op: "top"}
configureStore.js:33 > {action: {…}, name: "L(USERS_FETCH)-0", op: "end"}

Triggering now cancel, no process:

> {action: {…}, op: "top"}

Here all the logics get wrapped https://github.com/jeffbski/redux-logic/blob/master/src/createLogicMiddleware.js#L223 and iside the wrapping function https://github.com/jeffbski/redux-logic/blob/master/src/logicWrapper.js#L39 each logic would receive separate stream? When some action A triggers, all the logics X streams would start filtering and merge mapping? More logics there are, the heavier gets the stack?

[jeffbski]

Ok, I can look closely at that example to see what is occurring. Thanks.

As for https://github.com/jeffbski/redux-logic/blob/master/src/logicWrapper.js#L39 I am using a share() because all the logics do want to share the same rxjs stream. That allows later logics to intercept previous values since they feed through this pipeline.

However that brings up an idea. The most common use of logic is for the process hook and not all logic actually need to do any interception (validate or transform). It is probably a much smaller subset.

I was already pondering an internal restructuring of redux-logic that I thought would simplify the code and maybe it would help the situation in this way. I was planning on handling the interception and processing hooks in a separate fashion.

So if the most logic doesn't use interception (validate or transform), then those pure process logics doesn't need to see all of the previous actions. Logic that is purely a process only hook only needs to know when to fire with its action. So the interception pipeline depth would be much smaller than the total number of logics.

Then for the process hooks, they probably don't even need to be in the same pipeline. They just need to be triggered in a predictable order but otherwise they do their thing independently.

So for your logic use cases, how many of them use interception (validate or transform)?

If it is indeed a minority, then I think the new structure would work well in reducing the stack size.

Let me know what you think.

[riston]

So in our case, we have 100% pure process hook based logics we don't use the validation, transform hooks. Many of our logics use multi actions dispatching which causes really long call stack. The separation of handling interception and processing hooks seems a great idea 👍 .

[VitalyChe]

The idea sounds great but could you, please, share some example of the implementation! Thank you!

[jeffbski]

Great @riston that sounds like a winner then.

Yes @VitalyChe I can write or show a design of how it will work, that way we can all look at it before I dive into the code. Just in case there are other things to consider. The more eyes the better.

I'll get something up for review asap.

[jeffbski]

@askot @riston I have an experimental branch that I would like you to try to see if it helps with the limited call stack issue on mobile browsers.

I have uploaded it to a branch "add-process-delay"

So you should be able to install it into your app using

npm i -S "jeffbski/redux-logic#add-process-delay"

This should pull it directly from the github branch. I have pushed the build/dist files to this branch so they are already built and thus it should install just like it would from npm.

Once it installs it should list it as redux-logic@0.12.4-a0-async-process, so you will know you are using the updated version

npm ls redux-logic

Anyway, I'd like to see if this help with your limited call stack problem. Basically I just made a change so that it runs the process hook asynchronously which should result in a smaller call stack for the process side of things.

Let me know how this version works for you.

Thanks.

[riston]

Hello Jeff, I tested the Rx scheduler change on the async-fetch-proc-options example, the execution cycle seems to be faster compared to the previous result(~650ms), although the stack size would stay same.

[jeffbski]

@riston does it prevent the failures you were seeing?

I'm still planning on doing the refactor I mentioned, but I was hoping that this might buy me some time to not have to rush it.

[askot]

Hi, Jeff, thank you for you work.

Yes, this patch seems to work around the freezing issue, so that our app does not hang any more on iOS where it was reproducible before.

As a side effect it brings out some wrong assumptions we had made on running order of logics, breaking data loading and some other timing issues. So it could be a breaking change for other users as well.

Also, wanted to thank you for a great library, it has helped us a lot to organise redux code in a clear and meaningful way.

[erkiesken]

Hey @jeffbski,

I'm in the same team with @riston and @askot. For now we had to fork this library to fix our issues for good. Your above workaround only alleviates the problem a bit.

The main issue is that this code in applyLogic chains all the logics together in nested merged observables: https://github.com/jeffbski/redux-logic/blob/v0.12.3/src/createLogicMiddleware.js#L227

Also, the share call here doesn't actually share the original actionIn$/actionSrc$ stream: https://github.com/jeffbski/redux-logic/blob/v0.12.3/src/logicWrapper.js#L39

It only shares it for the first logic's nonMatchingAction$ and matchingAction$ streams. Next logic gets this merged stream etc etc: https://github.com/jeffbski/redux-logic/blob/v0.12.3/src/logicWrapper.js#L55

What I did in our fork was to forego (for now) the feature of validate being able to change the action for downstream middlewares/reducers. In my fork validate output only affects same logic process input action. This allowed me to make the actionSrc$ here (https://github.com/jeffbski/redux-logic/blob/v0.12.3/src/createLogicMiddleware.js#L50) be observeOn(asap), and unchain the applyLogic reducer. Now all actions will be handled async (with asap scheduler).

I suggest introducing a validateSync feature if you want redux-logic to be able to change actions for next middlewares without having the penalty of needing to chain all the logic validations together.

I'll try to clean up a bit and commit and share my fork so you can get some ideas for your own refactor.

[jeffbski]

Thanks for responding @tehnomaag with this great summary of your changes. I'll plan to also go through these and understand the details so we might be able to bring these into the this repo as well.

[shauntrennery]

@jeffbski I'm working on an application with 465 items of logic. 465 is seemingly too many for createLogicMiddleware as I'm not getting a "Maximum call stack size exceeded" error from within RXJS. Any ideas how I could approach this problem?

[jeffbski]

@shauntrennery I'm not sure I understood you. Are you saying that you are having an issue with 465 items but don't see any error? If so explain to me what happens. Did it work fine with 464 items, but now 465 is a problem?

@askot and @erkiesken I know you were experimenting with some tweaks to the code. Did this pan out for you and if so, can you point me to what you did and suggestions on what could be done?

Some of the ideas I had originally didn't pan out once I worked through the details.

[shauntrennery]

@jeffbski correct, 465 was the magic number for me. Calling createLogicMiddleware with an array of 465 items gave me a "Maximum call stack size exceeded".

I've opted for the following approach (splitting the logic array into two) as a workaround:

const middlewares = [ routerMiddleware(history), createLogicMiddleware(core, logicDependencies), createLogicMiddleware(app, logicDependencies), noticeMiddleware ]

[jeffbski]

ok, thanks. I'll look at the changes that the @erkiesken was experimenting with to see if we can find a good way to improve this.

[askot]

@erkiesken had a patch that changed how the logics are triggered and it seemed to work around the stack size issue. In principle it was similar to triggering each logic code with setTimeout, to get a new stack for every logic that runs.

It helped the stack issue but changed the order of how logics are actually called, a lot of redux-logic tests stopped working and also existing features were broken (it affected debouncing and cancelling logics if i remember correctly). So we couldn't use it without major changes in application code.

[jeffbski]

Thanks for clarifying @askot, that helps.

I'm directing a retreat the next few days, but after that (Monday) I will be able to dive into that and see if I can figure out a solution.

[pmualaba]

@ jeffbski I guess the next obvious question is: has there been any real world failures in a redux-logic app due to this (on Mobile Safari or otherwise)? or is are you just doing due dilligence to anticipate potential problems?

Yes, same problem over here: We chose redux-logic for its scalability and capability to organize bigger and more complex applications, but now we face a showstopper just on this very point of scalability. We also have the "Max call stack exceeded" issue because we reached the maximum number of logic middlewares... We are sadly forced to introduce a separate npm package (redux-observable) to distribute the middlewares over both middlewares, until this issue is fundamentally fixed?

We would love to stick to redux-logic only though, because it is conceptually by far the best middleware for modern redux applications :-)

[shauntrennery]

@pmualaba I seperated my logic array into two; core and app. The code below has resolved the issue for us.

const middlewares = [ routerMiddleware(history), createLogicMiddleware(core, logicDependencies), createLogicMiddleware(app, logicDependencies), noticeMiddleware ]

[jeffbski]

Thanks @shauntrennery , that is a good work around.

Yes @pmualaba it sounds like when one user got to 465 logics they started having problem. The exact number may vary by JS engine.

So it seems to me that this is only affecting a few users with large number of logics. The work around by splitting into multiple middleware instances will help go beyond that. However I am going to sit down this Saturday and take a hard look to see if can find any other ideas to help this situation.

[pmualaba]

@jeffbski Thanks already for taking a look at it. In case it can not be fixed fundamentally, it could be considered to handle @shauntrennery 's workaround internally (for example auto creating of a new array every 200 logics ?), so that the splitting of logic can be abstracted away ?

[jeffbski]

thats a good idea and will keep that in mind. thanks!

On Thu, Feb 22, 2018 at 8:57 AM pmualaba notifications@github.com wrote:

@jeffbski https://github.com/jeffbski Thanks already for taking a look at it. In case it can not be fixed fundamentally, it could be considered to handle @shauntrennery https://github.com/shauntrennery 's workaround internally (for example auto creating of a new array every 200 logics ?), so that the splitting of logic can be abstracted away ?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/jeffbski/redux-logic/issues/83#issuecomment-367706523, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAWOThz9nXjqNFpFdIu5Zzs9147-QTxks5tXYBRgaJpZM4QOIJT .

-- Jeff Barczewski Founder of CodeWinds http://codewinds.com/ Live and Online developer training

[pmualaba]

Hello Jeff, do we already have white smoke ;-) ?

[jeffbski]

@pmualaba not yet, but hopefully soon. I just wrapped up a big project, so I'll have time to dig into this.

[jeffbski]

My plan is to create test code that generates and connects any number of logic and runs them in the middleware.

I should be able to reproduce the situation and then try some things to improve it.

[SkReD]

Hello, everybody We have about 180 logics in one app and we are supporting app work on IE10+. So, on IE11 and IE10, sometimes, application crashing due to stack limit exceed. Any clues about how to, probably, reduce call stack and save current behaviour, would be very appreciate. Right now i'm trying to mess around with some promises that replace wrapped logics chaining, but with no luck

[jeffbski]

I'm in the process of seeing how we can improve the resource usage in the observable chain.

In the meantime, it seems like others have had luck by splitting their group of logic into two groups. This should reduce the depth of the call stack.

You can basically create two (or more) logic middleware instances and pass a portion into each.

const logicMiddlewareA = createLogicMiddleware(arrLogic1, deps);
const logicMiddlewareB = createLogicMiddleware(arrLogic2, deps);

const middleware = applyMiddleware(
  logicMiddlewareA,
  logicMiddlewareB
);

const enhancer = middleware; // could compose in dev tools too

export default function configureStore() {
  const store = createStore(rootReducer, enhancer);
  return store;
}

[pmualaba]

Has this issue been fixed in 0.13?

[jeffbski]

I'm still working on it. I'm trying to see if we can avoid having to split by optimizing something in the pipeline but the work around will still work in the meantime of splitting into separate instances.

[sanbornhilland]

Has there been any movement on this issue since May? I ended up here because I'm trying to understand what the performance impact is of having many logics registered in an application. I'm not having any failures per se (e.g. hanging) but I am seeing the same deep callstacks as other users and some long running tasks that are causing the FPS of our application to dip. It's unclear to me currently if this is due to the lib itself or some of the handlers that we have registered.

It had occurred to me as well that it would be useful to unregister logics like @jeffbski mentioned either when moving between application sections or if you know they won't be run anymore. If it's the case that simply having logics registered is causing a performance bottleneck then being able to remove logics once they aren't needed may be very useful. I'm curious what your current thoughts are around removing logics.

[jeffbski]

@sanbornhilland I think you are right, unregistering logics would reduce the working stack size. So besides splitting into different instances this should also help.

So basically if you want to do that, you would keep hold of the logicMiddleware instance (I usually tack it onto the store after creation) then call replaceLogic to swap out some middleware. mergeNewLogic to add new logic at runtime.

const logicMiddleware = createLogicMiddleware(arrLogic);
const store = createStore(...)
store.logicMiddleware = logicMiddleware; // save for later
...
// removing logic dynamically
const arrLogic2 = arrLogic.filter(x => x.name !== 'foo'); // everything but foo
store.logicMiddleware.replaceLogic(arrLogic2);

// adding later
store.logicMiddleware.mergeNewLogic(newLogicArr); 

I'll keep investigating the issue. I was hoping to see whether rxjs@6 would help, but it is a bit of work to finish the upgrade. I'll keep plugging away that and trying to find a good solution.

Hopefully the above will help in the meantime.

[jeffbski]

After updating to rxjs@6, I took some time to optimize the code for v2.0.0 so that it would be more efficient with stack usage. Basically if a particular logic isn't using some of the features (debounce, throttle, cancelType, latest, validate/transform, process, etc.) then I optimize out the extra code from the pipeline which in turn reduces the stack size. Obviously it will depend on your individual use cases, but I think in general everyone should see considerable improvements in the number of logic you can run in a single instance. In my stress tests, I was able to get up to 2.7x more logic running (540 vs 200). Give it a try and report back whether you are still having issues after this upgrade.

[henninghall]

Hi! We are still experiencing this issue after v2.0.0 and we have less than 100 logics. We have tried to split it up in several groups of logics which seems to fix it for now. But since we don't wan't this bug to hit us again later we probably need to replace redux-logic with another solution like redux-saga.

Btw, we are using redux-logic in React Native and have experienced this issue on both Android and iOS.

[jeffbski]

@henninghall That is sad to hear. If I could ask a few questions to better understand what you encountered. This is the first I am hearing of anyone having a problem since v2.

So in your case it is with React Native (ie mobile devices) where you are experiencing the issue, correct? Is it actual failure like stack overflow or performance degradation? Are these low end devices that encounter the problem?

Are the majority of your logics using validate/transform or just process hooks? How many logics do you have and what is the nature of them?

If we added a way to unload groups of logic would that be of use in your application? You'd be able to load groups of logic (when use enters a portion of the program) and unload them when no longer needed (when they leave that feature).

[henninghall]

Yes correct, it's on mobile devices, but we have experienced it on both low- and high-end devices, we have not really investigated where it happens most often.

The issues that occur are really weird ones, some kind of overflow I guess where functions simple aren't been running as they should, sometimes they do, sometimes they don't. It seems to happen in cases where we have some chains of logics which are dispatching actions. However, I don't believe we are using it in an extreme way. Some of them are using validate hooks, maybe half of them or less. We have around 70 logics I think and many of them are dispatching one or several actions inside of them. Many are using getState(). It seems to be a larger risk of running into these problems when logics are chained without any timeouts between them. Changing the order of logics seems to help in some cases.

At the moment it feels too risky to add more logics since we won't know where the problems will pop up next as you probably understand.

Hope you manage to reproduce this issue somehow and that it could be solved in the future. You are doing a really good job with this library, i really like it otherwise, keep it up!

[bverbist]

Hi! Same here that we have this issue in v2 (also after upgrade to v2.1.1).

For us, we experience the issue in IE11, where we regularly get the 'Out of stack space' error causing the app to crash. Strangely this error does not occur each time on IE11 ... after refreshing with the same code it can be that the app loads without this error.

The workaround for having multiple 'createLogicMiddleware' does have an effect, but is not future-proof as we probably will get the same error again after adding some additional logics (and hitting our magic number of logics).

In our case we have around 200 logics where maybe 30% of them (rough guess) have also a validate or a transform beside the process.

[bverbist]

Any progress on this?

And @jeffbski, can you elaborate on which logic functionality, besides 'a validate/transform' and 'latest=true' will increase the stack significantly?

• having a logic for multiple action types instead of just one type?
• having a logic that uses a regex for the action type?
• having a cancelType?
• having a throttle time?
• having a ...?
• furthermore I assume that setting options (like the validate-or-transform , or latest=false, or cancelType, ...) as undefined will not have a negative effect, is this correct? (or shouldn't those options be specified at all when calling the createLogic function to reduce the stack?).

[clintonmedbery]

Is this still be looked at? Has it been fixed? Getting: error in mw dispatch or next call, probably in middlware/reducer/render fn: RangeError: Maximum call stack size exceeded

at about 354 logic objects,

[jeffbski]

@bverbist I believe that all of these features when used contribute to the stack depth of the observable. I am removing the steps that are not enabled. So a good plan of attack is to just use the features that are necessary.

@clintonmedbery and others, my original plan to reduce the call stack didn't pan out.

I wonder if activating and deactivating logic would be a way to approach this. I'd imagine that in these large systems, that many of the logic are needed only for particular pages, so maybe if we improved the ability to activate and deactivate logic stacks would help us reduce the number of active logic running at a time (and thus the stack depth). Do you all think this would be a reasonable approach that could work with your applications?

[jeffbski]

I was thinking about the issue more last night and one common way to deal with stack size is to use asynchronicity, so I am thinking that maybe if I introduce a microtask in between groups of logics (maybe every 100 or so) then that might reduce the stack size. I have a test where I can create any number of logics to test this, so I can try that out and see if that allows us to remove our boundaries. I'm traveling today, but I'll see if I can test this out tomorrow.

[jeffbski]

Well introducing a scheduler (asap, async, or queue) into the pipeline (using observeOn) after each logic didn't resolve things as I had hoped. I'm not sure why.

However using the suggestion @shauntrennery had made a while back, splitting the array of logic and creating multiple middlewares seems to work well.

const  mw1 = createLogicMiddleware(arrLogic.slice(0, 300));
const mw2 = createLogicMiddleware(arrLogic.slice(300));
const store = createStore(reducer, undefined, applyMiddleware(mw1, mw2));

@pmualaba had suggested that redux-logic could automatically do this under the hood at a certain threshold so I am going to investigate implementing that. Thus if it works out it would automatically split the array into groups creating mw for each, and then returning an enclosing middleware which would invoke each group.

If anyone has tried splitting the logic and using multiple middlewares and had any issues, let me know, otherwise I'll work on implementing the auto splitting. In the meantime anyone running into this issue can manually split using code similar to above. Depending on how complex your logic is you may dial down the split size from 300 to 200 or whatever works for your use cases.

[pmualaba]

@jeffbski I think this would be a great approach: First it guarantees full backwards compatibiity, developers using redux-logic don't have to change a single line of code :-), just upgrade to the latest version of redux-logic and they 're set.

[Zakerius]

@jeffbski we also hit into this issue (on Firefox only) having around 250 logic functions. Chunking them by 200 helped out

[sara-greenblatt]

Hi @jeffbski We are facing this old issue now... We are getting 'out of stack space' errors from createMiddleware using IE8. The errors appear only when developer tools is open.

Chunking the middleware didn't help in our case...