[webkit-dev] WebKit memory management?

[Maciej Stachowiak]

On Jun 3, 2008, at 10:58 PM, Paul Pedriana wrote:

> Thanks for the response. I'm sorry, and perhaps I misunderstand, but  
> I believe your statement about inline operator new is incorrect.  
> Unless I misunderstand you, what you say is not supported by any  
> existing compiler nor is it supported by the C++ language standard.  
> In summary, the 'inline' keyword does not negate or obviate the One  
> Definition Rule. You can demonstrate the problem with the code  
> below. Feel free to correct any misunderstanding that I may have of  
> your explanation.

This happens to work as intended on Mac OS X because  
WTF_PRIVATE_INLINE expands to:

__private_extern__ inline __attribute__((always_inline))

This prevents an externally visible definition of global operator new  
and delete from being seen.

I agree this is technically wrong and I suspect it may cause problems  
on, for example, Linux platforms. I think the Qt port has turned off  
FastMalloc for this reason.

I can think of two possible solutions:

1) Instead of overloading the global operator new and delete, have a  
FastAllocated base class that overloads the class operator new and  
delete, and make every class in WebCore and JavaScriptCore inherit  
from it; for non-class types, avoid using new, delete, new[] or  
delete[] (perhaps template functions could be provided). The downside  
is that I can't think of an easy way to then flag mistaken use of new/ 
delete on primitive types, or forgetting to inherit from the  
FastAllocated base class. Then again, we don't try to flag mistaken  
use of malloc() instead of fastMalloc() and that has been ok.

2) Require allocation to happen in some way other than "new" and  
"delete", for instance always with template functions. Then perhaps we  
could use #defines to make any actual use of "new" and "delete" an  
error.

Either of these would be a large change to the source, especially #2  
(#1 only needs to affect classes with no other subclass and the few  
places we use new[] on non-class types to make arrays).

Perhaps someone else has a more clever idea.

Regards,
Maciej

>
>
> I do not mean to criticize WebKit. We think it is a great thing  
> which in general is surprisingly well coded. We would love to work  
> with any resolution which has the desired effect in the way of  
> memory management.
>
> The error you get:
> SomeLib.lib: error LNK2005: "void * __cdecl operator new(unsigned  
> int)" (??2 at YAPAXI@Z) already defined in main.obj
> SomeLib.lib: error LNK2005: "void __cdecl operator delete(void  
> *)" (??3 at YAXPAX@Z) already defined in main.obj
> Source code:
> // Main.cpp
> #include <stdlib.h>
> extern void DoSomething();
>
> void* operator new(size_t s)    { return malloc(s); }
> void  operator delete(void* p)  { free(p); }
> void* operator new[](size_t s)  { return malloc(s); }
> void  operator delete[](void* p){ free(p); }
>
> int main(int, char*[]) {
>     void* p = malloc(10);
>     free(p);
>     DoSomething();
>     return 0;
> }
>
> // SomeLib.cpp - compiled in a separate lib
> #include <stdlib.h>
>
> inline void* operator new(size_t s)     { return malloc(s); }
> inline void  operator delete(void* p)   { free(p); }
> inline void* operator new[](size_t s)   { return malloc(s); }
> inline void  operator delete[](void* p) { free(p); }
>
> void DoSomething(){
>     void* p = malloc(10);
>     free(p);
> }
> Thanks.
>
>
>
>
>> On 03/06/2008, at 21:13, Paul Pedriana wrote:
>>
>>> Thanks for the info. IMHO, tcmalloc is not appropriate for console,
>>> embedded, and mobile platforms. It trades space for speed, and  
>>> that's
>>> the opposite of what you want outside the desktop. This is why the  
>>> Nokia
>>> S60 people replaced tcmalloc, for example.
>>
>> As far as I can tell, Nokia's S60 port predates the adoption of  
>> tcmalloc by WebKit.  The code in their latest svn.webkit.org source  
>> tree contains a variant of dlmalloc that was used up until Safari  
>> 2.0, though I have not checked to see whether it is compiled in to  
>> their build.  That said, it is obvious that the space vs. speed  
>> tradeoffs differ between devices, and that flexibility in the  
>> memory allocator used is desirable.
>>
>>> Unfortunately, overriding operator new and delete does not do the  
>>> right
>>> thing. These operators are application-global functions and when you
>>> redirect them for one library you are thus redirecting them for the
>>> entire rest of the app. Needless to say, that is a bad thing. In  
>>> console
>>> and embedded development, as I note in the aforementioned paper,  
>>> it is
>>> typically verboten for a library to use operator new/delete.
>>
>> On the platforms with which I am familiar, the implementation that  
>> I linked to has no effect outside of the library in which it is  
>> defined.  I've not worked with consoles or embedded devices so the  
>> toolchain and environment may differ there, but I would be a little  
>> surprised to see an inline function that is implemented in a header  
>> become visible to an object file that did not include the header.
>>
>>
>>> Neither will you see professional commercial software do this.
>>
>>
>>> It's also a problem to have any calls to system malloc at all,  
>>> because often on
>>> these platforms there is little or no memory available, as the
>>> application has taken it all to distribute to private heaps as per  
>>> their
>>> budget.
>>
>> The direct calls are few and far between.  They can easily be  
>> evaluated to determine which, if any, have a legitimate need to  
>> call the system allocator and the remainder updated to use  
>> "fastMalloc" / "fastFree".  I'd gladly review a patch that moves in  
>> this direction.
>>
>>> One simple and effective way to solve this problem is to provide a
>>> memory config header file which defines macros or templates which
>>> replace new/delete, malloc/free. Instead of calling global new,  
>>> WC_NEW
>>> (e.g.) is called instead.
>>
>> How does this differ from FastMalloc.h and "fastMalloc" /  
>> "fastFree" that I described in my previous email, other than  
>> addressing the perceived problem with "operator new" / "operator  
>> delete"?
>>
>>
>>> This is how commercial-quality software is done
>>
>>
>> Kind regards,
>>
>> Mark Rowe
>>
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.webkit.org/pipermail/webkit-dev/attachments/20080604/d57472cc/attachment.htm