tcmalloc is unusually busy on images by arm-gcc |
||||||||||||
Issue description
# Overhead Samples Command Shared Object Symbol
# ........ ........ ................ ................ ................................................................
#
2.23% 59417 chrome chrome [.] tcmalloc::ThreadCache::CreateCacheIfNecessary
1.29% 32686 chrome chrome [.] content::protocol::StringValue::writeJSON
1.24% 38603 CompositorTileW chrome [.] neon_and_crc32::blit_row_s32a_opaque
1.01% 49138 swapper [kernel.kallsyms [k] cpuidle_enter_state
1.01% 23368 chrome chrome [.] v8::internal::BodyDescriptorBase::IteratePointers<v8::intern
0.93% 29075 chrome libmali.so.0.1.2 [.] stdlibp_neon_fast_aligned_memcpy_64
0.90% 43748 swapper [kernel.kallsyms [k] _raw_spin_unlock_irq
0.82% 27078 chrome [kernel.kallsyms [k] _raw_spin_unlock_irqrestore
0.81% 22145 chrome chrome [.] tcmalloc::CentralFreeList::InsertRange
0.74% 20842 chrome chrome [.] tcmalloc::CentralFreeList::ReleaseListToSpans
0.72% 20470 chrome chrome [.] tcmalloc::CentralFreeList::RemoveRange
A gcc bug breaks the caching mechanism of tcmalloc so CreateCacheIfNecessary is called every time.
This caused ~8.5% performance drop on arm Chrome OS devices.
,
Jan 19 2017
,
Jan 19 2017
Related bugs: crbug.com/629593 crbug.com/650137
,
Jan 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/07dcf42f46e211a53f7c33ed634cc5f416a3929f commit 07dcf42f46e211a53f7c33ed634cc5f416a3929f Author: laszio <laszio@chromium.org> Date: Fri Jan 20 18:53:36 2017 tcmalloc: Use the default TLS model on arm-gcc, CrOS only. gcc has a bug in __attribute__ ((tls_model ("initial-exec"))) on arm. This patch undoes the TLS optimization on arm-gcc. BUG= 682840 TEST=threadlocal_heap_ works as expected; CreateCacheIfNecessary() was only called a few times in a reasonably long benchmark. Review-Url: https://codereview.chromium.org/2640263003 Cr-Commit-Position: refs/heads/master@{#445109} [modify] https://crrev.com/07dcf42f46e211a53f7c33ed634cc5f416a3929f/third_party/tcmalloc/README.chromium [modify] https://crrev.com/07dcf42f46e211a53f7c33ed634cc5f416a3929f/third_party/tcmalloc/chromium/src/thread_cache.cc [modify] https://crrev.com/07dcf42f46e211a53f7c33ed634cc5f416a3929f/third_party/tcmalloc/chromium/src/thread_cache.h
,
Jan 20 2017
The grapevine says that this happened on pagecycler benchmarks.
,
Jan 20 2017
More contexts: the page load time (cold and warm), in terms of page_cycler_v2.typical_25, increased 8.5% ~ 8.6% because of this bug.
,
Jan 20 2017
Pagecycler certainly showed the issue, but Sonny saw other things affected as well. Officially requesting a merge. Plan would be to double-check the Canary (once it builds) and make sure things are good. Then decide how much bake time we want and land in R56. Since we're so close to R56, we can always have an IM conversation about this.
,
Jan 20 2017
This bug requires manual review: We are only 10 days from stable. Please contact the milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), gkihumba@(cros), bustamante@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 20 2017
Got approval to M57 from ketakid@ on IM. Will first land on 57. Specifically: git drover --branch 2984 --cherry-pick 07dcf42f46e211a53f7c33ed634cc5f416a3929f git drover --branch 2987 --cherry-pick 07dcf42f46e211a53f7c33ed634cc5f416a3929f
,
Jan 20 2017
,
Jan 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ea757b6a950c73efcf91e3425d64e75cf2793beb commit ea757b6a950c73efcf91e3425d64e75cf2793beb Author: Alex Mineer <amineer@chromium.org> Date: Fri Jan 20 23:10:59 2017 tcmalloc: Use the default TLS model on arm-gcc, CrOS only. gcc has a bug in __attribute__ ((tls_model ("initial-exec"))) on arm. This patch undoes the TLS optimization on arm-gcc. BUG= 682840 TEST=threadlocal_heap_ works as expected; CreateCacheIfNecessary() was only called a few times in a reasonably long benchmark. (cherry picked from commit 07dcf42f46e211a53f7c33ed634cc5f416a3929f) Review-Url: https://codereview.chromium.org/2640263003 Cr-Original-Commit-Position: refs/heads/master@{#445109} Cr-Commit-Position: refs/branch-heads/2924@{#825} Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059} [modify] https://crrev.com/ea757b6a950c73efcf91e3425d64e75cf2793beb/third_party/tcmalloc/README.chromium [modify] https://crrev.com/ea757b6a950c73efcf91e3425d64e75cf2793beb/third_party/tcmalloc/chromium/src/thread_cache.cc [modify] https://crrev.com/ea757b6a950c73efcf91e3425d64e75cf2793beb/third_party/tcmalloc/chromium/src/thread_cache.h
,
Jan 21 2017
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 21 2017
1/20/17 at 5pm, Luis said: > the fix has been put into R56, R57 and R58(ToT) Chrome. > We will be monitoring the release branch testing this weekend. > > So, hopefully everything will be ok and we will deliver on 56.
,
Jan 23 2017
interestingly, this change has only gone through in R56. https://chromium.googlesource.com/chromium/src/+log/56.0.2924.70..56.0.2924.71?pretty=fuller&n=10000 and after that testing on 56 look ok. however, 58 is totally broken in the PFQ. The breakage is not related to our change. I filed: https://bugs.chromium.org/p/chromium/issues/detail?id=683828 for the R58 failure. And I dont see the change in R57 yet (according to goldeneye)
,
Jan 23 2017
Might be missing from M57 due to some branching issues. Hopefully will be sorted out today.
,
Jan 25 2017
,
Jan 25 2017
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 25 2017
,
Aug 1 2017
,
Aug 3 2017
Closing. Please reopen it if its not fixed. Thanks!
,
Sep 27
Any chance of people still remembering what exactly was the "gcc bug" mentioned above ? |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by laszio@chromium.org
, Jan 19 2017