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

Investigate the size of memory leaks #10749

Closed
Reinmar opened this issue Oct 25, 2021 · 3 comments · Fixed by #10803
Closed

Investigate the size of memory leaks #10749

Reinmar opened this issue Oct 25, 2021 · 3 comments · Fixed by #10803
Assignees
Labels
squad:core Issue to be handled by the Core team. type:task This issue reports a chore (non-production change) and other types of "todos".

Comments

@Reinmar
Copy link
Member

Reinmar commented Oct 25, 2021

To unblock #5954 we need to figure out what issues we have currently.

  • Confirmation (that the bug occurs without the Inspector, make sure to call garbage collector manually before the last snapshot)
  • Analysis of memory profiler logs to figure out what kind of retainer chains are there
@Reinmar Reinmar added type:task This issue reports a chore (non-production change) and other types of "todos". squad:core Issue to be handled by the Core team. labels Oct 25, 2021
@Reinmar
Copy link
Member Author

Reinmar commented Oct 25, 2021

Check out Olek's comment: #5954 (comment).

@oleq
Copy link
Member

oleq commented Nov 4, 2021

TL;DR: Turns out, the editor does not leak much in successive init&destroy tests (core features). It's the MathType+ChemType that causes issues.

I ran a series of 10 init()/destroy() of the editor, each destroy followed by GC on demand (then memory snapshot). There are a few things retained between each run, the memory usage rises and drops. However, when I enabled the MathType plugin I noticed that the memory usage grew tenfold

Screenshot 2021-11-04 at 09 50 46

A quick look at the snapshot revealed that the MathType integration retains many editing view elements (despite no math type widgets loaded in the data). Looks like this has something to do with the way the integration attaches event listeners for clicking, etc.; this needs further investigation

@oleq
Copy link
Member

oleq commented Nov 4, 2021

FYI: There is a PR related to the research (new manual test + safeguarded some bits of UI) #10803.

@oleq oleq reopened this Nov 4, 2021
@oleq oleq added this to the iteration 49 milestone Nov 4, 2021
Reinmar added a commit that referenced this issue Nov 5, 2021
Internal: Made sure `KeystrokeHandler` and `FocusTracker` instances are destroyed across the UI in the project to avoid memory leaks. Closes #10749. Closes #6561.

Tests (ckeditor5): Added a semi-automated manual test to investigate memory leaks in the editor (see #10749).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
squad:core Issue to be handled by the Core team. type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
3 participants