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

High CPU and RAM usage on large files (due to show-whitespace) #2732

Closed
pdonias opened this issue Jan 23, 2020 · 7 comments · Fixed by #2737
Closed

High CPU and RAM usage on large files (due to show-whitespace) #2732

pdonias opened this issue Jan 23, 2020 · 7 comments · Fixed by #2737
Assignees
Labels

Comments

@pdonias
Copy link
Contributor

pdonias commented Jan 23, 2020

Opening the link below with Refined GitHub uses a lot of CPU and eats all the RAM very quickly.

Be ready to have your computer freeze before clicking the link https://github.com/teropa/to-sting/blob/master/index.js

Capture_2020-01-23_15:59:25

Refined GitHub 20.1.22
Firefox 72.0.1 (I couldn't reproduce the issue on Chrome)
Ubuntu 18.04.3 LTS

@pdonias pdonias added the bug label Jan 23, 2020
@fregante fregante changed the title High CPU and RAM usage High CPU and RAM usage on large files (due to show-whitespace) Jan 23, 2020
@fregante
Copy link
Member

fregante commented Jan 23, 2020

Confirmed, this is due to show-whitespace. The easy solution would be to detect the slow loop and make it stop for a moment, so the browser can paint a frame: https://github.com/sindresorhus/refined-github/pull/2087/files/c2c44b10e42ff2ccbbf94cf66fef5bf7ef9f6f2e..e3474a401792d5dab19b391208798fd8d5d3e77a

Also perhaps it would be good to find any bottlenecks in the code so it can be made faster in the first place.

@AleksandrHovhannisyan
Copy link

// eslint-disable-next-line no-await-in-loop

What if instead of using await we use old-fashioned promise handlers?

@fregante
Copy link
Member

I don't understand. How would you rewrite that loop with "promise handlers"?

@fregante fregante self-assigned this Jan 25, 2020
@AleksandrHovhannisyan
Copy link

What I probably meant is promise chaining. I don't know the terminology too well.

But await is gonna slow down that loop for sure.

@fregante
Copy link
Member

Await isn’t slower than promise chains. The lint rule is because await stops the loop and some users don’t realize that.

I found the problem of this issue: the feature is currently wrapping each individual character into its own DocumentFragment and then appending the whole thing back. The line in the middle is 200k characters long so you can see how that might take forever to finish.

I’m rewriting the feature to be a lot more efficient and I’ll probably also add a line length limit for these cases

@AleksandrHovhannisyan
Copy link

The lint rule is because await stops the loop and some users don’t realize that.

Right, I just figured stopping the loop is what was slowing the whole thing down.

Sorry for sticking my nose in this, though; I'm trying to get more involved in OSS but haven't found something that works for me.

@fregante
Copy link
Member

fregante commented Jan 26, 2020

I’m rewriting the feature to be a lot more efficient

Fixed without adding limits, but it needs a little more work: #2737

Right, I just figured stopping the loop is what was slowing the whole thing down.

You got it the other way: await setTimeout was there specifically to avoid freezing the browser.

Loops will freeze the browser until they finish. In this case, the loop was doing thousands of operations that probably took minutes in Firefox, without letting the browser breathe.

await setTimeout was there specifically to pause the loop so Firefox could do other operations, like interacting with mouse clicks. But this code only checked once per line. In this extreme example, one line was enough to freeze the browser for minutes so it never reached await

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

Successfully merging a pull request may close this issue.

3 participants