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

Reuse markdown instance for preferences widget #12790

Merged
merged 1 commit into from
Aug 2, 2023

Conversation

msujew
Copy link
Member

@msujew msujew commented Aug 1, 2023

What it does

Closes #12789

The current implementation creates a new instance of markdownit for every preference leaf node. This is quite costly in frontend memory. The change moves the markdownit instance to a dedicated service that is shared across all nodes in a preference model.

In my tests, this reduces the memory consumption of the preference widget by ~165MB:

Before (267MB):

image

After (102MB):

image

How to test

  1. Open the preference widget.
  2. Assert that the memory consumption has been reduced.

Review checklist

Reminder for reviewers

@msujew msujew added preferences issues related to preferences performance issues related to performance labels Aug 1, 2023
@kittaakos
Copy link
Contributor

Hello, thanks for the fix. Here, the idea was to reuse the markdown rendering logic from monaco and remove markdown-it. Was it rejected?

@msujew
Copy link
Member Author

msujew commented Aug 1, 2023

@kittaakos I'm not sure how reusable the MarkdownRenderer or MonacoMarkdownRenderer actually are for the purposes of rendering preferences. Especially since we need a renderInline (i.e. no <p> HTML element wrapper) method for the preferences.

@kittaakos
Copy link
Contributor

I'm not sure how reusable the MarkdownRenderer or MonacoMarkdownRenderer actually are for the purposes of rendering preferences.

All clear, thank you!

this reduces the memory consumption of the preference widget by ~165MB:

Awesome contribution

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

LGTM 👍

  • confirmed the rendering is correct
  • confirmed the memory is significantly improved

@msujew msujew merged commit e4b9075 into master Aug 2, 2023
6 checks passed
@msujew msujew deleted the msujew/reuse-markdownit-instance branch August 2, 2023 11:13
@github-actions github-actions bot added this to the 1.41.0 milestone Aug 2, 2023
@TotooriaHyperion
Copy link

Wow, finding my IDE based on 1.39.0 consumes too much memory.
Upgrading to 1.45.1 reduces welcome page memory consumption from 15xMB->7xMB, more than half of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance issues related to performance preferences issues related to preferences
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

preference editor take up a lot of memory
4 participants