Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1875173002:
Reland of Enable allocator shim for Android (https://codereview.chromium.org/1875043003/) (Closed)
|
Created:
4 years, 8 months ago by Primiano Tucci (use gerrit) Modified:
4 years, 8 months ago Reviewers:
Nico CC:
chromium-reviews, wfh+watch_chromium.org, Dai Mikurube (NOT FULLTIME), DmitrySkiba, ssid, kraynov, DaleCurtis Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionReland of Enable allocator shim for Android (crrev.com/1875043003)
Reason for reland:
The original CL was reverted by crrev.com/1881523003 because it broke
clang builds. This CL contains the fix for clang (see diff between
patch-set 1 and 2).
The culprit of the fix is that LLVM libc++ uses _NOEXCEPT instead
of glibc's __THROW. The absence of it was causing errors like the
following:
../../base/allocator/allocator_shim_override_cpp_symbols.h:20:25: error: function previously declared with an explicit exception specification redeclared with an implicit exception specification [-Werror,-Wimplicit-exception-spec-mismatch]
SHIM_ALWAYS_EXPORT void operator delete(void* p) __THROW
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
CC=dalecurtis@chromium.org
Committed: https://crrev.com/88bf797383cd290d9bd23db0045c5fb23e010beb
Cr-Commit-Position: refs/heads/master@{#386444}
Patch Set 1 : Original CL #Patch Set 2 : Fix for LLVM libc++ #
Messages
Total messages: 15 (7 generated)
Description was changed from ========== Reland of Enable allocator shim for Android (https://codereview.chromium.org/1875043003/ Reason for reland: The original CL was reverted by crrev.com/1881523003 because it broke clang builds. This CL contains the fix for clang (see diff between patch-set 1 and 2) 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 CC=dalecurtis@chromium.org ========== to ========== Reland of Enable allocator shim for Android (https://codereview.chromium.org/1875043003/ Reason for reland: The original CL was reverted by crrev.com/1881523003 because it broke clang builds. This CL contains the fix for clang (see diff between patch-set 1 and 2). The culprit of the fix is that LLVM libc++ uses _NOEXCEPT instead of glibc's __THROW. The absence of it was causing errors like the following: ../../base/allocator/allocator_shim_override_cpp_symbols.h:20:25: error: function previously declared with an explicit exception specification redeclared with an implicit exception specification [-Werror,-Wimplicit-exception-spec-mismatch] SHIM_ALWAYS_EXPORT void operator delete(void* p) __THROW 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 CC=dalecurtis@chromium.org ==========
primiano@chromium.org changed reviewers: + thakis@chromium.org
Nico, PTAL again? Well this was quick. Attempt number #2 (I manually reprod and fixed the issue by building libbase.so locally with clang=1. I am now testing a full chrome_public_apk build with)
lgtm
Description was changed from ========== Reland of Enable allocator shim for Android (https://codereview.chromium.org/1875043003/ Reason for reland: The original CL was reverted by crrev.com/1881523003 because it broke clang builds. This CL contains the fix for clang (see diff between patch-set 1 and 2). The culprit of the fix is that LLVM libc++ uses _NOEXCEPT instead of glibc's __THROW. The absence of it was causing errors like the following: ../../base/allocator/allocator_shim_override_cpp_symbols.h:20:25: error: function previously declared with an explicit exception specification redeclared with an implicit exception specification [-Werror,-Wimplicit-exception-spec-mismatch] SHIM_ALWAYS_EXPORT void operator delete(void* p) __THROW 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 CC=dalecurtis@chromium.org ========== to ========== Reland of Enable allocator shim for Android (crrev.com/1875043003) Reason for reland: The original CL was reverted by crrev.com/1881523003 because it broke clang builds. This CL contains the fix for clang (see diff between patch-set 1 and 2). The culprit of the fix is that LLVM libc++ uses _NOEXCEPT instead of glibc's __THROW. The absence of it was causing errors like the following: ../../base/allocator/allocator_shim_override_cpp_symbols.h:20:25: error: function previously declared with an explicit exception specification redeclared with an implicit exception specification [-Werror,-Wimplicit-exception-spec-mismatch] SHIM_ALWAYS_EXPORT void operator delete(void* p) __THROW 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 CC=dalecurtis@chromium.org ==========
The CQ bit was checked by primiano@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1875173002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1875173002/20001
Message was sent while issue was closed.
Description was changed from ========== Reland of Enable allocator shim for Android (crrev.com/1875043003) Reason for reland: The original CL was reverted by crrev.com/1881523003 because it broke clang builds. This CL contains the fix for clang (see diff between patch-set 1 and 2). The culprit of the fix is that LLVM libc++ uses _NOEXCEPT instead of glibc's __THROW. The absence of it was causing errors like the following: ../../base/allocator/allocator_shim_override_cpp_symbols.h:20:25: error: function previously declared with an explicit exception specification redeclared with an implicit exception specification [-Werror,-Wimplicit-exception-spec-mismatch] SHIM_ALWAYS_EXPORT void operator delete(void* p) __THROW 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 CC=dalecurtis@chromium.org ========== to ========== Reland of Enable allocator shim for Android (crrev.com/1875043003) Reason for reland: The original CL was reverted by crrev.com/1881523003 because it broke clang builds. This CL contains the fix for clang (see diff between patch-set 1 and 2). The culprit of the fix is that LLVM libc++ uses _NOEXCEPT instead of glibc's __THROW. The absence of it was causing errors like the following: ../../base/allocator/allocator_shim_override_cpp_symbols.h:20:25: error: function previously declared with an explicit exception specification redeclared with an implicit exception specification [-Werror,-Wimplicit-exception-spec-mismatch] SHIM_ALWAYS_EXPORT void operator delete(void* p) __THROW 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 CC=dalecurtis@chromium.org ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Reland of Enable allocator shim for Android (crrev.com/1875043003) Reason for reland: The original CL was reverted by crrev.com/1881523003 because it broke clang builds. This CL contains the fix for clang (see diff between patch-set 1 and 2). The culprit of the fix is that LLVM libc++ uses _NOEXCEPT instead of glibc's __THROW. The absence of it was causing errors like the following: ../../base/allocator/allocator_shim_override_cpp_symbols.h:20:25: error: function previously declared with an explicit exception specification redeclared with an implicit exception specification [-Werror,-Wimplicit-exception-spec-mismatch] SHIM_ALWAYS_EXPORT void operator delete(void* p) __THROW 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 CC=dalecurtis@chromium.org ========== to ========== Reland of Enable allocator shim for Android (crrev.com/1875043003) Reason for reland: The original CL was reverted by crrev.com/1881523003 because it broke clang builds. This CL contains the fix for clang (see diff between patch-set 1 and 2). The culprit of the fix is that LLVM libc++ uses _NOEXCEPT instead of glibc's __THROW. The absence of it was causing errors like the following: ../../base/allocator/allocator_shim_override_cpp_symbols.h:20:25: error: function previously declared with an explicit exception specification redeclared with an implicit exception specification [-Werror,-Wimplicit-exception-spec-mismatch] SHIM_ALWAYS_EXPORT void operator delete(void* p) __THROW 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 CC=dalecurtis@chromium.org Committed: https://crrev.com/88bf797383cd290d9bd23db0045c5fb23e010beb Cr-Commit-Position: refs/heads/master@{#386444} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/88bf797383cd290d9bd23db0045c5fb23e010beb Cr-Commit-Position: refs/heads/master@{#386444}
Message was sent while issue was closed.
Description was changed from ========== Reland of Enable allocator shim for Android (crrev.com/1875043003) Reason for reland: The original CL was reverted by crrev.com/1881523003 because it broke clang builds. This CL contains the fix for clang (see diff between patch-set 1 and 2). The culprit of the fix is that LLVM libc++ uses _NOEXCEPT instead of glibc's __THROW. The absence of it was causing errors like the following: ../../base/allocator/allocator_shim_override_cpp_symbols.h:20:25: error: function previously declared with an explicit exception specification redeclared with an implicit exception specification [-Werror,-Wimplicit-exception-spec-mismatch] SHIM_ALWAYS_EXPORT void operator delete(void* p) __THROW 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 CC=dalecurtis@chromium.org Committed: https://crrev.com/88bf797383cd290d9bd23db0045c5fb23e010beb Cr-Commit-Position: refs/heads/master@{#386444} ========== to ========== Reland of Enable allocator shim for Android (crrev.com/1875043003) Reason for reland: The original CL was reverted by crrev.com/1881523003 because it broke clang builds. This CL contains the fix for clang (see diff between patch-set 1 and 2). The culprit of the fix is that LLVM libc++ uses _NOEXCEPT instead of glibc's __THROW. The absence of it was causing errors like the following: ../../base/allocator/allocator_shim_override_cpp_symbols.h:20:25: error: function previously declared with an explicit exception specification redeclared with an implicit exception specification [-Werror,-Wimplicit-exception-spec-mismatch] SHIM_ALWAYS_EXPORT void operator delete(void* p) __THROW 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 CC=dalecurtis@chromium.org Committed: https://crrev.com/88bf797383cd290d9bd23db0045c5fb23e010beb Cr-Commit-Position: refs/heads/master@{#386444} ==========
Message was sent while issue was closed.
Took a look to the perf dashboard and, at least for the major benchmarks I know about, didn't see any regression: https://chromeperf.appspot.com/report?sid=bae6c680b06a1869ef1a79fca7677afb90e... I'll probably regret this early message, but the odds are this might stick (at least from a perf viewpoint, which was what made more more nervous as this is the only platform where we didn't have the extra hop).
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/1884223002/ by pasko@chromium.org. The reason for reverting is: Causes re-entrancy issues in builds instrumented with -finstrument-functions. See: crbug.com/602744.
Message was sent while issue was closed.
> I'll probably regret this early message, but the odds are this might stick +1 to primiano from the past :) |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
