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

Issue 1881523003: Revert of Enable allocator shim for Android (Closed)

Created:
4 years, 8 months ago by Nico
Modified:
4 years, 8 months ago
CC:
chromium-reviews, DmitrySkiba, kraynov, ssid
Base URL:
https://chromium.googlesource.com/chromium/src.git@androidexp
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Enable allocator shim for Android (patchset #2 id:20001 of https://codereview.chromium.org/1875043003/ ) Reason for revert: Looks like this breaks android clang builds. 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} TBR=dalecurtis@chromium.org,primiano@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=550886, 598075 Committed: https://crrev.com/6fdf2cfe0228e221c2cd738edabf3514459d6a89 Cr-Commit-Position: refs/heads/master@{#386408}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -3 lines) Patch
M base/allocator/README.md View 1 chunk +1 line, -1 line 0 comments Download
M build/common.gypi View 1 chunk +1 line, -1 line 0 comments Download
M build/config/allocator.gni View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 5 (1 generated)
Nico
Created Revert of Enable allocator shim for Android
4 years, 8 months ago (2016-04-11 18:11:15 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1881523003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1881523003/1
4 years, 8 months ago (2016-04-11 18:11:42 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 8 months ago (2016-04-11 18:13:14 UTC) #3
commit-bot: I haz the power
4 years, 8 months ago (2016-04-11 18:15:11 UTC) #5
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/6fdf2cfe0228e221c2cd738edabf3514459d6a89
Cr-Commit-Position: refs/heads/master@{#386408}

Powered by Google App Engine
This is Rietveld 408576698