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 for: magicline plugin leaks memory #2963

Closed
wants to merge 3 commits into from

Conversation

engineering-this
Copy link
Contributor

What is the purpose of this pull request?

Bug fix/ memory leaks

Does your PR contain necessary tests?

All patches which change the editor code must include tests. You can always read more
on PR testing,
how to set the testing environment and
how to create tests
in the official CKEditor documentation.

This PR contains

  • Unit tests
  • Manual tests - manual test is located in memory-test branch.

What changes did you make?

Magic line plugin exposes backdoor object property for debugging purposes in CKEDITOR.plugins.magicline, this object is replaced with each editor creation.
To fix memory leaks I've moved this property into editor._.magicLineBackdoor, so all references are removed along with editor instance.

For manual testing enable test on your current branch by:
git checkout memory-test tests/_assets/memorytest.html

Then read:
https://github.com/ckeditor/ckeditor-dev/blob/memory-test/tests/core/memory/memory.md

master branch
Screenshot 2019-03-19 at 12 09 34
After applying fix
Screenshot 2019-03-19 at 12 07 29

Snapshots

  1. After page load.
  2. After creating and destroying first editor.
  3. After creating and destroying second editor.
  4. After 100 next cycles of create-destroy editor.

Notes

  • First editor takes plenty of memory which is not released, but most of it is reused by next editors.
  • Second editor takes more memory than any next editors. I assume that Chrome after second creating of editor performs some optimizations which trade some memory for performance.

Partially solves #589

@engineering-this
Copy link
Contributor Author

Closing because commits from this branch are also present in #2969

@engineering-this engineering-this deleted the t/589-magicline branch March 21, 2019 12:32
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.

1 participant