Bug 28943 - Logging fast* allocated heap usage
Summary: Logging fast* allocated heap usage
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-09-03 05:29 PDT by Balazs Kelemen
Modified: 2009-10-08 03:47 PDT (History)
1 user (show)

See Also:


Attachments
proposed patch (28.13 KB, patch)
2009-09-03 05:31 PDT, Balazs Kelemen
mrowe: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Balazs Kelemen 2009-09-03 05:29:57 PDT
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.
Comment 1 Balazs Kelemen 2009-09-03 05:31:05 PDT
Created attachment 38982 [details]
proposed patch
Comment 2 Balazs Kelemen 2009-09-03 05:47:37 PDT
I would note that I used stl containers because those supports custom allocators.
Comment 3 Mark Rowe (bdash) 2009-09-03 19:23:28 PDT
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.
Comment 4 Balazs Kelemen 2009-09-04 09:03:04 PDT
(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.