Issue 162208 in chromium: Chromium on Android should probably use TCMalloc

[chro...@googlecode.com]

Status: Untriaged
Owner: ----
CC: esei...@chromium.org, pe...@chromium.org, joc...@chromium.org,
jam...@chromium.org, di...@chromium.org, aber...@chromium.org
Labels: Type-Bug Pri-2 Area-Internals

New issue 162208 by aba...@chromium.org: Chromium on Android should
probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

TCMalloc is a big win over the system allocator on Windows and Linux. We
should try turning it on for Android to see if its a win there too.

See also https://bugs.webkit.org/show_bug.cgi?id=102866 and
https://code.google.com/p/chromium/issues/detail?id=158645 for a similar
discussion about Mac.

[chro...@googlecode.com]

Updates:
Labels: OS-Android

Comment #1 on issue 162208 by aba...@chromium.org: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

(No comment was entered for this change.)

[chro...@googlecode.com]

Comment #2 on issue 162208 by macd...@gmail.com: Chromium on Android should

probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

It's possible that Android can use TCMalloc for the whole binary, like how
linux/win works.

[chro...@googlecode.com]

Updates:
Cc: dmik...@chromium.org

Comment #6 on issue 162208 by dmik...@chromium.org: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

(No comment was entered for this change.)

--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

[chro...@googlecode.com]

Updates:
Cc: migu...@chromium.org bul...@chromium.org

Comment #7 on issue 162208 by bul...@chromium.org: Chromium on Android

[chro...@googlecode.com]

Comment #10 on issue 162208 by bul...@chromium.org: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

Thanks! I just quickly tried "disable_debugallocation=1" but it still fails.
I didn't do a full clean build, I'll do that soon.

[chro...@googlecode.com]

Comment #11 on issue 162208 by dmik...@chromium.org: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

Please let us know the stacktrace when you try clean build. :)

[chro...@googlecode.com]

Comment #12 on issue 162208 by bul...@chromium.org: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

Fairly similar "infinite" stack trace here
with "disable_debugallocation=1"..
It looks like there's some misconfiguration somewhere and it keeps calling
itself. Investigating.

#0 tcmalloc::ThreadCache::GetCache ()
at ../../third_party/tcmalloc/chromium/src/thread_cache.h:409
#1 0x5f7d37a8 in (anonymous namespace)::do_malloc (size=136)
at ../../third_party/tcmalloc/chromium/src/tcmalloc.cc:1094
#2 0x60370f30 in do_calloc (elem_size=<optimized out>, n=34)
at ../../third_party/tcmalloc/chromium/src/tcmalloc.cc:1127
#3 tc_calloc (n=34, elem_size=<optimized out>)
at ../../third_party/tcmalloc/chromium/src/tcmalloc.cc:1574
#4 0x6036de1c in __emutls_get_address (obj=0x614b95a0)
at /tmp/ndk-toolchain-src/gcc/gcc-4.6/libgcc/../gcc/emutls.c:158
#5 0x5f7d2d12 in GetThreadHeap ()
at ../../third_party/tcmalloc/chromium/src/thread_cache.h:403
#6 tcmalloc::ThreadCache::GetCache ()
at ../../third_party/tcmalloc/chromium/src/thread_cache.h:414
#7 0x5f7d37a8 in (anonymous namespace)::do_malloc (size=136)
at ../../third_party/tcmalloc/chromium/src/tcmalloc.cc:1094
#8 0x60370f30 in do_calloc (elem_size=<optimized out>, n=34)
at ../../third_party/tcmalloc/chromium/src/tcmalloc.cc:1127
#9 tc_calloc (n=34, elem_size=<optimized out>)
at ../../third_party/tcmalloc/chromium/src/tcmalloc.cc:1574
#10 0x6036de1c in __emutls_get_address (obj=0x614b95a0)
at /tmp/ndk-toolchain-src/gcc/gcc-4.6/libgcc/../gcc/emutls.c:158
#11 0x5f7d2d12 in GetThreadHeap ()
at ../../third_party/tcmalloc/chromium/src/thread_cache.h:403
#12 tcmalloc::ThreadCache::GetCache ()
at ../../third_party/tcmalloc/chromium/src/thread_cache.h:414
#13 0x5f7d37a8 in (anonymous namespace)::do_malloc (size=136)
at ../../third_party/tcmalloc/chromium/src/tcmalloc.cc:1094
#14 0x60370f30 in do_calloc (elem_size=<optimized out>, n=34)
at ../../third_party/tcmalloc/chromium/src/tcmalloc.cc:1127
#15 tc_calloc (n=34, elem_size=<optimized out>)
at ../../third_party/tcmalloc/chromium/src/tcmalloc.cc:1574
#16 0x6036de1c in __emutls_get_address (obj=0x614b95a0)
at /tmp/ndk-toolchain-src/gcc/gcc-4.6/libgcc/../gcc/emutls.c:158
#17 0x5f7d2d12 in GetThreadHeap ()
at ../../third_party/tcmalloc/chromium/src/thread_cache.h:403
#18 tcmalloc::ThreadCache::GetCache ()
at ../../third_party/tcmalloc/chromium/src/thread_cache.h:414
#19 0x5f7d37a8 in (anonymous namespace)::do_malloc (size=136)
at ../../third_party/tcmalloc/chromium/src/tcmalloc.cc:1094
#20 0x60370f30 in do_calloc (elem_size=<optimized out>, n=34)
at ../../third_party/tcmalloc/chromium/src/tcmalloc.cc:1127
#21 tc_calloc (n=34, elem_size=<optimized out>)
at ../../third_party/tcmalloc/chromium/src/tcmalloc.cc:1574
#22 0x6036de1c in __emutls_get_address (obj=0x614b95a0)
at /tmp/ndk-toolchain-src/gcc/gcc-4.6/libgcc/../gcc/emutls.c:158
#23 0x5f7d2d12 in GetThreadHeap ()
at ../../third_party/tcmalloc/chromium/src/thread_cache.h:403
#24 tcmalloc::ThreadCache::GetCache ()
at ../../third_party/tcmalloc/chromium/src/thread_cache.h:414
#25 0x5f7d37a8 in (anonymous namespace)::do_malloc (size=136)
at ../../third_party/tcmalloc/chromium/src/tcmalloc.cc:1094
#26 0x60370f30 in do_calloc (elem_size=<optimized out>, n=34)
at ../../third_party/tcmalloc/chromium/src/tcmalloc.cc:1127
#27 tc_calloc (n=34, elem_size=<optimized out>)
at ../../third_party/tcmalloc/chromium/src/tcmalloc.cc:1574
...

repeats itself on and on...

[chro...@googlecode.com]

Comment #13 on issue 162208 by pli...@chromium.org: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

Interesting, looks like malloc() is calling malloc() :) At a glance, to me
this seems caused by __emutls_get_address() (in the NDK) which
unfortunately does a calloc.

I believe the current malloc() that we use doesn't use TLS which would
explain why we are only seeing this problem now.

[chro...@googlecode.com]

Comment #14 on issue 162208 by bul...@chromium.org: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

yes, thanks for the hint! :)
tweaking:
https://codereview.chromium.org/14321006/diff/1/third_party/tcmalloc/chromium/src/config_android.h

so that HAVE_TLS is undefined, I can see the page loading and it shows a
google logo, yay! :)

it crashes far further down the line, let me see if it's actually due to
TCMalloc.

[chro...@googlecode.com]

Comment #15 on issue 162208 by pli...@google.com: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

Nice, congrats!

[chro...@googlecode.com]

Comment #16 on issue 162208 by di...@chromium.org: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

FWIW, __emutls_get_address() is provided by libgcc to emulate the support
of __thread variables on platforms where the dynamic linker doesn't support
them (e.g. Android).

An alternative would be to use pthread_setspecific/getspecific instead to
access the thread-local data of interest. This will work without a malloc()
and will be much faster than emults as well.

Looking at the code, it seems you only need to undefine HAVE_TLS in
config_android.h

[chro...@googlecode.com]

Comment #17 on issue 162208 by bul...@chromium.org: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

Thanks everyone! The crash I saw it's actually due to Geolocation in
content shell (sigh, also mine..).
https://code.google.com/p/chromium/issues/detail?id=175749

I'll fix that in a bit so we can get back to this and see if we can get all
the traces needed by DMP.

[chro...@googlecode.com]

Comment #18 on issue 162208 by bul...@chromium.org: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

FYI, thanks do Dai, I just managed to get a heap dump from TCMalloc.

Since chrome on android doesn't have "environment variables", it requires
forcing it via a command line flag.

My understanding is that DMP's Heap Profiler extensions just checks for env
variables, so Dai will soon provide entry hooks that can be called from our
port.

Sample:
adb_content_shell_command_line "--HEAPPROFILE=/data/local/tmp/w/"

heap profile: 0: 0 [ 0: 0] @ heapprofile

MAPPED_LIBRARIES:
40000000-40008000 rw-p 00000000 00:00 0
40008000-40009000 r--- 012a6000 00:00 773684
/data/app/org.chromium.content_shell_apk-1.apk
40009000-4000a000 r--- 012a6000 00:00 773684
/data/app/org.chromium.content_shell_apk-1.apk
4000a000-4000c000 r-xp 00000000 00:00 100 /system/bin/app_process
4000c000-4000d000 r--p 00001000 00:00 100 /system/bin/app_process
4000d000-4000e000 rw-p 00000000 00:00 0
4000e000-40016000 rw-p 00000000 00:00 0
40017000-40018000 rw-p 00000000 00:00 0
40018000-40028000 rw-p 00000000 00:00 0
40029000-4002e000 rw-p 00000000 00:00 0
4002e000-40032000 rw-p 00000000 00:00 0
40032000-4003e000 r--- 0014a000 00:00 773684
/data/app/org.chromium.content_shell_apk-1.apk

[chro...@googlecode.com]

Comment #19 on issue 162208 by dmik...@chromium.org: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

I hope this patch works to set some options in tcmalloc's heap profiler.
Could you try?

https://codereview.chromium.org/14642011/ (not landed yet)

[chro...@googlecode.com]

Comment #20 on issue 162208 by di...@google.com: Chromium on Android should

probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

Nice work guys :-)

By the way, it's usually easier to use system properties, rather than
command-line flags to control this kind of this. Look at the
<sys/system_properties.h> header in the NDK
(third_party/android_tools/ndk/platforms/android-9/arch-arm/usr/include/sys/system_properties.h).

[chro...@googlecode.com]

Comment #25 on issue 162208 by dmik...@chromium.org: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

Hmm, the lock is clearly acquired by
MemoryRegionMap::LockHolder lock_holder;
in the top of SnapshotMaps().

It's working on Linux, and I guess there's no difference in the function
between Linux and Android. (I'm a little worried that base/spinlock is not
working well, but it's unlikely...)

Let's take a deeper look at this tomorrow.

[chro...@googlecode.com]

Comment #26 on issue 162208 by bul...@chromium.org: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

Hooked up TCMalloc's log with android:

I/chromium( 6577): Starting tracking the heap
I/chromium( 6577): [6577] Starting a deep memory profiler
D/TraceEvent( 6577): New enabled flags: 10
I/chromium( 6577): Dumping heap profile to
/data/local/tmp/w/dump.06577.0001.heap (0 sec since the last dump)
I/chromium( 6577): Check failed: regions_ != NULL:


ideas?

[chro...@googlecode.com]

Comment #27 on issue 162208 by dmik...@chromium.org: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

That means that no mmap calls are recorded yet (the second assert in
BeginRegionLocked). Firstly I thought that the mmap profiler didn't start,
but another possibility may be no mmap was called between mmap profiler's
initialization and BeginRegionLocked().

I just found that tcmalloc's mmap profiler initializes |record_| when the
first mmap is called after initialization. The failure usually doesn't
happen since mmap profiler starts very early in usual Linux processes.

I'm looking into it. If you want to check it quickly, try calling some
mmap() just before calling the first HeapProfilerDump().

[chro...@googlecode.com]

Comment #28 on issue 162208 by dmik...@chromium.org: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

Also, could you try https://codereview.chromium.org/14645016/ to use system
properties in tcmalloc?

[chro...@googlecode.com]

Comment #29 on issue 162208 by dmik...@chromium.org: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

Saw the dumped files.

* mmap calls are actually not recorded.
* malloc calls are recorded, but no backtraces.

Hmm, will take a look...

[chro...@googlecode.com]

Comment #30 on issue 162208 by dmik...@chromium.org: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

fyi, enabling tcmalloc's logging in Android:
https://codereview.chromium.org/14845011/

[chro...@googlecode.com]

Comment #32 on issue 162208 by bul...@chromium.org: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

Thanks for the patch! I just moved that to tcmalloc, and got:

cat /data/local/tmp/w/T.26259.0002.buckets
1139 malloc 0x5f2edc7e 0x5f707874 0x5f6cb6a4 0x5f706798 0x5f716d34
735 malloc 0x5f90011e 0x5f900148 0x5f900188 0x5f90027e 0x5f9002b2
931 malloc 0x5f7ecf40 0x5f7e10e4 0x5f272d06 0x5f7d70f6 0x5f7d7206
875 malloc 0x5f2edc7e 0x5f707874 0x5f6cb6a4 0x5f6cc69e 0x5f6d98d4
1087 malloc 0x5f38c302 0x5f3a0ebe 0x5f3a1084 0x5f94e83a 0x5ef49a78
890 malloc 0x5f6c83ca 0x5f6d9784 0x5f6d9de6 0x5f779868 0x5f1d159a
713 malloc 0x5f8a5244 0x5f8a68ca 0x5f8bf710 0x5fb65084 0x5fb6518e
887 malloc 0x5f8aada4 0x5f8aade4 0x5f1a9a64 0x5f1d1520 0x5f1d174e
...
...

[chro...@googlecode.com]

Comment #33 on issue 162208 by dmik...@chromium.org: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

Yay! Thanks for working on that. Just in case, is the length of each line
just 5?

For mmap tracing, I think the following three patches would work.
1. https://codereview.chromium.org/14740014/ (just canceling your
disablement of malloc_hook_mmap_linux.h)
2. https://codereview.chromium.org/14859005/ (make malloc_hook_mmap_linux.h
work. may need more work.)
3. https://codereview.chromium.org/14859006/ (fix printing pointer values)


btw, just in case, do you keep separate patches before merging into
https://codereview.chromium.org/14321006/? I'm just wondering that each
patch would be updated independently and that we finally should land these
patches separately.

[chro...@googlecode.com]

Comment #35 on issue 162208 by bugdro...@chromium.org: Chromium on Android
should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208#c35

------------------------------------------------------------------------
r198355 | dmik...@chromium.org | 2013-05-05T03:47:44.830176Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/third_party/tcmalloc/chromium/src/heap-profiler.cc?r1=198355&r2=198354&pathrev=198355
M
http://src.chromium.org/viewvc/chrome/trunk/src/third_party/tcmalloc/chromium/src/base/commandlineflags.h?r1=198355&r2=198354&pathrev=198355
M
http://src.chromium.org/viewvc/chrome/trunk/src/third_party/tcmalloc/chromium/src/base/sysinfo.cc?r1=198355&r2=198354&pathrev=198355
M
http://src.chromium.org/viewvc/chrome/trunk/src/third_party/tcmalloc/chromium/src/deep-heap-profile.cc?r1=198355&r2=198354&pathrev=198355
M
http://src.chromium.org/viewvc/chrome/trunk/src/third_party/tcmalloc/chromium/src/deep-heap-profile.h?r1=198355&r2=198354&pathrev=198355

Use system properties for heap profiler in Android instead of environment
variables.

The following system properties are used instead of the environment
variables.
HEAPPROFILE => heapprof
HEAP_PROFILE_ALLOCATION_INTERVAL => heapprof.allocation_interval
HEAP_PROFILE_DEALLOCATION_INTERVAL => heapprof.deallocation_interval
HEAP_PROFILE_INUSE_INTERVAL => heapprof.inuse_interval
HEAP_PROFILE_TIME_INTERVAL => heapprof.time_interval
HEAP_PROFILE_MMAP_LOG => heapprof.mmap_log
HEAP_PROFILE_MMAP => heapprof.mmap
HEAP_PROFILE_ONLY_MMAP => heapprof.only_mmap
DEEP_HEAP_PROFILE => heapprof.deep_heap_profile
HEAP_PROFILE_TYPE_STATISTICS => heapprof.type_statistics

BUG=162208
R=bul...@chromium.org, will...@chromium.org

Review URL: https://chromiumcodereview.appspot.com/14645016
------------------------------------------------------------------------

[chro...@googlecode.com]

Updates:
Cc: cev...@chromium.org

Comment #36 on issue 162208 by esei...@chromium.org: Chromium on Android
should probably use TCMalloc

http://code.google.com/p/chromium/issues/detail?id=162208

(No comment was entered for this change.)

[chro...@googlecode.com]

Comment #39 on issue 162208 by bugdro...@chromium.org: Chromium on Android
should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208#c39

------------------------------------------------------------------------
r198878 | dmik...@chromium.org | 2013-05-08T12:01:10.849710Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/third_party/tcmalloc/chromium/src/deep-heap-profile.cc?r1=198878&r2=198877&pathrev=198878
M
http://src.chromium.org/viewvc/chrome/trunk/src/third_party/tcmalloc/chromium/src/memory_region_map.cc?r1=198878&r2=198877&pathrev=198878
M
http://src.chromium.org/viewvc/chrome/trunk/src/third_party/tcmalloc/chromium/src/memory_region_map.h?r1=198878&r2=198877&pathrev=198878

Fix mmap region iteration while no regions are recorded.

If no mmap regions are recorded, iteration failed since the
RegionSet (std::set) object is not initialized.

BUG=162208
R=will...@chromium.org

Review URL: https://codereview.chromium.org/14769008
------------------------------------------------------------------------

[chro...@googlecode.com]

Comment #40 on issue 162208 by bugdro...@chromium.org: Chromium on Android
should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208#c40

------------------------------------------------------------------------
r198879 | dmik...@chromium.org | 2013-05-08T12:05:52.617188Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/third_party/tcmalloc/chromium/src/base/logging.h?r1=198879&r2=198878&pathrev=198879

Enable tcmalloc's logging in Android.

BUG=162208
R=bul...@chromium.org, will...@chromium.org

Review URL: https://codereview.chromium.org/14845011

[chro...@googlecode.com]

Issue 162208: Chromium on Android should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

This issue is now blocking issue chromium:231800.
See http://code.google.com/p/chromium/issues/detail?id=231800

--
You received this message because you are listed in the owner
or CC fields of this issue, or because you starred this issue.
You may adjust your issue notification preferences at:
http://code.google.com/hosting/settings

[chro...@googlecode.com]

Comment #42 on issue 162208 by bugdro...@chromium.org: Chromium on Android
should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208#c42

------------------------------------------------------------------------
r199471 | dmik...@chromium.org | 2013-05-10T13:23:22.256916Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/third_party/tcmalloc/chromium/src/deep-heap-profile.cc?r1=199471&r2=199470&pathrev=199471

Use PRIx64 instead of PRIxPTR for uint64 in deep-heap-profile.

BUG=162208
R=jyas...@chromium.org, will...@chromium.org

Review URL: https://codereview.chromium.org/14859006
------------------------------------------------------------------------

--

[chro...@googlecode.com]

Comment #43 on issue 162208 by bugdro...@chromium.org: Chromium on Android
should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208#c43

------------------------------------------------------------------------
r199899 | dmik...@chromium.org | 2013-05-14T04:48:31.354105Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/third_party/tcmalloc/chromium/src/malloc_hook_mmap_linux.h?r1=199899&r2=199898&pathrev=199899
M
http://src.chromium.org/viewvc/chrome/trunk/src/third_party/tcmalloc/chromium/src/base/linux_syscall_support.h?r1=199899&r2=199898&pathrev=199899

Support Android in MallocHook for mmap, munmap and mremap.

sbrk is not supported for now because __sbrk is not provided by Bionic.
Chrome for Android doesn't look like using sbrk.

BUG=162208

Review URL: https://chromiumcodereview.appspot.com/14859005

[chro...@googlecode.com]

Comment #44 on issue 162208 by dmik...@chromium.org: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

Status:

To just enable tcmalloc and its heap profiler behind a flag, only
https://codereview.chromium.org/14321006/ is left (while we still need
performance tuning and android_use_tcmalloc=1 to make it default).

Deep Memory Profiler things will go http://crbug.com/231800.

[chro...@googlecode.com]

Updates:
Labels: Performance-Memory

Comment #48 on issue 162208 by l.gom...@samsung.com: Chromium on Android

should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208

(No comment was entered for this change.)

[chro...@googlecode.com]

Comment #49 on issue 162208 by bugdro...@chromium.org: Chromium on Android
should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208#c49

The following revision refers to this bug:

https://chromium.googlesource.com/chromium/src.git/+/1b99b4ab5e12bd9e1e420aeb03da9b8c3d1b5310

commit 1b99b4ab5e12bd9e1e420aeb03da9b8c3d1b5310
Author: jungj...@samsung.com
<jungj...@samsung.com@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri May 16 11:48:14 2014

Fix build error with tcmalloc for android

malloc_usable_size has been changed in android ndk header.
(https://code.google.com/p/android/issues/detail?id=55725)
so to override the function properly, this header should be changed too.

BUG= 162208

Review URL: https://codereview.chromium.org/284843002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@270993
0039d316-1c4b-4281-b951-d872f2087c98

[chro...@googlecode.com]

Comment #50 on issue 162208 by bugdro...@chromium.org: Chromium on Android
should probably use TCMalloc
http://code.google.com/p/chromium/issues/detail?id=162208#c50

------------------------------------------------------------------
r270993 | jungj...@samsung.com | 2014-05-16T11:48:14.079558Z

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/third_party/tcmalloc/chromium/src/libc_override_gcc_and_weak.h?r1=270993&r2=270992&pathrev=270993

Fix build error with tcmalloc for android

malloc_usable_size has been changed in android ndk header.
(https://code.google.com/p/android/issues/detail?id=55725)
so to override the function properly, this header should be changed too.

BUG= 162208

Review URL: https://codereview.chromium.org/284843002

-----------------------------------------------------------------

[chro...@googlecode.com]

Comment #52 on issue 162208 by pa...@chromium.org: Chromium on Android
should probably use TCMalloc
https://code.google.com/p/chromium/issues/detail?id=162208

I think we took other optimization opportunities and did not experiment
much with TCMalloc. As digit@ mentioned earlier, tcmalloc might be not as
good on ARM as it is on x86, needs non-trivial experimentation. I think it
would be useful to run benchmarks and collect data, this may also help
eliminate some unnecessary allocation patterns, even if tcmalloc is not
enabled in the end.

Also, there is PartitionAlloc that primiano@ was looking at, but this
effort was phased out by more important work.

[chro...@googlecode.com]

Updates:
Status: Available
Owner: ---

Comment #55 on issue 162208 by to...@chromium.org: Chromium on Android

should probably use TCMalloc
https://code.google.com/p/chromium/issues/detail?id=162208

Not sure how I ended up owning this but I'm not intending to work on it.

[chro...@googlecode.com]

Comment #57 on issue 162208 by primi...@chromium.org: Chromium on Android

should probably use TCMalloc
https://code.google.com/p/chromium/issues/detail?id=162208

Just an unrelated thing we realized while looking at something similar with
ruuda@:
if you guys want to do this, this is going to be very tricky.
System libraries are preloaded on Android, and malloc there is not even a
weak symbol.
This means that you are not going to be able to force all the libraries on
Android to use whatever allocator you look like, potentially opening the
doors to lot of split-heap bugs, where something is allocated by a system
library via malloc and is freed by tcmalloc.

One example to rule them all?
strdup() is defined in libc, where malloc lives.
If you write code in Chrome like:

content/something.cc:
auto x = strdup("fooo");
free(x);

this will pass a heap pointer allocated via android malloc() to tcmalloc's
free.
Is tcmalloc smart enough to deal with that? Dunno.
Just saying: be really careful if you go there.

[chro...@googlecode.com]

Comment #58 on issue 162208 by primi...@chromium.org: Chromium on Android

should probably use TCMalloc
https://code.google.com/p/chromium/issues/detail?id=162208

self-re #57 s/whatever allocator you look like/whatever allocator you want/

[chro...@googlecode.com]

Updates:
Owner: ---
Cc: li...@chromium.org

Comment #59 on issue 162208 by pa...@chromium.org: Chromium on Android

should probably use TCMalloc
https://code.google.com/p/chromium/issues/detail?id=162208

yeah, allocating from libc functions is annoying when malloc is not weak.

Some context since last change:

lizeb@ has thoughts on how to compare memory fragmentation across
tcmalloc/jemalloc in more fair terms. I think we want that before enabling
malloc/whatever.

also, there has been some experimentation with jemalloc (plus tweaking) in
Android M. Would be interesting to know the conclusion, especially
because "chrome is special".

[chro...@googlecode.com]

Updates:
Cc: agr...@chromium.org

Comment #60 on issue 162208 by pa...@chromium.org: Chromium on Android

should probably use TCMalloc
https://code.google.com/p/chromium/issues/detail?id=162208

I don't have an owner in mind for this in the short term. This issue is
Free!