[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kfm-devel
Subject:    RE: Bug#1812: KFM memory leak: updated patch for khtmlw.
From:       David Faure <David.Faure () CRAMER ! CO ! UK>
Date:       1999-08-31 9:27:51
[Download RAW message or body]

Hi Bjarni.

Thanks *a lot* for this excellent work !
It looks very good.
I can't test it myself, though (still no net access at home, but I'm moving 
into my new house tomorrow ! ;) ).
I hope other kfm/khtmlw developers will be able to test it very quickly, 
and that they will apply it before the release.

--
David Faure
faure@kde.org - KDE developer
david@mandrakesoft.com - Mandrake
david.faure@cramer.co.uk - Cramer Systems



> -----Original Message-----
> From: Bjarni R. Einarsson [mailto:bre@netverjar.is]
> Sent: Monday, August 30, 1999 4:41 PM
> To: 1812@max.tat.physik.uni-tuebingen.de
> Cc: mjones@kde.org
> Subject: Bug#1812: KFM memory leak: updated patch for khtmlw.
> 
> 
> This is my khtmlw memory leak patch number 5.  It adds fixes for 
> GIF-related leaks, and just about solves my memory problems.  Thanks
> to Hrafnkell <he@kvintus.dk> for pointing out the movieCache leak.
> 
> I'm really happy now! :)
> 
> How I tested it:
> 
> ------------------------------------------------------------------
> I prepared the test, by executing following sequence once, to fill
> disk cache.
> 
> I'm testing a snapshot of KDE 1.1.2pre, from 20/08/99.
> 
> Test sequence:
> 
> 	Log in.
> 	Load www.google.com using ALT-F2, type "icons", hit 
> "I'm feeling lucky".
> 	Maximize window (no scrolling around).
> 	Hit "enjoy".
> 	This is the page we're measuring.
> 
> 1.1.2pre:
> 
>     Reload:      X size:      kfm size:    Leaked:
>     0            13112        10608        0
>     1            13800        11016        1096
>     2            14484        11252        920
>     3            15168        11344        776
>     4            15848        11488        824
> 
> 1.1.2pre+khtmlw.memleak.patch.4:
> 
>     Reload:      X size:      kfm size:    Leaked:
>     0            13000        10632        0
>     1            13464        11176        1008
>     2            13868        11328        556
>     3            14244        11464        512
>     4            14612        11624        528
> 
> 1.1.2pre+khtmlw.memleak.patch.5:
> 
>     Reload:      X size:      kfm size:    Leaked:
>     0            12860        7760         0
>     1            12880        10636        2896
>     2            12900        11752        1136
>     3            12916        12012        276
>     4            12900        12128        100
> 
> ------------------------------------------------------------------
> 
> Attached is an incremental patch, for people who've already applied
> patch 4, and a complete patch for anyone else.
> 
> -- 
> Bjarni R. Einarsson                           PGP: 02764305, B7A3AB89
>  bre@netverjar.is           -><-           http://www.mmedia.is/~bre/
> 
>              Don't let THEM immanentize the Eschaton.
> 
> 


["khtmlw.memleak.patch.4-5" (application/octet-stream)]
["khtmlw.memleak.patch.5" (application/octet-stream)]

Only in khtmlw: .libs
Only in khtmlw: debug.lo
Only in khtmlw: html.lo
Only in khtmlw: htmlchain.lo
Only in khtmlw: htmlclue.lo
Only in khtmlw: htmldata.lo
Only in khtmlw: htmlfont.lo
Only in khtmlw: htmlform.lo
Only in khtmlw: htmlframe.lo
Only in khtmlw: htmliter.lo
diff -u khtmlw.org/htmlobj.cpp khtmlw/htmlobj.cpp
--- khtmlw.org/htmlobj.cpp	Mon Aug 30 15:44:07 1999
+++ khtmlw/htmlobj.cpp	Mon Aug 30 15:56:45 1999
@@ -50,6 +50,12 @@
 QDict<HTMLCachedImage>* HTMLImage::pCache = 0L;
 int HTMLObject::objCount = 0;
 
