[GitHub] [trafficserver] masaori335 commented on a change in pull request #7501: JeMalloc performance improvments

[GitBox <gi...@apache.org>]

masaori335 commented on a change in pull request #7501:
URL: https://github.com/apache/trafficserver/pull/7501#discussion_r662029113



##########
File path: src/tscore/JeAllocator.cc
##########
@@ -52,44 +50,52 @@ JemallocNodumpAllocator::alloc(extent_hooks_t *extent, void *new_addr, size_t si
   return result;
 }
 
-#endif /* JEMALLOC_NODUMP_ALLOCATOR_SUPPORTED */
-
-bool
-JemallocNodumpAllocator::extend_and_setup_arena()
+unsigned int
+JemallocNodumpAllocator::extend_and_setup_arena(pthread_t thread_id)
 {
-#ifdef JEMALLOC_NODUMP_ALLOCATOR_SUPPORTED
-  size_t arena_index_len_ = sizeof(arena_index_);
-  if (auto ret = mallctl("arenas.create", &arena_index_, &arena_index_len_, nullptr, 0)) {
+  unsigned int arena_id;
+  size_t arena_id_len = sizeof(arena_id);
+  if (auto ret = mallctl("arenas.create", &arena_id, &arena_id_len, nullptr, 0)) {
     ink_abort("Unable to extend arena: %s", std::strerror(ret));
   }
-  flags_ = MALLOCX_ARENA(arena_index_) | MALLOCX_TCACHE_NONE;
+
+  int flags = MALLOCX_ARENA(arena_id) | MALLOCX_TCACHE_NONE;
 
   // Read the existing hooks
-  const auto key = "arena." + std::to_string(arena_index_) + ".extent_hooks";
+  const auto key = "arena." + std::to_string(arena_id) + ".extent_hooks";
   extent_hooks_t *hooks;
   size_t hooks_len = sizeof(hooks);
   if (auto ret = mallctl(key.c_str(), &hooks, &hooks_len, nullptr, 0)) {
     ink_abort("Unable to get the hooks: %s", std::strerror(ret));
   }
-  if (original_alloc_ == nullptr) {
-    original_alloc_ = hooks->alloc;
-  } else {
-    ink_release_assert(original_alloc_ == hooks->alloc);
-  }
 
-  // Set the custom hook
-  extent_hooks_             = *hooks;
-  extent_hooks_.alloc       = &JemallocNodumpAllocator::alloc;
-  extent_hooks_t *new_hooks = &extent_hooks_;
-  if (auto ret = mallctl(key.c_str(), nullptr, nullptr, &new_hooks, sizeof(new_hooks))) {
-    ink_abort("Unable to set the hooks: %s", std::strerror(ret));
+  {
+    std::unique_lock lock(je_mutex);
+    if (original_alloc == nullptr) {
+      original_alloc = hooks->alloc;
+    } else {
+      ink_release_assert(original_alloc == hooks->alloc);
+    }
+
+    // Set the custom hook
+    if (extent_hooks.alloc != &JemallocNodumpAllocator::alloc) {
+      extent_hooks       = *hooks;
+      extent_hooks.alloc = &JemallocNodumpAllocator::alloc;
+    }
+    extent_hooks_t *new_hooks = &extent_hooks;
+    if (auto ret = mallctl(key.c_str(), nullptr, nullptr, &new_hooks, sizeof(new_hooks))) {
+      ink_abort("Unable to set the hooks: %s", std::strerror(ret));
+    }
+
+    Debug("JeAllocator", "arena \"%ud\" created", arena_id);
+
+    arenas[thread_id]     = arena_id;

Review comment:
       This is just a random idea and I'm not sure it makes sense. If the `Thread` class (in the `iocore/eventsystem/I_Thread.h`) has `arena_id` as a member like `ProxyAllocator`s, we'd not need this unordered_map and lock?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org