Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(152)

Issue 2818713003: Revert of tcmalloc: Use C++11 atomics where appropriate. (Closed)

Created:
3 years, 8 months ago by Nico
Modified:
3 years, 8 months ago
Reviewers:
JF
CC:
chromium-reviews, wfh+watch_chromium.org, Dai Mikurube (NOT FULLTIME), vmpstr+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of tcmalloc: Use C++11 atomics where appropriate. (patchset #9 id:160001 of https://codereview.chromium.org/1549913002/ ) Reason for revert: Doesn't build on 32-bit: https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.linux%2FLinux_Builder__dbg__32_%2F66018%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout ../../build/linux/debian_jessie_i386-sysroot/usr/lib/gcc/i586-linux-gnu/4.8/../../../../include/c++/4.8/bits/atomic_base.h:506: error: undefined reference to '__atomic_load_8' ../../build/linux/debian_jessie_i386-sysroot/usr/lib/gcc/i586-linux-gnu/4.8/../../../../include/c++/4.8/bits/atomic_base.h:486: error: undefined reference to '__atomic_store_8' Original issue's description: > tcmalloc: Use C++11 atomics where appropriate. > > Reland now that we have a newer sysroot. > > Ports these CLs to tcmalloc: > https://codereview.chromium.org/636783002/ > https://codereview.chromium.org/1466833002/ (except mac) > > No intended behavior change, but it should remove > the static initializer in atomicops_internals_x86_gcc.h > on Linux. It's also less code. > > BUG=94925, 559247, 572525 > > Committed: https://crrev.com/e13537fe418eff11d3cab9077f6a647d7c74f103 > Cr-Original-Commit-Position: refs/heads/master@{#366904} > Review-Url: https://codereview.chromium.org/1549913002 > Cr-Commit-Position: refs/heads/master@{#464440} > Committed: https://chromium.googlesource.com/chromium/src/+/2f6d8f01d9087e8bebab5b7d2d25b28d657dbbb7 TBR=jfb@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=94925, 559247, 572525 Review-Url: https://codereview.chromium.org/2818713003 Cr-Commit-Position: refs/heads/master@{#464445} Committed: https://chromium.googlesource.com/chromium/src/+/b20672f0a5080fe4318e049bd408db1a01105904

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1601 lines, -225 lines) Patch
M base/allocator/BUILD.gn View 1 chunk +5 lines, -2 lines 0 comments Download
M third_party/tcmalloc/README.chromium View 1 chunk +0 lines, -1 line 0 comments Download
M third_party/tcmalloc/chromium/src/base/atomicops.h View 2 chunks +23 lines, -3 lines 0 comments Download
A third_party/tcmalloc/chromium/src/base/atomicops-internals-arm-generic.h View 1 chunk +235 lines, -0 lines 0 comments Download
A third_party/tcmalloc/chromium/src/base/atomicops-internals-arm-v6plus.h View 1 chunk +369 lines, -0 lines 0 comments Download
A third_party/tcmalloc/chromium/src/base/atomicops-internals-linuxppc.h View 1 chunk +416 lines, -0 lines 0 comments Download
A third_party/tcmalloc/chromium/src/base/atomicops-internals-x86.h View 1 chunk +428 lines, -0 lines 0 comments Download
A third_party/tcmalloc/chromium/src/base/atomicops-internals-x86.cc View 1 chunk +125 lines, -0 lines 0 comments Download
D third_party/tcmalloc/chromium/src/base/atomicops_internals_portable.h View 1 chunk +0 lines, -219 lines 0 comments Download

Messages

Total messages: 7 (3 generated)
Nico
Created Revert of tcmalloc: Use C++11 atomics where appropriate.
3 years, 8 months ago (2017-04-13 16:59:36 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2818713003/1
3 years, 8 months ago (2017-04-13 17:00:19 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/b20672f0a5080fe4318e049bd408db1a01105904
3 years, 8 months ago (2017-04-13 17:02:57 UTC) #6
JF
3 years, 8 months ago (2017-04-13 17:07:10 UTC) #7
Message was sent while issue was closed.
This is caused by the toolchain conservatively assuming that CMPXCHG8B isn't
available on all x86 machines.

Is Chrome supported on those older CPUs?

A while ago I ran a query against all crash reports to see which CPUs we
actually saw in the wild, to determine support. Maybe it would help quantify
this question.

Powered by Google App Engine
This is Rietveld 408576698