[compiler-rt] r352057 - [scudo] Tuning changes based on feedback from current use

Kostya Kortchinsky via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 25 08:15:37 PST 2019


On it, thanks!

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190125/f38fd7c3/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4845 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190125/f38fd7c3/attachment.bin>


More information about the llvm-commits mailing list