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

List:       kfm-devel
Subject:    Re: FW: Bug#1812: KFM memory leak: updated patch for khtmlw.
From:       "Bjarni R. Einarsson" <bre () netverjar ! is>
Date:       1999-08-31 10:51:16
[Download RAW message or body]

On Tue, Aug 31, 1999 at 11:44:58AM +0100, David Faure wrote:
> Some more on the subject.
> 
> A main khtmlw developer (Waldo) told me this about the cache :
> 
> > > > The cache in khtmlw is for icons only. It is intended that this cache
> > > > is never deleted/cleared. A normal web-page does not load any
> > > > icons and does not use this cache at all.

This may be true - but if you look at the patch test results
you'll see that my first patch, which didn't address the
movieCache problem at all, did improve memory usage measurably,
with no noticable performance impact.

IMHO that justifies it. :-)

Any buffer that can grow indefinately, without ever being
deleted/cleared is IMHO a misfeature.  If they feel I'm too
aggressive they should just grow the PCACHESIZE to a larger
number which they feel is more appropriate - then if the number
of icons really *does* increase wildly my patch will kick in and
do it's thing, without interfering with normal operation.

Defensive programming is cool.

> But, on the other hand, another one said :
> 
> > I looked a little more carefully.  The magic line is
> 
> > + if ( movieCache ) delete movieCache;
> 
> > I clicked back and forward between two pages with
> > animated gifs.  Without the above line the X
> > server grew quite quickly.
> > We can probably ignore the rest of the patch.
> 
> As we are very close to the release, we don't want to apply a huge
> patch if only a small part of it is really necessary.

But really, the patch isn't all that big, and it should be
trivial to verify that it isn't doing anything bad - especially
for people familiar with the code.

Although my main goal was to decrease memory usage I, was
excercising pretty darn boring "defensive programming" tactics
throughout - and correcting some minor errors I noticed as I
went along (e.g. the GIF/GIF89a fix).  Therefore I think the
whole thing should at least be reviewed, line by line, to see
what I'm doing.  I didn't make any changes that I wasn't sure
wouldn't help (of course I could be wrong, but that goes without
saying).

But, stability is king and it's your call. :-)

> What do you think about reverting to the current version of the code,
> then testing adding only the line above, to see if it fixes the problem
> by itself ? (I wish I could test this myself but I can't).

I'll test it, and send you the results later today.

-- 
Bjarni R. Einarsson                           PGP: 02764305, B7A3AB89
 bre@netverjar.is           -><-           http://www.mmedia.is/~bre/

            Study Demonology With An Enemy This Sunday.

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