impala-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ph...@apache.org
Subject [02/19] impala git commit: IMPALA-5528: Bump total thread cache size when KRPC is enabled
Date Fri, 02 Feb 2018 18:51:26 GMT
IMPALA-5528: Bump total thread cache size when KRPC is enabled

KRPC in general tends to put more pressure on the thread
caches due to allocations of more small objects (i.e. <1MB).
While some of them are being addressed in KUDU-1865, it's shown
that the following TCMalloc workarounds will provide reasonable
performance with KRPC:

- TCMALLOC_TRANSFER_NUM_OBJ:
   - maximum number of object per classe type to transfer between
     thread and central caches.
   - the default value of 512 in 2.5.2 seems to cause the spin lock
     in the central cache to be held for too long with KRPC. 2.5.90
     and latter reverts this value to 32 by default.

- TCMALLOC_MAX_TOTAL_THREAD_CACHE_BYTES
  - total amount of memory allocated to all thread caches in bytes
  - the default value is 32MB. We need to bump it to 1GB which is the
    internal cap in TCMalloc.

This change bumps the thread cache sizes to 1GB when KRPC is enabled and
FLAGS_TCMALLOC_MAX_TOTAL_THREAD_CACHE_BYTES has the default value of 0.

GPerfTools/TCMalloc needs to be upgraded to 2.5.90 or above to pick up
the change of the default value of TCMALLOC_TRANSFER_NUM_OBJ. Previous
attempt to upgrade GPerfTools to 2.6.3 failed on certain platforms (IMPALA-6414).

Also fixes a couple of BE tests to initialize the test environment properly.

Change-Id: I8407528942051fb19a0222491347c9090d4b4b8d
Reviewed-on: http://gerrit.cloudera.org:8080/9058
Reviewed-by: Michael Ho <kwho@cloudera.com>
Reviewed-by: Sailesh Mukil <sailesh@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/d24f23b1
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/d24f23b1
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/d24f23b1

Branch: refs/heads/2.x
Commit: d24f23b19edbcb2dd06468a93661d772a2fae4e6
Parents: eec5b48
Author: Michael Ho <kwho@cloudera.com>
Authored: Wed Jan 17 19:33:34 2018 -0800
Committer: Impala Public Jenkins <impala-public-jenkins@gerrit.cloudera.org>
Committed: Fri Feb 2 01:10:14 2018 +0000

----------------------------------------------------------------------
 .../runtime/bufferpool/buffer-allocator-test.cc |  7 +++++++
 be/src/runtime/bufferpool/free-list-test.cc     | 13 +++++++++++-
 be/src/runtime/bufferpool/suballocator-test.cc  | 13 +++++++++++-
 be/src/runtime/exec-env.cc                      | 22 +++++++++++---------
 4 files changed, 43 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/d24f23b1/be/src/runtime/bufferpool/buffer-allocator-test.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/bufferpool/buffer-allocator-test.cc b/be/src/runtime/bufferpool/buffer-allocator-test.cc
index 21a9c08..2b87278 100644
--- a/be/src/runtime/bufferpool/buffer-allocator-test.cc
+++ b/be/src/runtime/bufferpool/buffer-allocator-test.cc
@@ -22,6 +22,8 @@
 #include "runtime/bufferpool/buffer-pool-internal.h"
 #include "runtime/bufferpool/buffer-pool.h"
 #include "runtime/bufferpool/system-allocator.h"
+#include "runtime/test-env.h"
+#include "service/fe-support.h"
 #include "testutil/cpu-util.h"
 #include "testutil/gtest-util.h"
 #include "util/cpu-info.h"