+// Testing implied that this number was just fine - impact on
+// performance was negligable.  Conserving memory is good (tm).
+// It would be best if this could be tuned via. some advanced
+// configuration dialog.
+#define PCACHESIZE 73
+
 //-----------------------------------------------------------------------------
 
 HTMLObject::HTMLObject()
@@ -1239,12 +1245,19 @@
 
 //-----------------------------------------------------------------------------
 
+
 HTMLCachedImage::HTMLCachedImage( const char *_filename )
 {
     pixmap = 0;
     filename = _filename;
 }
 
+HTMLCachedImage::~HTMLCachedImage( )
+{
+    if ( pixmap ) delete pixmap;
+    pixmap = 0;
+}
+
 QPixmap* HTMLCachedImage::getPixmap()
 {
     if ( !pixmap )
@@ -1262,7 +1275,8 @@
 	// yet been initialized. Better be careful.
 	if( !pCache )
 	{
-		pCache = new QDict<HTMLCachedImage>( 503, true, false );
+		pCache = new QDict<HTMLCachedImage>( PCACHESIZE, true, false );
+	        pCache->setAutoDelete(TRUE);
 		return 0l;
 	}
 
@@ -1279,10 +1293,16 @@
 {
 	// Since this method is static, it is possible that pCache has not
 	// yet been initialized. Better be careful.
-	if( !pCache )
-		pCache = new QDict<HTMLCachedImage>( 503, true, false );
+	if( !pCache ) {
+		pCache = new QDict<HTMLCachedImage>( PCACHESIZE, true, false );
+	        pCache->setAutoDelete(TRUE);
+	}
 
-	pCache->insert( _filename, new HTMLCachedImage( _filename ) );
+	// Brute force garbage collection.  So we take a 
+	// performance hit - this should plug the leak!
+	if (pCache->count() > PCACHESIZE/2) pCache->clear();
+	
+	pCache->replace( _filename, new HTMLCachedImage( _filename ) );
 }
 
 HTMLImage::HTMLImage( KHTMLWidget *widget, const char *_filename,
@@ -1290,8 +1310,10 @@
 	int _max_width, int _width, int _height, int _percent, int bdr )
     : QObject(), HTMLObject()
 {
-    if ( pCache == 0 )
-	pCache = new QDict<HTMLCachedImage>( 503, true, false );;
+    if ( pCache == 0 ) {
+	pCache = new QDict<HTMLCachedImage>( PCACHESIZE, true, false );;
+        pCache->setAutoDelete(TRUE);
+    }
 
     pixmap = 0;
     movieCache = 0;
@@ -1371,7 +1393,7 @@
     }
 
     // Is the image available ?
-    if ( pixmap == 0 || pixmap->isNull() )
+    if ( !pixmap || pixmap->isNull() )
     {
 	if ( !predefinedWidth && !percent)
 	    width = 32;
@@ -1427,6 +1449,7 @@
     if ( p )
     {
       *pixmap = *p;
+      cached = true;
     }
     else
     {
@@ -1445,7 +1468,7 @@
   }
   
   // Is the image available ?
-  if ( pixmap == 0 || pixmap->isNull() )
+  if ( !pixmap || pixmap->isNull() )
   {
     if ( !predefinedWidth && !percent)
       width = 32;
@@ -1475,8 +1498,9 @@
   _buffer.readBlock( buffer, 3 );
   _buffer.close();
     
-  if ( strcmp( buffer, "GIF" ) == 0 )
+  if ( strcmp( buffer, "GIF89a" ) == 0 )
   {
+    if (movie) delete movie;
     movie = new QMovie( _buffer.buffer() );
     movie->connectUpdate( this, SLOT( movieUpdated( const QRect &) ) );
 #if QT_VERSION <= 141
@@ -1485,11 +1509,11 @@
   }
   else
   {
-    pixmap = new QPixmap();
+    if ( !pixmap ) pixmap = new QPixmap();
     pixmap->loadFromData( _buffer.buffer() );	    
     cached = false;
 
-    if ( pixmap == 0 || pixmap->isNull() )
+    if ( !pixmap || pixmap->isNull() )
       return false;
   }
   
@@ -1543,6 +1567,7 @@
       fclose( f );
       QByteArray arr;
       arr.assign( p, size );
+      if (movie) delete movie;
       movie = new QMovie( arr, 8192 );
       // End Workaround
 	// movie = new QMovie( _filename, 8192 );
@@ -1554,11 +1579,11 @@
     }
     else
     {
-	pixmap = new QPixmap();
+	if ( !pixmap ) pixmap = new QPixmap();
 	pixmap->load( _filename );	    
 	cached = false;
 
-	if ( pixmap == 0 || pixmap->isNull() )
+	if ( !pixmap || pixmap->isNull() )
 	    return;
 
 	init();
@@ -1596,7 +1621,7 @@
     if ( percent > 0 )
 	max_width = _max_width;
 
-    if ( pixmap == 0 || pixmap->isNull() )
+    if ( !pixmap || pixmap->isNull() )
 	return;
 
     if ( percent > 0 )
@@ -1637,12 +1662,12 @@
     {
 	// Wow... all this mess, just to get QMovies with transparent
 	// parts working... (Lars 30.11.98)
+	QPainter p;
 	rect = movie->getValidRect();
 	if( !movieCache )
 	{
 	    movieCache = new QPixmap(pixmap->width(), pixmap->height(),
 				     pixmap->depth());
-	    QPainter p;
 	    p.begin(movieCache);
 	    if( bgColor.isValid() )
 		p.fillRect( 0, 0, pixmap->width(), pixmap->height(), bgColor );
@@ -1653,8 +1678,8 @@
 				       y - ascent + _ty + border,
 				       pixmap->width(), pixmap->height(), &p);
 	    p.end();
+		oldRect = rect;
 	}
-	QPainter p;
 
 	QPixmap pm;
 	pm = movie->framePixmap();
@@ -1816,8 +1841,9 @@
     // if ( !imageURL.isEmpty() && !pixmap )
     // htmlWidget->cancelRequestFile( this );
 
-    if ( pixmap && !cached )
-	delete pixmap;
+    if ( pixmap ) delete pixmap;
+	if ( movieCache ) delete movieCache;
+
 #ifdef USE_QMOVIE
     if ( movie )
     {
Only in khtmlw: htmlobj.cpp~
diff -u khtmlw.org/htmlobj.h khtmlw/htmlobj.h
--- khtmlw.org/htmlobj.h	Mon Aug 30 15:44:07 1999
+++ khtmlw/htmlobj.h	Mon Aug 30 15:44:35 1999
@@ -571,7 +571,7 @@
 {
 public:
     HTMLCachedImage( const char * );
-    virtual ~HTMLCachedImage() { }
+    virtual ~HTMLCachedImage();
 
     QPixmap* getPixmap();
     const char *getFileName() { return filename.data(); }
@@ -669,9 +669,9 @@
     QString imageURL;
     
     KHTMLWidget *htmlWidget;
-    
-    static QDict<HTMLCachedImage>* pCache;
 
+    static QDict<HTMLCachedImage>* pCache;    
+   
     /*
      * Flag telling wether this image was found in the cache
      * If this flag is set, you may not delete the pixmap since the pixmap
Only in khtmlw: htmlobj.lo
diff -u khtmlw.org/htmltable.h khtmlw/htmltable.h
--- khtmlw.org/htmltable.h	Mon Aug 30 15:44:07 1999
+++ khtmlw/htmltable.h	Mon Aug 30 15:44:35 1999
@@ -79,7 +79,7 @@
 	{ HTMLClueV::print(_painter,_obj,_x,_y,_width,_height,_tx,_ty); }
 
     void link() { refcount++; }
-    void unlink() { if (--refcount == 0) delete this; }
+    void unlink() { if (--refcount <= 0) delete this; }
 
 protected:
     int rspan;
Only in khtmlw: htmltable.lo
Only in khtmlw: htmltoken.lo
Only in khtmlw: htmlview.lo
Only in khtmlw: jscript.lo
Only in khtmlw: libkhtmlw.la
Common subdirectories: khtmlw.org/test and khtmlw/test


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic