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

fix memory leak in syntax highlighting (causing gray webviews) #4459

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

sqs
Copy link
Member

@sqs sqs commented Jun 5, 2024

rehype-highlight, the syntax highlighting lib we use, has a memory leak bug (remarkjs/react-markdown#791). Downgrading to rehype-highlight@^6.0.0 as recommended fixes the memory leak.

This was causing an issue where the Cody chat webview turned gray due to an out-of-memory situation.

Before (~1.8GB memory usage, unbounded)

image

After (328MB memory usage, bounded)

image

Test plan

CI, and test a simple question returning code snippets with the Chrome memory inspector open

@sqs sqs requested a review from a team June 5, 2024 02:02
Copy link
Member

@valerybugakov valerybugakov left a comment

Choose a reason for hiding this comment

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

Wow, what an awesome find!

rehype-highlight, the syntax highlighting lib we use, has a memory leak bug (remarkjs/react-markdown#791). Downgrading to rehype-highlight@^6.0.0 as recommended fixes the memory leak.

This was causing an issue where the Cody chat webview turned gray due to an out-of-memory situation.

- Fixes #4431
- Fixes https://linear.app/sourcegraph/issue/CODY-2062/gray-screen-in-chat
- Fixes https://linear.app/sourcegraph/issue/CODY-2017/reports-of-chat-ui-lagginess-on-long-messages
- Fixes https://community.sourcegraph.com/t/a-condensed-report-on-key-issues-affecting-cody/439#h-8-performance-issues-with-vscode-extension-18
@sqs sqs enabled auto-merge (squash) June 5, 2024 02:11
@sqs sqs merged commit 5e921f0 into main Jun 5, 2024
19 checks passed
@sqs sqs deleted the sqs/chat-perf branch June 5, 2024 02:16
@Lebofly
Copy link

Lebofly commented Jun 5, 2024

Legend

@abeatrix
Copy link
Contributor

abeatrix commented Jun 5, 2024

Thank you!!

@NebulusIO
Copy link

ty so much, I was doing reload window so many times. quick fix!

umpox pushed a commit that referenced this pull request Aug 23, 2024
There has been a (long ongoing) issue with `rehype-highlight` /
`highight-js` causing OOM issues on `>v6.0.0`. This reverts a recent
version bump that might have [re-introduced this
issue](#4459) in Cody chat
causing grey-screens and crashes.

> Important: I wasn't able to re-produce the issue myself for some
reason. So this is a bit of a "shot-in-the-dark" fix. Hopefully we can
have those who reported the issue validate if it actually worked.
Lowering the version shouldn't hurt any other changes though as it was
merely done to reduce the bundle size somewhat.

## Test plan
CI
umpox pushed a commit that referenced this pull request Aug 23, 2024
There has been a (long ongoing) issue with `rehype-highlight` /
`highight-js` causing OOM issues on `>v6.0.0`. This reverts a recent
version bump that might have [re-introduced this
issue](#4459) in Cody chat
causing grey-screens and crashes.

> Important: I wasn't able to re-produce the issue myself for some
reason. So this is a bit of a "shot-in-the-dark" fix. Hopefully we can
have those who reported the issue validate if it actually worked.
Lowering the version shouldn't hurt any other changes though as it was
merely done to reduce the bundle size somewhat.

## Test plan
CI
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.

bug: Grey chat window is back with vengeance
5 participants