tcmalloc debug windows port

[GoogleCodeExporter]

Hi,

following issue 284, and patches submitted in issues 292, 293 and 294, attached 
a patched porting tcmalloc debug to windows (this patch contain the previous 
patches, since i could not compile it without them).

As explained in issue 284 (you can read there for more detailed explanations), 
I had to statically link tcmalloc and CRT in order to use debugallocation 
correctly.
in the patch there is a perl script (stript_crt.pl) which change symbols in 
libcmt.lib. you have to use this script in order to link your program with this 
port of tcmalloc.

I know that this patch is not small, but it is the best I could do.

changes in the libtcmalloc_minimal project:
1. properties -> general -> Configuration type -> Static lib
2. properties -> c/c++ -> Code Generation -> runtime lib -> empty
3. omit patch_function.cc from compilation
4. add zbacktrace.cc to compilation

I have another patch with heap profiling port to windows, but it is based on 
this patch, and I would like to discuss (and hopefully merge to trunk) this 
patch, before I continue.

yoni.

Original issue reported on code.google.com by jontra@gmail.com on 8 Dec 2010 at 3:10

Attachments:

• debug.patch

[GoogleCodeExporter]

This patch is still doing at least three different things, that I could see at 
first glance:

1) Some sort of stacktrace printing on error in debugallocation.cc.
2) Changing debugallocation so instead of overriding the libc functions 
directly, it's defining new tc_debug_foo methods.
3) Adding indirection(?) in override_functions.cc

Each of these should be its own patch ((1) should arguably be part of the 
previous stacktrace-printing patch).

Basically, I'd like to see a progression of patches -- applying them one by one 
in series, each of them should work correctly, and the end result should be 
what you want.  For instance, your patch for (2) can add the new tc_debug_foo 
methods, and then make them aliases for malloc/free/etc just like tcmalloc.cc 
does now.  I know eventually you won't want them to be aliases, but for that 
patch they should be, because you don't want to break existing behavior.  The 
point is that this patch is arguably a cleanup in its own right, and a 
necessary modification for a future patch to be able to do what you want it to 
do.

I suggest we concentrate on the stacktrace stuff first, and figure out what's 
going on there -- what problem you're trying to solve, and a patch that solves 
it.  Once that's done, let's figure out how to refactor debugallocation.cc to 
prepare it for your windows-specific changes (without doing anything 
windows-specific yet).  And then we'll go from there.

Original comment by csilv...@gmail.com on 9 Dec 2010 at 5:19

• Added labels: Type-Patch, Priority-Medium

[GoogleCodeExporter]

Any more word on this?  I'm hoping to do a new perftools release relatively 
soon, and would love to have this functionality in, but I'm afraid I'll need 
some help disentangling the bits of the patch.

Original comment by csilv...@gmail.com on 10 Jan 2011 at 1:45

[GoogleCodeExporter]

Hi,

I have been busy for a while, and could not make time to work on splitting the 
patch.
In the mean time, I continued to improve my patch, and now it contains 3 issues:

1. improve malloc performance on win32 by 30% (by enabling TLS)
2. port debugallocation to windows
3. enable override_functions with patch_functions + script to strip libcmt

I know this is a large patch, and it contains different issues, But it does 
improve
tcmalloc significally, and I hope someone will take it from here toward merging 
to trunk.

Here is an example how debugallocation works on win32:

program:
#include <stdlib.h>

int main(int argc, char *argv[])
{
    char *s = malloc(10);
    return 0;
}

output:
1 leaks of malloc(10).
  0x01381012 c:/cygwin/home/yoni/zon-dev/build.appd/pkg/util/test.c:5:main
  0x0144887E f:/dd/vctools/crt_bld/self_x86/crt/src/crt0.c:278:__tmainCRTStartup
  0x75A13C45 N/A

totals:
mallocs: 1 (10 bytes)
frees: 0 (0 bytes)
leaks: 1 (10 bytes)

Original comment by jontra@gmail.com on 29 Jun 2011 at 9:13

Attachments:

• tcmalloc.patch

[GoogleCodeExporter]

OK, the TLS fix is a one-liner (well, 4-liner), so I've made that.

*blush*  I thought I had enabled TLS for windows but had missed this 
requirement.  That's what I get for making the test have so many moving parts...

The other two changes, regarding debugallocation and 
override_functions+patch_functions together, I have not yet tried (and probably 
won't for the next release).

Original comment by csilv...@gmail.com on 8 Jul 2011 at 10:49

[GoogleCodeExporter]

Great.

Let me know when you have time to look at debugallocation and 
override_functions+patch_functions.
I will try to help as much as i can.

Original comment by jontra@gmail.com on 12 Jul 2011 at 8:12

[GoogleCodeExporter]

Hi Craig.
have you by any chance looked at the debugallocation change?

Original comment by jontra@gmail.com on 28 Jul 2011 at 6:53

[GoogleCodeExporter]

} I will try to help as much as i can.

The best way to help would be to split this patch up.  I'd love to see a change 
that just adds debugallocation, without adding a level of redirection like the 
patch has now (as I understand it), and doesn't try to change the way functions 
are overridden.  The idea is to allow the creation of two libraries, one with 
tcmalloc.cc and one with debugallocation.cc, like is currently done in 
unix-land.

Original comment by csilv...@gmail.com on 28 Jul 2011 at 6:58

[GoogleCodeExporter]

Haven't heard more about this in a few months, so I'm closing.  Jontra, or 
anyone else reading this, feel free to reopen if you can split this up into 
self-contained, single-functionality patches.

Original comment by csilv...@gmail.com on 18 Oct 2011 at 4:55

• Changed state: WontFix