[webkit-dev] FontCache refactoring proposal

2008-12-11 Thread Julien Chaffraix
Hi everyone,

while working on memory leaks inside WebCore, the Pleyo team has found
that the FontCache was responsible for a few of them. In order to
solve those leaks and prevent future ones, we have done a refactoring
of the FontCache and its internal working (mainly making the
SimpleFontData Refcounted and change several sites inside WebCore to
hold RefPtr). The modification are done and they are too big for
integration right now so we would like to split them and contribute
them back to WebKit. You will find below the different parts that we
have worked on (this will follow more or less the order in which they
will be contributed back):

- initial clean-up
* share some methods that are the same in all implementations
* add a 'platform' suffix for those that should be implemented per platform
* avoid using FontPlatformData ouside the few font files inside platform
* make FontCache a singleton and remove all the current static methods

- Add leaks probe using RefCountedLeakCounter to track our progress as
well as see where the leaks occurs.

- Make the FontCache mechanism use smart pointers
* have SimpleFontData derive from RefCounted
* use internally RefPtr
* use RefPtr for external reference to SimpleFontData all over WebCore

- FontCache HashMap refactoring: currently some HashMaps return
FontPlatformData internally and there is a mapping between those and
SimpleFontData so it should be better to use the SimpleFontData
instead.

Those items are open to discussion and we will try to address the
comments before starting and as the work moves forward.

Best regards,
Julien
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] FontCache refactoring proposal

2008-12-11 Thread Dan Bernstein

Hi Julien,

On Dec 11, 2008, at 2:14 AM, Julien Chaffraix wrote:


Hi everyone,

while working on memory leaks inside WebCore, the Pleyo team has found
that the FontCache was responsible for a few of them.


It would be good to have a bug filed about each leak.


In order to
solve those leaks and prevent future ones, we have done a refactoring
of the FontCache and its internal working (mainly making the
SimpleFontData Refcounted and change several sites inside WebCore to
hold RefPtr). The modification are done and they are too big for
integration right now so we would like to split them and contribute
them back to WebKit. You will find below the different parts that we
have worked on (this will follow more or less the order in which they
will be contributed back):

- initial clean-up
   * share some methods that are the same in all implementations
   * add a 'platform' suffix for those that should be implemented  
per platform
   * avoid using FontPlatformData ouside the few font files inside  
platform
   * make FontCache a singleton and remove all the current static  
methods


- Add leaks probe using RefCountedLeakCounter to track our progress as
well as see where the leaks occurs.

- Make the FontCache mechanism use smart pointers
   * have SimpleFontData derive from RefCounted
   * use internally RefPtr
   * use RefPtr for external reference to SimpleFontData all over  
WebCore


- FontCache HashMap refactoring: currently some HashMaps return
FontPlatformData internally and there is a mapping between those and
SimpleFontData so it should be better to use the SimpleFontData
instead.


How do you plan to maintain the constructor
Font::Font(const FontPlatformData fontData, bool isPrinterFont)
without a mapping from FontPlatformData to FontData?

—Dan___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] FontCache refactoring proposal

2008-12-11 Thread Julien Chaffraix
Hi Dan,

On Thu, Dec 11, 2008 at 6:40 PM, Dan Bernstein m...@apple.com wrote:
 Hi Julien,
 On Dec 11, 2008, at 2:14 AM, Julien Chaffraix wrote:

 Hi everyone,

 while working on memory leaks inside WebCore, the Pleyo team has found
 that the FontCache was responsible for a few of them.

 It would be good to have a bug filed about each leak.


Sure. I was not the one to look for leaks but I will get the
information and file the bugs.

 In order to
 solve those leaks and prevent future ones, we have done a refactoring
 of the FontCache and its internal working (mainly making the
 SimpleFontData Refcounted and change several sites inside WebCore to
 hold RefPtr). The modification are done and they are too big for
 integration right now so we would like to split them and contribute
 them back to WebKit. You will find below the different parts that we
 have worked on (this will follow more or less the order in which they
 will be contributed back):

 - initial clean-up
* share some methods that are the same in all implementations
* add a 'platform' suffix for those that should be implemented per
 platform
* avoid using FontPlatformData ouside the few font files inside platform
* make FontCache a singleton and remove all the current static methods

 - Add leaks probe using RefCountedLeakCounter to track our progress as
 well as see where the leaks occurs.

 - Make the FontCache mechanism use smart pointers
* have SimpleFontData derive from RefCounted
* use internally RefPtr
* use RefPtr for external reference to SimpleFontData all over WebCore

 - FontCache HashMap refactoring: currently some HashMaps return
 FontPlatformData internally and there is a mapping between those and
 SimpleFontData so it should be better to use the SimpleFontData
 instead.

 How do you plan to maintain the constructor
 Font::Font(const FontPlatformData fontData, bool isPrinterFont)
 without a mapping from FontPlatformData to FontData?

I may have not explained well. A mapping between FontPlatformData and
FontData is required (as you were pointing out at least this
constructor requires this). However once you have made most of the
methods in FontCache return FontData, it seems relevant to also switch
to using FontData internally (and wrap the FontPlatformData in a
FontData if necessary). This switch is for FontPlatformDataCache which
currently maps a custom key to a FontPlatformData (but changing it
means to change all the methods that rely on the current mapping).

Best regards,
Julien
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev