Bug 28943 – Logging fast* allocated heap usage

• Component: Web Template Framework
• Status: RESOLVED INVALID

[Balazs Kelemen]

I have implemented the possibility to logging all allocations and deallocations that goes through fast* allocation functions. This could help us profiling heap usage. The implementation is restricted to linux/qt at the time,
because I used glibc's backtrace function to programmatically create backtraces, and I extended only the qt build system with the option. You can build it as "build-webkit --qt --debug CONFIG+=memlogger" (building in release makes no sense since than the backtraces would not be informative). I implemented a dbus listener to create the output and a new tool -MemLogDumper- what can be used to trigger it. It has the advantage that you can create it by running any of the binaries (QtLauncher/jsc/DumpRenderTree). I am gladly welcome all suggestions about the stuff.

[Balazs Kelemen]

Created attachment 38982 [details]
proposed patch

[Balazs Kelemen]

I would note that I used stl containers because those supports custom allocators.

[Mark Rowe (bdash)]

The purpose of this code isn't particularly clear to me.  The comments say that it is only useful in debug builds, but debug builds of WebKit use the system memory allocator rather than TCMalloc.  This means that the usual memory profiling tools can easily be used to analyze memory usage.  That seems like a much more effective route to take.

I'd also recommend that you take a look at the WebKit coding style guidelines: <http://webkit.org/coding/coding-style.html>.  Of particular note are the lack of license headers on newly-added files, naming conventions, and brace placement.

The change to the fastMalloc function seems incorrect, as the early-return bypasses the "if (!result) CRASH;" when ENABLE(FAST_MALLOC_MATCH_VALIDATION) is true.

It's not clear to me what this has to do with DBus or why there is DBus-specific code in this patch.

[Balazs Kelemen]

(In reply to comment #3)
> The purpose of this code isn't particularly clear to me.  The comments say that
> it is only useful in debug builds, but debug builds of WebKit use the system
> memory allocator rather than TCMalloc.  This means that the usual memory
> profiling tools can easily be used to analyze memory usage.  That seems like a
> much more effective route to take.
> 

I think doing the profiling in the code directly (not with external tools)
has the advantage that we can better trust to the results. Btw, if you - the core developers - does not agree with that, then this patch is not valid.

> I'd also recommend that you take a look at the WebKit coding style guidelines:
> <http://webkit.org/coding/coding-style.html>.  Of particular note are the lack
> of license headers on newly-added files, naming conventions, and brace
> placement.

This patch would be an initial version for starting konversation about the stuff, so I did not deal with copyright or changelog.

> 
> The change to the fastMalloc function seems incorrect, as the early-return
> bypasses the "if (!result) CRASH;" when ENABLE(FAST_MALLOC_MATCH_VALIDATION) is
> true.

Rigth, it was a mistake.

> 
> It's not clear to me what this has to do with DBus or why there is
> DBus-specific code in this patch.

I used DBus to send a message to the MemLogger object what forces it to generating a memory dump.