[compiler-rt] r352057 - [scudo] Tuning changes based on feedback from current use
Yvan Roux via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 25 09:17:53 PST 2019
On Fri, 25 Jan 2019 at 18:03, Kostya Kortchinsky <kostyak at google.com> wrote:
>
> No problem, sorry for the breakage.
> Sent out https://reviews.llvm.org/D57241 for review, not sure if you can test it beforehand or we should land it & wait for the bots.
I've accepted it and will check the bots status
> On Fri, Jan 25, 2019 at 9:00 AM Yvan Roux <yvan.roux at linaro.org> wrote:
>>
>> On Fri, 25 Jan 2019 at 17:15, Kostya Kortchinsky <kostyak at google.com> wrote:
>> >
>> > On it, thanks!
>>
>> Thanks Kostya.
>>
>> >
>> > On Fri, Jan 25, 2019 at 6:57 AM Yvan Roux <yvan.roux at linaro.org> wrote:
>> >>
>> >> Hi Kostya
>> >>
>> >> This change broke armv7 bot, logs are available here:
>> >> http://lab.llvm.org:8011/builders/clang-cmake-armv7-full/builds/4600/steps/ninja%20check%201/logs/FAIL%3A%20Scudo-armhf%3A%3A%20rss.c
>> >>
>> >> Cheers,
>> >> Yvan
>> >>
>> >>
>> >> On Thu, 24 Jan 2019 at 16:56, Kostya Kortchinsky via llvm-commits
>> >> <llvm-commits at lists.llvm.org> wrote:
>> >> >
>> >> > Author: cryptoad
>> >> > Date: Thu Jan 24 07:56:54 2019
>> >> > New Revision: 352057
>> >> >
>> >> > URL: http://llvm.org/viewvc/llvm-project?rev=352057&view=rev
>> >> > Log:
>> >> > [scudo] Tuning changes based on feedback from current use
>> >> >
>> >> > Summary:
>> >> > This tunes several of the default parameters used within the allocator:
>> >> > - disable the deallocation type mismatch on Android by default; this
>> >> > was causing too many issues with third party libraries;
>> >> > - change the default `SizeClassMap` to `Dense`, it caches less entries
>> >> > and is way more memory efficient overall;
>> >> > - relax the timing of the RSS checks, 10 times per second was too much,
>> >> > lower it to 4 times (every 250ms), and update the test so that it
>> >> > passes with the new default.
>> >> >
>> >> > Reviewers: eugenis
>> >> >
>> >> > Reviewed By: eugenis
>> >> >
>> >> > Subscribers: srhines, delcypher, #sanitizers, llvm-commits
>> >> >
>> >> > Differential Revision: https://reviews.llvm.org/D57116
>> >> >
>> >> > Modified:
>> >> > compiler-rt/trunk/lib/scudo/scudo_allocator.cpp
>> >> > compiler-rt/trunk/lib/scudo/scudo_flags.inc
>> >> > compiler-rt/trunk/lib/scudo/scudo_platform.h
>> >> > compiler-rt/trunk/test/scudo/rss.c
>> >> >
>> >> > Modified: compiler-rt/trunk/lib/scudo/scudo_allocator.cpp
>> >> > URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/scudo/scudo_allocator.cpp?rev=352057&r1=352056&r2=352057&view=diff
>> >> > ==============================================================================
>> >> > --- compiler-rt/trunk/lib/scudo/scudo_allocator.cpp (original)
>> >> > +++ compiler-rt/trunk/lib/scudo/scudo_allocator.cpp Thu Jan 24 07:56:54 2019
>> >> > @@ -587,11 +587,11 @@ NOINLINE void Allocator::performSanityCh
>> >> > }
>> >> >
>> >> > // Opportunistic RSS limit check. This will update the RSS limit status, if
>> >> > -// it can, every 100ms, otherwise it will just return the current one.
>> >> > +// it can, every 250ms, otherwise it will just return the current one.
>> >> > NOINLINE bool Allocator::isRssLimitExceeded() {
>> >> > u64 LastCheck = atomic_load_relaxed(&RssLastCheckedAtNS);
>> >> > const u64 CurrentCheck = MonotonicNanoTime();
>> >> > - if (LIKELY(CurrentCheck < LastCheck + (100ULL * 1000000ULL)))
>> >> > + if (LIKELY(CurrentCheck < LastCheck + (250ULL * 1000000ULL)))
>> >> > return atomic_load_relaxed(&RssLimitExceeded);
>> >> > if (!atomic_compare_exchange_weak(&RssLastCheckedAtNS, &LastCheck,
>> >> > CurrentCheck, memory_order_relaxed))
>> >> >
>> >> > Modified: compiler-rt/trunk/lib/scudo/scudo_flags.inc
>> >> > URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/scudo/scudo_flags.inc?rev=352057&r1=352056&r2=352057&view=diff
>> >> > ==============================================================================
>> >> > --- compiler-rt/trunk/lib/scudo/scudo_flags.inc (original)
>> >> > +++ compiler-rt/trunk/lib/scudo/scudo_flags.inc Thu Jan 24 07:56:54 2019
>> >> > @@ -36,7 +36,9 @@ SCUDO_FLAG(int, QuarantineChunksUpToSize
>> >> > "Size in bytes up to which chunks will be quarantined (if lower than"
>> >> > "or equal to). Defaults to 256 (32-bit) or 2048 (64-bit)")
>> >> >
>> >> > -SCUDO_FLAG(bool, DeallocationTypeMismatch, true,
>> >> > +// Disable the deallocation type check by default on Android, it causes too many
>> >> > +// issues with third party libraries.
>> >> > +SCUDO_FLAG(bool, DeallocationTypeMismatch, !SANITIZER_ANDROID,
>> >> > "Report errors on malloc/delete, new/free, new/delete[], etc.")
>> >> >
>> >> > SCUDO_FLAG(bool, DeleteSizeMismatch, true,
>> >> >
>> >> > Modified: compiler-rt/trunk/lib/scudo/scudo_platform.h
>> >> > URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/scudo/scudo_platform.h?rev=352057&r1=352056&r2=352057&view=diff
>> >> > ==============================================================================
>> >> > --- compiler-rt/trunk/lib/scudo/scudo_platform.h (original)
>> >> > +++ compiler-rt/trunk/lib/scudo/scudo_platform.h Thu Jan 24 07:56:54 2019
>> >> > @@ -79,7 +79,7 @@ const uptr RegionSizeLog = SANITIZER_AND
>> >> > #endif // SANITIZER_CAN_USE_ALLOCATOR64
>> >> >
>> >> > #if !defined(SCUDO_SIZE_CLASS_MAP)
>> >> > -# define SCUDO_SIZE_CLASS_MAP Default
>> >> > +# define SCUDO_SIZE_CLASS_MAP Dense
>> >> > #endif
>> >> >
>> >> > #define SIZE_CLASS_MAP_TYPE SIZE_CLASS_MAP_TYPE_(SCUDO_SIZE_CLASS_MAP)
>> >> >
>> >> > Modified: compiler-rt/trunk/test/scudo/rss.c
>> >> > URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/scudo/rss.c?rev=352057&r1=352056&r2=352057&view=diff
>> >> > ==============================================================================
>> >> > --- compiler-rt/trunk/test/scudo/rss.c (original)
>> >> > +++ compiler-rt/trunk/test/scudo/rss.c Thu Jan 24 07:56:54 2019
>> >> > @@ -1,15 +1,15 @@
>> >> > // RUN: %clang_scudo %s -o %t
>> >> > // RUN: %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-nolimit
>> >> > -// RUN: %env_scudo_opts="soft_rss_limit_mb=256" %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-nolimit
>> >> > -// RUN: %env_scudo_opts="hard_rss_limit_mb=256" %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-nolimit
>> >> > -// RUN: %env_scudo_opts="soft_rss_limit_mb=64:allocator_may_return_null=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit
>> >> > -// RUN: %env_scudo_opts="soft_rss_limit_mb=64:allocator_may_return_null=1" %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit-returnnull
>> >> > -// RUN: %env_scudo_opts="soft_rss_limit_mb=64:allocator_may_return_null=0:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit
>> >> > -// RUN: %env_scudo_opts="soft_rss_limit_mb=64:allocator_may_return_null=1:can_use_proc_maps_statm=0" %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit-returnnull
>> >> > -// RUN: %env_scudo_opts="hard_rss_limit_mb=64:allocator_may_return_null=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit
>> >> > -// RUN: %env_scudo_opts="hard_rss_limit_mb=64:allocator_may_return_null=1" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit
>> >> > -// RUN: %env_scudo_opts="hard_rss_limit_mb=64:allocator_may_return_null=0:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit
>> >> > -// RUN: %env_scudo_opts="hard_rss_limit_mb=64:allocator_may_return_null=1:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit
>> >> > +// RUN: %env_scudo_opts="soft_rss_limit_mb=128" %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-nolimit
>> >> > +// RUN: %env_scudo_opts="hard_rss_limit_mb=128" %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-nolimit
>> >> > +// RUN: %env_scudo_opts="soft_rss_limit_mb=32:allocator_may_return_null=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit
>> >> > +// RUN: %env_scudo_opts="soft_rss_limit_mb=32:allocator_may_return_null=1" %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit-returnnull
>> >> > +// RUN: %env_scudo_opts="soft_rss_limit_mb=32:allocator_may_return_null=0:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit
>> >> > +// RUN: %env_scudo_opts="soft_rss_limit_mb=32:allocator_may_return_null=1:can_use_proc_maps_statm=0" %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-softlimit-returnnull
>> >> > +// RUN: %env_scudo_opts="hard_rss_limit_mb=32:allocator_may_return_null=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit
>> >> > +// RUN: %env_scudo_opts="hard_rss_limit_mb=32:allocator_may_return_null=1" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit
>> >> > +// RUN: %env_scudo_opts="hard_rss_limit_mb=32:allocator_may_return_null=0:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit
>> >> > +// RUN: %env_scudo_opts="hard_rss_limit_mb=32:allocator_may_return_null=1:can_use_proc_maps_statm=0" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-hardlimit
>> >> >
>> >> > // Tests that the soft and hard RSS limits work as intended. Without limit or
>> >> > // with a high limit, the test should pass without any malloc returning NULL or
>> >> > @@ -22,7 +22,7 @@
>> >> > #include <string.h>
>> >> > #include <unistd.h>
>> >> >
>> >> > -static const size_t kNumAllocs = 128;
>> >> > +static const size_t kNumAllocs = 64;
>> >> > static const size_t kAllocSize = 1 << 20; // 1MB.
>> >> >
>> >> > static void *allocs[kNumAllocs];
>> >> > @@ -31,7 +31,7 @@ int main(int argc, char *argv[]) {
>> >> > int returned_null = 0;
>> >> > for (int i = 0; i < kNumAllocs; i++) {
>> >> > if ((i & 0xf) == 0)
>> >> > - usleep(50000);
>> >> > + usleep(100000);
>> >> > allocs[i] = malloc(kAllocSize);
>> >> > if (allocs[i])
>> >> > memset(allocs[i], 0xff, kAllocSize); // Dirty the pages.
>> >> >
>> >> >
>> >> > _______________________________________________
>> >> > llvm-commits mailing list
>> >> > llvm-commits at lists.llvm.org
>> >> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list