@@ -39,6 +41,8 @@ using BufferHandle = BufferPool::BufferHandle;
 class BufferAllocatorTest : public ::testing::Test {
  public:
   virtual void SetUp() {
+    test_env_.reset(new TestEnv);
+    ASSERT_OK(test_env_->Init());
     dummy_pool_ = obj_pool_.Add(new BufferPool(1, 0, 0));
     dummy_reservation_.InitRootTracker(nullptr, 0);
     ASSERT_OK(dummy_pool_->RegisterClient("", nullptr, &dummy_reservation_, nullptr,
0,
@@ -59,6 +63,8 @@ class BufferAllocatorTest : public ::testing::Test {
   /// The minimum buffer size used in most tests.
   const static int64_t TEST_BUFFER_LEN = 1024;
 
+  boost::scoped_ptr<TestEnv> test_env_;
+
   ObjectPool obj_pool_;
 
   /// Need a dummy pool and client to pass around. We bypass the reservation mechanisms
@@ -200,6 +206,7 @@ TEST_F(SystemAllocatorTest, LargeAllocFailure) {
 int main(int argc, char** argv) {
   ::testing::InitGoogleTest(&argc, argv);
   impala::InitCommonRuntime(argc, argv, true, impala::TestInfo::BE_TEST);
+  impala::InitFeSupport();
   int result = 0;
   for (bool mmap : {false, true}) {
     for (bool madvise : {false, true}) {

http://git-wip-us.apache.org/repos/asf/impala/blob/d24f23b1/be/src/runtime/bufferpool/free-list-test.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/bufferpool/free-list-test.cc b/be/src/runtime/bufferpool/free-list-test.cc
index 7cb80b4..d3c4c9a 100644
--- a/be/src/runtime/bufferpool/free-list-test.cc
+++ b/be/src/runtime/bufferpool/free-list-test.cc
@@ -21,6 +21,8 @@
 #include "common/object-pool.h"
 #include "runtime/bufferpool/free-list.h"
 #include "runtime/bufferpool/system-allocator.h"
+#include "runtime/test-env.h"
+#include "service/fe-support.h"
 #include "testutil/gtest-util.h"
 #include "testutil/rand-util.h"
 
@@ -31,6 +33,8 @@ namespace impala {
 class FreeListTest : public ::testing::Test {
  protected:
   virtual void SetUp() override {
+    test_env_.reset(new TestEnv);
+    ASSERT_OK(test_env_->Init());
     allocator_ = obj_pool_.Add(new SystemAllocator(MIN_BUFFER_LEN));
     RandTestUtil::SeedRng("FREE_LIST_TEST_SEED", &rng_);
   }
@@ -71,6 +75,8 @@ class FreeListTest : public ::testing::Test {
 
   const static int MIN_BUFFER_LEN = 1024;
 
+  boost::scoped_ptr<TestEnv> test_env_;
+
   /// Per-test random number generator. Seeded before every test.
   std::mt19937 rng_;
 
@@ -157,4 +163,9 @@ TEST_F(FreeListTest, ReturnOrder) {
 }
 }
 
-IMPALA_TEST_MAIN();
+int main(int argc, char** argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  impala::InitCommonRuntime(argc, argv, true, impala::TestInfo::BE_TEST);
+  impala::InitFeSupport();
+  return RUN_ALL_TESTS();
+}

http://git-wip-us.apache.org/repos/asf/impala/blob/d24f23b1/be/src/runtime/bufferpool/suballocator-test.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/bufferpool/suballocator-test.cc b/be/src/runtime/bufferpool/suballocator-test.cc
index 6cd53fb..470b065 100644
--- a/be/src/runtime/bufferpool/suballocator-test.cc
+++ b/be/src/runtime/bufferpool/suballocator-test.cc
@@ -27,6 +27,8 @@
 #include "common/object-pool.h"
 #include "runtime/bufferpool/reservation-tracker.h"
 #include "runtime/bufferpool/suballocator.h"
+#include "runtime/test-env.h"
+#include "service/fe-support.h"
 #include "testutil/death-test-util.h"
 #include "testutil/gtest-util.h"
 #include "testutil/rand-util.h"
@@ -44,6 +46,8 @@ namespace impala {
 class SuballocatorTest : public ::testing::Test {
  public:
   virtual void SetUp() override {
+    test_env_.reset(new TestEnv);
+    ASSERT_OK(test_env_->Init());
     RandTestUtil::SeedRng("SUBALLOCATOR_TEST_SEED", &rng_);
     profile_ = RuntimeProfile::Create(&obj_pool_, "test profile");
   }
@@ -111,6 +115,8 @@ class SuballocatorTest : public ::testing::Test {
   /// Clients for the buffer pool. Deregistered and freed after every test.
   vector<unique_ptr<BufferPool::ClientHandle>> clients_;
 
+  boost::scoped_ptr<TestEnv> test_env_;
+
   /// Global profile - recreated for every test.
   RuntimeProfile* profile_;
 
@@ -362,4 +368,9 @@ void SuballocatorTest::AssertMemoryValid(
 }
 }
 
-IMPALA_TEST_MAIN();
+int main(int argc, char** argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  impala::InitCommonRuntime(argc, argv, true, impala::TestInfo::BE_TEST);
+  impala::InitFeSupport();
+  return RUN_ALL_TESTS();
+}

http://git-wip-us.apache.org/repos/asf/impala/blob/d24f23b1/be/src/runtime/exec-env.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/exec-env.cc b/be/src/runtime/exec-env.cc
index f191921..0e5b0f6 100644
--- a/be/src/runtime/exec-env.cc
+++ b/be/src/runtime/exec-env.cc
@@ -271,16 +271,6 @@ Status ExecEnv::Init() {
           "bytes value or percentage: $0", FLAGS_mem_limit));
   }
 
-#if !defined(ADDRESS_SANITIZER) && !defined(THREAD_SANITIZER)
-  // Aggressive decommit is required so that unused pages in the TCMalloc page heap are
-  // not backed by physical pages and do not contribute towards memory consumption.
-  // Enable it in TCMalloc before InitBufferPool().
-  if (!MallocExtension::instance()->SetNumericProperty(
-          "tcmalloc.aggressive_memory_decommit", 1)) {
-    return Status("Failed to enable TCMalloc aggressive decommit.");
-  }
-#endif
-
   if (!BitUtil::IsPowerOf2(FLAGS_min_buffer_size)) {
     return Status(Substitute(
         "--min_buffer_size must be a power-of-two: $0", FLAGS_min_buffer_size));
@@ -320,6 +310,11 @@ Status ExecEnv::Init() {
         FLAGS_datastream_service_num_svc_threads : CpuInfo::num_cores();
     RETURN_IF_ERROR(rpc_mgr_->RegisterService(num_svc_threads,
         FLAGS_datastream_service_queue_depth, move(data_svc)));
+    // Bump thread cache to 1GB to reduce contention for TCMalloc central
+    // list's spinlock.
+    if (FLAGS_tcmalloc_max_total_thread_cache_bytes == 0) {
+      FLAGS_tcmalloc_max_total_thread_cache_bytes = 1 << 30;
+    }
   }
 
   mem_tracker_.reset(
@@ -432,6 +427,13 @@ Status ExecEnv::StartKrpcService() {
 
 void ExecEnv::InitBufferPool(int64_t min_buffer_size, int64_t capacity,
     int64_t clean_pages_limit) {
+#if !defined(ADDRESS_SANITIZER) && !defined(THREAD_SANITIZER)
+  // Aggressive decommit is required so that unused pages in the TCMalloc page heap are
+  // not backed by physical pages and do not contribute towards memory consumption.
+  // Enable it in TCMalloc before InitBufferPool().
+  MallocExtension::instance()->SetNumericProperty(
+      "tcmalloc.aggressive_memory_decommit", 1);
+#endif
   buffer_pool_.reset(new BufferPool(min_buffer_size, capacity, clean_pages_limit));
   buffer_reservation_.reset(new ReservationTracker());
   buffer_reservation_->InitRootTracker(nullptr, capacity);


Mime
View raw message