Opened 7 years ago

Last modified 4 years ago

#5317 pending Task

Create tool to improve detection of non released memory

Reported by: Alfonso Martínez de Lizarrondo Owned by:
Priority: Normal Milestone:
Component: QA Version: 3.0
Keywords: Cc:

Description

Follow up of #4555

It's not the typical "memory leak" problem present in the browsers, but it's easy to store data in a way that it isn't released after one instance is destroyed in an AJAX way, and that means more and more memory use, so it's in the end a memory leak.

In the previous bug we have cleaned up the main codeline, but there are other parts that will have to be reviewed and instead of trying to dig into every plugin we must try to find an automated way that help us diagnose where are the problems.

Attachments (2)

5317.patch (21.7 KB) - added by Alfonso Martínez de Lizarrondo 7 years ago.
First version
5317_1.patch (21.7 KB) - added by Alfonso Martínez de Lizarrondo 7 years ago.
Little clean up

Download all attachments as: .zip

Change History (10)

Changed 7 years ago by Alfonso Martínez de Lizarrondo

Attachment: 5317.patch added

First version

comment:1 Changed 7 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review? added
Owner: set to Alfonso Martínez de Lizarrondo
Status: newassigned

Ok, this version I think that is able to catch all the leaks fixed in #4555.

A word of caution: disable firebug before using it or the script might give timeouts trying to store all the callstacks.

To test it disable any of the fixes of #4555 and check that it's properly reported here. Start the test, use the editor as you want and then click the end test to check if it has detected any problem.

Changed 7 years ago by Alfonso Martínez de Lizarrondo

Attachment: 5317_1.patch added

Little clean up

comment:2 Changed 7 years ago by Wiktor Walc

Milestone: CKEditor 3.6
Version: 3.0

The _dev folder is not included in release anyway, so having there an already created tool that can help us in tracing issues with memory sounds good.

comment:3 Changed 7 years ago by Garry Yao

Status: reviewreview_failed

The tool reports interference object changes like "CKEDITOR.ui._" and "CKEDITOR.config" which leads to actually no leak.

comment:4 Changed 6 years ago by Wiktor Walc

Milestone: CKEditor 3.6

It definitely should not block the 3.6 release, so I'm removing the milestone. Let's simply add the tool to the _dev folder once it works as expected

comment:5 Changed 4 years ago by Piotrek Koszuliński

Is this tool needed any more? Doesn't Chrome's memory profiling do the same job?

comment:6 Changed 4 years ago by Piotrek Koszuliński

Owner: Alfonso Martínez de Lizarrondo deleted
Status: review_failednew

comment:7 Changed 4 years ago by Piotrek Koszuliński

Status: newpending

comment:8 in reply to:  5 Changed 4 years ago by Alfonso Martínez de Lizarrondo

Replying to Reinmar:

Is this tool needed any more? Doesn't Chrome's memory profiling do the same job?

Things have changed a lot in these four years, at that time there was no memory profiling tools like these days.

Chrome's profiling would help to detect problems in the common code but not in the code paths that are executed by other browsers, so remember that any kind of tests should be done on each browser.

If something like this is implemented, I think that bugs like http://dev.ckeditor.com/ticket/10731 could have been detected earlier.

Of course, as CKEditor has changed so much, the current proposal is worthless and if anything is done is should be created almost from scratch.

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy