Reland (2) of Enable allocator shim for Android (crrev.com/1875043003)
Reason for reland: The CL was re-reverted by crrev.com/1884223002 because it broke the internal orderfile bot. The reason of the breakage was the following: - cygprofile.cc instruments all function calls with a __cyg_profile_func_enter preamble. - __cyg_profile_func_enter uses __thread. __thread under the hoods invokes calloc(), on every thread, to initialize the TLS. - The shim layer provides its own implementation of calloc(). - At this point, calloc() gets instrumented as well and it re-enters __cyg_profile_func_enter causing an infinite loop. The key of the problem here is that __thread silently causes calls to calloc() in a way that is out of control of cygprofile.cc. The solution proposed by this CL is the following: - Don't use __thread, use explicit POSIX functions for TLS (also there doesn't seem to be any precendent of using __thread in the codebase). - Use a global variable to prevent re-entrancy of the __cyg_profile_func_enter in the global (once per process) TLS slot initializer. - Re-entrancy is gone. Original issue's description: > Enable allocator shim for Android > > This is a follow-up to crrev.com/1719433002, which introduced the > shim for Android, and enables it by default by setting > use_experimental_allocator_shim=true for Android. > > Build/Perf sheriffs heads up > ---------------------------- > If you see any build error or crash related with __wrap_malloc, > __wrap_free, __real_malloc, __real_free, etc this CL is to blame. > > Performance considerations > ------------------------ > Binary size diff (GN, arm, static, official build): 24k > > I did a mixture of local and trybots run to estimate the perf impact > of this change. Didn't get any conclusive data, everything I tried > seems in the same ballpark, below noise levels. More in details: > > cc_perftests.PrepareTiles on a Nexus 4. > Rationale of the choice: in a previous CL (crbug.com/593344), this > benchmark revealed the presence of two mfences in the malloc path. > Results: https://goo.gl/8VC3Jp in the same ballpark. > > page-cycler on Nexus 9 via trybots: > Results: http://goo.gl/J3i50a seems to suggest that this CL improves > both warm and cold times in most cases. I doubt it, more likely it's > noise. > > All the other perf trybots failed. The perf waterfall seems to be in a > bad state in these days. > > BUG=550886,598075 > TEST=base_unittests --gtest_filter=AllocatorShimTest.* > TBR=thakis@chromium.org > > Committed: https://crrev.com/ebb95496c73dc0d5ce83968ac619921f154305f7 > Cr-Commit-Position: refs/heads/master@{#386386} BUG=550886,598075, 602744 TBR=thakis@chromium.org TEST=gn gen out/Debug --args='is_debug=true target_os="android" use_order_profiling=true target_cpu="arm" is_clang=false'; ninja -C out/Debug/ cygprofile_unittests; adb push out/Debug/cygprofile_unittests /data/local/tmp/cygprofile_unittests_debug; adb shell /data/local/tmp/cygprofile_unittests_debug Review URL: https://codereview.chromium.org/1883093005 Cr-Original-Commit-Position: refs/heads/master@{#387594} Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: f7a321facfdabd763ecdbc9536c890fe91c8c079
This commit is contained in:
Родитель
61f5561b34
Коммит
35f5ffe8da
|
@ -2274,7 +2274,7 @@
|
|||
'use_allocator%': 'none',
|
||||
'use_sanitizer_options%': 1,
|
||||
}],
|
||||
['OS=="linux" and asan==0 and msan==0 and lsan==0 and tsan==0 and build_for_tool==""', {
|
||||
['(OS=="linux" or OS=="android") and asan==0 and msan==0 and lsan==0 and tsan==0 and build_for_tool==""', {
|
||||
'use_experimental_allocator_shim%': 1,
|
||||
}],
|
||||
['OS=="linux" and asan==0 and msan==0 and lsan==0 and tsan==0', {
|
||||
|
|
|
@ -11,7 +11,7 @@ if (is_android || current_cpu == "mipsel" || is_mac || is_ios || is_asan ||
|
|||
_default_allocator = "tcmalloc"
|
||||
}
|
||||
|
||||
if (is_linux && !is_asan && !is_lsan && !is_tsan && !is_msan) {
|
||||
if ((is_linux || is_android) && !is_asan && !is_lsan && !is_tsan && !is_msan) {
|
||||
_default_use_experimental_allocator_shim = true
|
||||
} else {
|
||||
_default_use_experimental_allocator_shim = false
|
||||
|
|
Загрузка…
Ссылка в новой задаче