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

Loop breaking doesn't work if there's a console.log() in the loop body #1354

Closed
catrope opened this issue Feb 22, 2018 · 2 comments · Fixed by #1582
Closed

Loop breaking doesn't work if there's a console.log() in the loop body #1354

catrope opened this issue Feb 22, 2018 · 2 comments · Fixed by #1582
Assignees
Labels

Comments

@catrope
Copy link
Contributor

catrope commented Feb 22, 2018

Start with: https://popcode.org/?snapshot=05697db4-3317-43aa-b8c7-eb829c9e4992 . Popcode notices pretty quickly (within about a second) that there's an infinite loop, stops executing, and tells you about it. If you change i-- to something else that's syntactically valid (e.g. i or i -= 1), it'll respond reasonably quickly as well.

Now uncomment the console.log(i); statement. Depending on how quickly after that you try to type, either the tab will hang for a few seconds and then your typed characters will come through, or if you wait long enough the tab will hang unrecoverably and you have to close it.


Further information: This isn’t strictly a loop-breaking issue but rather one with performance of console.log() when it’s invoked a large number of times in quick succession. For instance, a finite loop that issues a console.log() 1,000,000 times will freeze Popcode. Initial research indicates that the problem is the large number of CONSOLE_LOG_PRODUCED actions being dispatched, yielding enough spins through React’s update cycle to overwhelm the browser.

I believe the simple answer here is just to debounce the log messages and dispatch them in batches. This could either be done at the Redux level or, probably more simply, in the component container.

@outoftime outoftime added the bug label Mar 3, 2018
@outoftime outoftime self-assigned this Mar 3, 2018
@wylieconlon
Copy link
Contributor

wylieconlon commented Aug 21, 2018

This is an issue with any slow-running or blocking code such as:

for (var i = 0; i < 7; i--) {
    document.body.innerText = document.body.innerText + ' ' + i;
}

but this code is not a problem:

for (var i = 0; i < 7; i--) {
    document.body.innerText = i;
}

So it's possible that the loop-breaker library needs to be more focused on execution time and less on infinite loops?

@wylieconlon
Copy link
Contributor

According to @outoftime he thinks that the solution is a combination of two things, specifically changing the console.log behavior in popcode to be faster with something like:

iirc the solution we settled on was a dispatch debouncer that buffered actions and then batch-dispatched them using something like https://github.com/tshelburne/redux-batched-actions
GitHub
although probably no need to get particularly fancy with the dispatch, could also just maintain the buffer ourselves in the saga and change the action’s payload structure to take a collection of log entries

as well as integrating a faster timeout for breaking loops with code similar to: popcodeorg/loop-breaker#9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants