Slow unloading / Repainting Issue

[guest]

As of r532 xinha takes several seconds to "unload" (tested using both IE and FF) causing a very appreciable delay in closing a window, submitting xinha text or anything that causes xinha to unload.

As of r533 whenever a submit button is pressed or xinha unloads xinha's textarea repaints itself several times. In IE I've only seen the select boxes move a single pixel several times while in FF it's the textarea that will move and redraw itself 6 ro 7 times.

[mokhet]

Yes, the repainting issue is coming first from the changeset:533 and the repainting is a little less apparent in changeset:536 but still active. I'm guilty here, the problem is coming from me trying to fix every leaks we can find. See ticket:30 for more explanations.

However, this problems needs 2 things before being correctly fixed :

1) Introducing a editor.dispose() method instead of the ugly HTMLArea.freeLater(), so we can select accuratly every elements to correctly unset to free the memory. Opposate to freeLater() which is sometimes taking every property and nullyifing them.

2) To be able to introduce a disposer, we need to get rid of ALL the anonymous function used in event listeners, to let us correctly remove some events when we need, and not just waiting for the garbageCollector to be launched. See ticket:658 for more explanation and my last proposition which can fix this point.

Anyway, if the repainting issue is a big problem, you can (while waiting for a real fix) take back the HTMLArea.free() method from changeset:532

[guest]

mokhet - I had been following the discussion and work on ticket 30, but thought it best to put in a ticket so it's recorded and so people know it's a known "bug".

[gogo]

mokhet: how do you propose that editor.dispose() works?

I don't really see what's wrong with freeLater to be honest, you give it an object (and optionally a property) and it frees it, later. Actually not quite correct, it only frees if you are running IE; it's basically a helper for IE to get it's garbage collection right. To be fair I havn't been following the latest developments in #30 so perhaps I'm missing something.

Of course it's not perfect, the best way would be to nullify as soon as it's not required, but that would be make it a lot more tricky to get it right. As I said in #658, I'm lazy, I always want the code to allow me the least work and thinking when I'm using it (of course, garbage collection is supposed to be doing this anyway, sigh, I hate Microsoft), or to put it another way, the code should automagically do as much work for me as it can, so I have more time for TV watching, reading slashdot, general farting about, that kind of thing.

[mokhet]

I'll try to explain but as always, sometimes my english is just not good enough, so please ask if something is confusing. I know i can be confusing :D

Concerning editor.dispose(), i'm working on a patch relying on the update i propose in #658, i'll attach it as soon as it is working correctly and obviously as soon as my life outside Xinha let me finish it.

Let me try to give you the basics lines on how i have started :

* HTMLArea.prototype._onUnGenerate

• private method
• used to call the plugins disposer if they have one

* HTMLArea.prototype.dispose

• public method
• used to dispose 1 single editor instance
• so it can be destroyed and reinit without the need to reload the whole page

* HTMLArea.dispose

• private method
• started onUnload to dispose of all remaining editors with the events flusher and IEGarbageCollector

The main concern, at least for me, is to be able to ungenerate an editor when i want with a simple click on my page. The second concern, still for me, is to be able to accuratly nullifying what needs to be removed from memory without once again having to wait for the reload of the whole page. And my third "point" would be that's i hate the HTMLArea.freeLater method, but that's very subjective.

The HTMLArea.freeLater is not used only for IE. Firefox has is own leaking issues too, just need to use the leakMonitor to be convinced. They working to fix all the leaks, however they are still here. http://dbaron.org/mozilla/leak-monitor/

The problem with freeLater, is exactly that : it is freeing "later". Since the web is in a total revolution (DOM manipulation, AJAX, etc.), later is too late. It needs a way to freeNow(), and that's where is coming the HTMLArea.prototype.dispose

I think with this update you should have more time to watch TV :)

Would it be possible to create a branch where i could commit the whole updates for #658, #30 and #825, or perhaps i can do it myself ? ( i definitly should RTFM of SVN :)

If i can create a branch myself, do you allow me to do it ? thanks gogo

[gogo]

Hmm, I see where you're going I think.

It sounds like you have a specific need to "unload" an editor without unloading the page (by navigating away/reloading), I was thinking more along the general lines. In that case, I agree freeing later is too late (particularly if you're writing something that will never, or seldom actually unload the page, like gmail for instance), you want it to free now.

I agree with the FFox leaks, however the difference is, that they can fix thier problem, I don't think Microsoft can ;-)

Of course you can create a branch. In SVN, a branch is just a ("cheap") copy...

  svn cp http://svn.xinha.python-hosting.com/trunk http://svn.xinha.python-hosting.com/branches/mokhet

if you have a workign copy with changes already in it, I think svn switch will create a branch out of it, if I remember correctly. The SVN book (online) tells all ;-)

[mokhet]

Yes, you got it, i need a lot of different instances of xinha on a page that is almost never unloaded. And all thoses tickets #30, #658 and #825 are kinda related.

I have made the branch from changeset:537 and not from my real working copy because it has others kind of changes. Anyway, it's created and i'll stick to ticket #658 for any more comments on this branch, and stop mixing comments on different tickets for same "problem".

To close this ticket, and waiting for a better approach (imo), this patch is supposed to resolve the issue, returning back to the HTMLArea.free() from changeset 532. But i suspect it to reopen some leaks closed in #30 Anyway, should it be commited to trunk ?

Index: htmlarea.js
===================================================================
--- htmlarea.js	(revision 538)
+++ htmlarea.js	(working copy)
@@ -6711,35 +6711,25 @@
 
 /**
  * Release memory properties from object
- * @param {object} O (Object)   The object to free memory
- * @param (string} P (Property) The property to release (optional)
+ * @param {object} object The object to free memory
+ * @param (string} prop   The property to release (optional)
  * @private
  */
-HTMLArea.free = function(O, P)
+HTMLArea.free = function(obj, prop)
 {
-  if ( O && !P )
+  if ( obj && !prop )
   {
-    for ( var p in O )
+    for ( var p in obj )
     {
-      HTMLArea.free(O, p);
+      HTMLArea.free(obj, p);
     }
   }
-  else if ( O )
+  else if ( obj )
   {
-    // is the try...catch really required here ?
     try
     {
-      // if it is the innerHTML property, just do nothing
-      if ( P !== 'innerHTML' )
-      {
-        // need to unhide TD cells before delete them or they will leak in IE
-        if ( O[P] && O[P].tagName && O[P].tagName.toLowerCase() == 'td' )
-        {
-          O[P].style.display = '';
-        }
-        O[P] = null;
-      }
-    } catch(x) {}
+      obj[prop] = null;
+    } catch(ex) {}
   }
 };
 

[gogo]

Yes, commit it to trunk if it solves this ticket, a leaky Xinha is better than a slow and repainting one ;-)

[mokhet]

Applied patch to trunk in changeset:544 , and so i'm closing this ticket now.