1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

deps: V8: cherry-pick b49206d from upstream

Browse Source 
Original commit message:
  ThreadDataTable: Change global linked list to per-Isolate hash map.

  For use cases with a large number of threads or a large number of
  isolates (or both), ThreadDataTable can be a major performance
  bottleneck due to O(n) lookup time of the linked list. Switching to a
  hash map reduces this to O(1).

  Example 1: Sandstorm.io, a Node.js app that utilizes "fibers", was
  observed spending the majority of CPU time iterating over the
  ThreadDataTable.
  See: https://sandstorm.io/news/2016-09-30-fiber-bomb-debugging-story

  Example 2: Cloudflare's Workers engine, a high-multi-tenancy web
  server framework built on V8 (but not Node), creates large numbers of
  threads and isolates per-process. It saw a 34x improvement in
  throughput when we applied this patch.

  Cloudflare has been using a patch in production since the Worker
  launch which replaces the linked list with a hash map -- but still
  global.

  This commit builds on that but goes further and creates a separate
  hash map and mutex for each isolate, with the table being a member of
  the Isolate class. This avoids any globals and should reduce lock
  contention.

  Bug: v8:5338
  Change-Id: If0d11509afb2e043b888c376e36d3463db931b47
  Reviewed-on: https://chromium-review.googlesource.com/1014407
  Reviewed-by: Yang Guo <yangguo@chromium.org>
  Commit-Queue: Yang Guo <yangguo@chromium.org>
  Cr-Commit-Position: refs/heads/master@{#52753}

PR-URL: https://github.com/nodejs/node/pull/20727
Ref: https://github.com/nodejs/node/issues/20083

Refs: https://github.com/nodejs/node/issues/20083
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
Ali Ijaz Sheikh 2018-05-14 12:17:38 -07:00
parent 2773de52fd
commit 0ebbd764a8
4 changed files with 44 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
common.gypi
View File

@ -27,7 +27,7 @@
# Reset this number to 0 on major V8 upgrades. # Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8. # Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.7', 'v8_embedder_string': '-node.8',
# Enable disassembler for `--print-code` v8 options # Enable disassembler for `--print-code` v8 options
'v8_enable_disassembler': 1, 'v8_enable_disassembler': 1,

82
deps/v8/src/isolate.cc vendored
View File

@ -8,6 +8,7 @@
#include <fstream> // NOLINT(readability/streams) #include <fstream> // NOLINT(readability/streams)
#include <sstream> #include <sstream>
#include <unordered_map>
#include "src/api.h" #include "src/api.h"
#include "src/assembler-inl.h" #include "src/assembler-inl.h"
@ -136,8 +137,6 @@ void ThreadLocalTop::Free() {
base::Thread::LocalStorageKey Isolate::isolate_key_; base::Thread::LocalStorageKey Isolate::isolate_key_;
base::Thread::LocalStorageKey Isolate::thread_id_key_; base::Thread::LocalStorageKey Isolate::thread_id_key_;
base::Thread::LocalStorageKey Isolate::per_isolate_thread_data_key_; base::Thread::LocalStorageKey Isolate::per_isolate_thread_data_key_;
base::LazyMutex Isolate::thread_data_table_mutex_ = LAZY_MUTEX_INITIALIZER;
Isolate::ThreadDataTable* Isolate::thread_data_table_ = nullptr;
base::Atomic32 Isolate::isolate_counter_ = 0; base::Atomic32 Isolate::isolate_counter_ = 0;
#if DEBUG #if DEBUG
base::Atomic32 Isolate::isolate_key_created_ = 0; base::Atomic32 Isolate::isolate_key_created_ = 0;
@ -148,13 +147,13 @@ Isolate::PerIsolateThreadData*
ThreadId thread_id = ThreadId::Current(); ThreadId thread_id = ThreadId::Current();
PerIsolateThreadData* per_thread = nullptr; PerIsolateThreadData* per_thread = nullptr;
{ {
base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer()); base::LockGuard<base::Mutex> lock_guard(&thread_data_table_mutex_);
per_thread = thread_data_table_->Lookup(this, thread_id); per_thread = thread_data_table_.Lookup(thread_id);
if (per_thread == nullptr) { if (per_thread == nullptr) {
per_thread = new PerIsolateThreadData(this, thread_id); per_thread = new PerIsolateThreadData(this, thread_id);
thread_data_table_->Insert(per_thread); thread_data_table_.Insert(per_thread);
} }
DCHECK(thread_data_table_->Lookup(this, thread_id) == per_thread); DCHECK(thread_data_table_.Lookup(thread_id) == per_thread);
} }
return per_thread; return per_thread;
} }
@ -165,12 +164,11 @@ void Isolate::DiscardPerThreadDataForThisThread() {
if (thread_id_int) { if (thread_id_int) {
ThreadId thread_id = ThreadId(thread_id_int); ThreadId thread_id = ThreadId(thread_id_int);
DCHECK(!thread_manager_->mutex_owner_.Equals(thread_id)); DCHECK(!thread_manager_->mutex_owner_.Equals(thread_id));
base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer()); base::LockGuard<base::Mutex> lock_guard(&thread_data_table_mutex_);
PerIsolateThreadData* per_thread = PerIsolateThreadData* per_thread = thread_data_table_.Lookup(thread_id);
thread_data_table_->Lookup(this, thread_id);
if (per_thread) { if (per_thread) {
DCHECK(!per_thread->thread_state_); DCHECK(!per_thread->thread_state_);
thread_data_table_->Remove(per_thread); thread_data_table_.Remove(per_thread);
} }
} }
} }
@ -186,23 +184,20 @@ Isolate::PerIsolateThreadData* Isolate::FindPerThreadDataForThread(
ThreadId thread_id) { ThreadId thread_id) {
PerIsolateThreadData* per_thread = nullptr; PerIsolateThreadData* per_thread = nullptr;
{ {
base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer()); base::LockGuard<base::Mutex> lock_guard(&thread_data_table_mutex_);
per_thread = thread_data_table_->Lookup(this, thread_id); per_thread = thread_data_table_.Lookup(thread_id);
} }
return per_thread; return per_thread;
} }
void Isolate::InitializeOncePerProcess() { void Isolate::InitializeOncePerProcess() {
base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer());
CHECK_NULL(thread_data_table_);
isolate_key_ = base::Thread::CreateThreadLocalKey(); isolate_key_ = base::Thread::CreateThreadLocalKey();
#if DEBUG #if DEBUG
base::Relaxed_Store(&isolate_key_created_, 1); base::Relaxed_Store(&isolate_key_created_, 1);
#endif #endif
thread_id_key_ = base::Thread::CreateThreadLocalKey(); thread_id_key_ = base::Thread::CreateThreadLocalKey();
per_isolate_thread_data_key_ = base::Thread::CreateThreadLocalKey(); per_isolate_thread_data_key_ = base::Thread::CreateThreadLocalKey();
thread_data_table_ = new Isolate::ThreadDataTable();
} }
Address Isolate::get_address_from_id(IsolateAddressId id) { Address Isolate::get_address_from_id(IsolateAddressId id) {
@ -2240,14 +2235,9 @@ char* Isolate::RestoreThread(char* from) {
return from + sizeof(ThreadLocalTop); return from + sizeof(ThreadLocalTop);
} }
Isolate::ThreadDataTable::ThreadDataTable() : list_(nullptr) {} Isolate::ThreadDataTable::ThreadDataTable() : table_() {}
Isolate::ThreadDataTable::~ThreadDataTable() { Isolate::ThreadDataTable::~ThreadDataTable() {}
// TODO(svenpanne) The assertion below would fire if an embedder does not
// cleanly dispose all Isolates before disposing v8, so we are conservative
// and leave it out for now.
// DCHECK_NULL(list_);
}
void Isolate::ReleaseManagedObjects() { void Isolate::ReleaseManagedObjects() {
Isolate::ManagedObjectFinalizer* current = Isolate::ManagedObjectFinalizer* current =
@ -2294,40 +2284,30 @@ Isolate::PerIsolateThreadData::~PerIsolateThreadData() {
#endif #endif
} }
Isolate::PerIsolateThreadData* Isolate::ThreadDataTable::Lookup(
Isolate::PerIsolateThreadData* ThreadId thread_id) {
Isolate::ThreadDataTable::Lookup(Isolate* isolate, auto t = table_.find(thread_id);
ThreadId thread_id) { if (t == table_.end()) return nullptr;
for (PerIsolateThreadData* data = list_; data != nullptr; return t->second;
data = data->next_) {
if (data->Matches(isolate, thread_id)) return data;
}
return nullptr;
} }
void Isolate::ThreadDataTable::Insert(Isolate::PerIsolateThreadData* data) { void Isolate::ThreadDataTable::Insert(Isolate::PerIsolateThreadData* data) {
if (list_ != nullptr) list_->prev_ = data; bool inserted = table_.insert(std::make_pair(data->thread_id_, data)).second;
data->next_ = list_; CHECK(inserted);
list_ = data;
} }
void Isolate::ThreadDataTable::Remove(PerIsolateThreadData* data) { void Isolate::ThreadDataTable::Remove(PerIsolateThreadData* data) {
if (list_ == data) list_ = data->next_; table_.erase(data->thread_id_);
if (data->next_ != nullptr) data->next_->prev_ = data->prev_;
if (data->prev_ != nullptr) data->prev_->next_ = data->next_;
delete data; delete data;
} }
void Isolate::ThreadDataTable::RemoveAllThreads() {
void Isolate::ThreadDataTable::RemoveAllThreads(Isolate* isolate) { for (auto& x : table_) {
PerIsolateThreadData* data = list_; delete x.second;
while (data != nullptr) {
PerIsolateThreadData* next = data->next_;
if (data->isolate() == isolate) Remove(data);
data = next;
} }
table_.clear();
} }
@ -2502,10 +2482,6 @@ Isolate::Isolate(bool enable_serializer)
cancelable_task_manager_(new CancelableTaskManager()), cancelable_task_manager_(new CancelableTaskManager()),
abort_on_uncaught_exception_callback_(nullptr), abort_on_uncaught_exception_callback_(nullptr),
total_regexp_code_generated_(0) { total_regexp_code_generated_(0) {
{
base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer());
CHECK(thread_data_table_);
}
id_ = base::Relaxed_AtomicIncrement(&isolate_counter_, 1); id_ = base::Relaxed_AtomicIncrement(&isolate_counter_, 1);
TRACE_ISOLATE(constructor); TRACE_ISOLATE(constructor);
@ -2563,8 +2539,8 @@ void Isolate::TearDown() {
Deinit(); Deinit();
{ {
base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer()); base::LockGuard<base::Mutex> lock_guard(&thread_data_table_mutex_);
thread_data_table_->RemoveAllThreads(this); thread_data_table_.RemoveAllThreads();
} }
#ifdef DEBUG #ifdef DEBUG
@ -2578,12 +2554,6 @@ void Isolate::TearDown() {
} }
void Isolate::GlobalTearDown() {
delete thread_data_table_;
thread_data_table_ = nullptr;
}
void Isolate::ClearSerializerData() { void Isolate::ClearSerializerData() {
delete external_reference_table_; delete external_reference_table_;
external_reference_table_ = nullptr; external_reference_table_ = nullptr;

27
deps/v8/src/isolate.h vendored
View File

@ -8,6 +8,7 @@
#include <cstddef> #include <cstddef>
#include <memory> #include <memory>
#include <queue> #include <queue>
#include <unordered_map>
#include <vector> #include <vector>
#include "include/v8.h" #include "include/v8.h"
@ -247,6 +248,8 @@ class ThreadId {
return *this; return *this;
} }
bool operator==(const ThreadId& other) const { return Equals(other); }
// Returns ThreadId for current thread. // Returns ThreadId for current thread.
static ThreadId Current() { return ThreadId(GetCurrentThreadId()); } static ThreadId Current() { return ThreadId(GetCurrentThreadId()); }
@ -287,7 +290,6 @@ class ThreadId {
friend class Isolate; friend class Isolate;
}; };
#define FIELD_ACCESSOR(type, name) \ #define FIELD_ACCESSOR(type, name) \
inline void set_##name(type v) { name##_ = v; } \ inline void set_##name(type v) { name##_ = v; } \
inline type name() const { return name##_; } inline type name() const { return name##_; }
@ -550,8 +552,6 @@ class Isolate {
void ReleaseManagedObjects(); void ReleaseManagedObjects();
static void GlobalTearDown();
void ClearSerializerData(); void ClearSerializerData();
// Find the PerThread for this particular (isolate, thread) combination // Find the PerThread for this particular (isolate, thread) combination
@ -1371,20 +1371,24 @@ class Isolate {
void* embedder_data_[Internals::kNumIsolateDataSlots]; void* embedder_data_[Internals::kNumIsolateDataSlots];
Heap heap_; Heap heap_;
// The per-process lock should be acquired before the ThreadDataTable is
// modified.
class ThreadDataTable { class ThreadDataTable {
public: public:
ThreadDataTable(); ThreadDataTable();
~ThreadDataTable(); ~ThreadDataTable();
PerIsolateThreadData* Lookup(Isolate* isolate, ThreadId thread_id); PerIsolateThreadData* Lookup(ThreadId thread_id);
void Insert(PerIsolateThreadData* data); void Insert(PerIsolateThreadData* data);
void Remove(PerIsolateThreadData* data); void Remove(PerIsolateThreadData* data);
void RemoveAllThreads(Isolate* isolate); void RemoveAllThreads();
private: private:
PerIsolateThreadData* list_; struct Hasher {
std::size_t operator()(const ThreadId& t) const {
return std::hash<int>()(t.ToInteger());
}
};
std::unordered_map<ThreadId, PerIsolateThreadData*, Hasher> table_;
}; };
// These items form a stack synchronously with threads Enter'ing and Exit'ing // These items form a stack synchronously with threads Enter'ing and Exit'ing
@ -1412,12 +1416,15 @@ class Isolate {
DISALLOW_COPY_AND_ASSIGN(EntryStackItem); DISALLOW_COPY_AND_ASSIGN(EntryStackItem);
}; };
static base::LazyMutex thread_data_table_mutex_; // TODO(kenton@cloudflare.com): This mutex can be removed if
// thread_data_table_ is always accessed under the isolate lock. I do not
// know if this is the case, so I'm preserving it for now.
base::Mutex thread_data_table_mutex_;
static base::Thread::LocalStorageKey per_isolate_thread_data_key_; static base::Thread::LocalStorageKey per_isolate_thread_data_key_;
static base::Thread::LocalStorageKey isolate_key_; static base::Thread::LocalStorageKey isolate_key_;
static base::Thread::LocalStorageKey thread_id_key_; static base::Thread::LocalStorageKey thread_id_key_;
static ThreadDataTable* thread_data_table_; ThreadDataTable thread_data_table_;
// A global counter for all generated Isolates, might overflow. // A global counter for all generated Isolates, might overflow.
static base::Atomic32 isolate_counter_; static base::Atomic32 isolate_counter_;

1
deps/v8/src/v8.cc vendored
View File

@ -49,7 +49,6 @@ void V8::TearDown() {
Bootstrapper::TearDownExtensions(); Bootstrapper::TearDownExtensions();
ElementsAccessor::TearDown(); ElementsAccessor::TearDown();
RegisteredExtension::UnregisterAll(); RegisteredExtension::UnregisterAll();
Isolate::GlobalTearDown();
sampler::Sampler::TearDown(); sampler::Sampler::TearDown();
FlagList::ResetAllFlags(); // Frees memory held by string arguments. FlagList::ResetAllFlags(); // Frees memory held by string arguments.
} }