Skip to content

Commit 646445b

Browse files
committed
deps: V8: backport b49206d from upstream
This is the v8.x backport of nodejs#20727. 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 <[email protected]> Commit-Queue: Yang Guo <[email protected]> Cr-Commit-Position: refs/heads/master@{nodejs#52753} Backport-PR-URL: nodejs#21529 PR-URL: nodejs#20727 Refs: nodejs#20083 Refs: nodejs#20083 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 978e1b1 commit 646445b

File tree

4 files changed

+45
-71
lines changed

4 files changed

+45
-71
lines changed

deps/v8/include/v8-version.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
#define V8_MAJOR_VERSION 6
1212
#define V8_MINOR_VERSION 2
1313
#define V8_BUILD_NUMBER 414
14-
#define V8_PATCH_LEVEL 57
14+
#define V8_PATCH_LEVEL 58
1515

1616
// Use 1 for candidates and 0 otherwise.
1717
// (Boolean macro values are not supported by all preprocessors.)

deps/v8/src/isolate.cc

+26-59
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include <fstream> // NOLINT(readability/streams)
1010
#include <sstream>
11+
#include <unordered_map>
1112

1213
#include "src/assembler-inl.h"
1314
#include "src/ast/ast-value-factory.h"
@@ -128,8 +129,6 @@ void ThreadLocalTop::Free() {
128129
base::Thread::LocalStorageKey Isolate::isolate_key_;
129130
base::Thread::LocalStorageKey Isolate::thread_id_key_;
130131
base::Thread::LocalStorageKey Isolate::per_isolate_thread_data_key_;
131-
base::LazyMutex Isolate::thread_data_table_mutex_ = LAZY_MUTEX_INITIALIZER;
132-
Isolate::ThreadDataTable* Isolate::thread_data_table_ = NULL;
133132
base::Atomic32 Isolate::isolate_counter_ = 0;
134133
#if DEBUG
135134
base::Atomic32 Isolate::isolate_key_created_ = 0;
@@ -140,13 +139,13 @@ Isolate::PerIsolateThreadData*
140139
ThreadId thread_id = ThreadId::Current();
141140
PerIsolateThreadData* per_thread = NULL;
142141
{
143-
base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer());
144-
per_thread = thread_data_table_->Lookup(this, thread_id);
142+
base::LockGuard<base::Mutex> lock_guard(&thread_data_table_mutex_);
143+
per_thread = thread_data_table_.Lookup(thread_id);
145144
if (per_thread == NULL) {
146145
per_thread = new PerIsolateThreadData(this, thread_id);
147-
thread_data_table_->Insert(per_thread);
146+
thread_data_table_.Insert(per_thread);
148147
}
149-
DCHECK(thread_data_table_->Lookup(this, thread_id) == per_thread);
148+
DCHECK(thread_data_table_.Lookup(thread_id) == per_thread);
150149
}
151150
return per_thread;
152151
}
@@ -157,12 +156,11 @@ void Isolate::DiscardPerThreadDataForThisThread() {
157156
if (thread_id_int) {
158157
ThreadId thread_id = ThreadId(thread_id_int);
159158
DCHECK(!thread_manager_->mutex_owner_.Equals(thread_id));
160-
base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer());
161-
PerIsolateThreadData* per_thread =
162-
thread_data_table_->Lookup(this, thread_id);
159+
base::LockGuard<base::Mutex> lock_guard(&thread_data_table_mutex_);
160+
PerIsolateThreadData* per_thread = thread_data_table_.Lookup(thread_id);
163161
if (per_thread) {
164162
DCHECK(!per_thread->thread_state_);
165-
thread_data_table_->Remove(per_thread);
163+
thread_data_table_.Remove(per_thread);
166164
}
167165
}
168166
}
@@ -178,23 +176,20 @@ Isolate::PerIsolateThreadData* Isolate::FindPerThreadDataForThread(
178176
ThreadId thread_id) {
179177
PerIsolateThreadData* per_thread = NULL;
180178
{
181-
base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer());
182-
per_thread = thread_data_table_->Lookup(this, thread_id);
179+
base::LockGuard<base::Mutex> lock_guard(&thread_data_table_mutex_);
180+
per_thread = thread_data_table_.Lookup(thread_id);
183181
}
184182
return per_thread;
185183
}
186184

187185

188186
void Isolate::InitializeOncePerProcess() {
189-
base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer());
190-
CHECK(thread_data_table_ == NULL);
191187
isolate_key_ = base::Thread::CreateThreadLocalKey();
192188
#if DEBUG
193189
base::Relaxed_Store(&isolate_key_created_, 1);
194190
#endif
195191
thread_id_key_ = base::Thread::CreateThreadLocalKey();
196192
per_isolate_thread_data_key_ = base::Thread::CreateThreadLocalKey();
197-
thread_data_table_ = new Isolate::ThreadDataTable();
198193
}
199194

200195
Address Isolate::get_address_from_id(IsolateAddressId id) {
@@ -2093,18 +2088,9 @@ char* Isolate::RestoreThread(char* from) {
20932088
return from + sizeof(ThreadLocalTop);
20942089
}
20952090

2091+
Isolate::ThreadDataTable::ThreadDataTable() : table_() {}
20962092

2097-
Isolate::ThreadDataTable::ThreadDataTable()
2098-
: list_(NULL) {
2099-
}
2100-
2101-
2102-
Isolate::ThreadDataTable::~ThreadDataTable() {
2103-
// TODO(svenpanne) The assertion below would fire if an embedder does not
2104-
// cleanly dispose all Isolates before disposing v8, so we are conservative
2105-
// and leave it out for now.
2106-
// DCHECK_NULL(list_);
2107-
}
2093+
Isolate::ThreadDataTable::~ThreadDataTable() {}
21082094

21092095
void Isolate::ReleaseManagedObjects() {
21102096
Isolate::ManagedObjectFinalizer* current =
@@ -2151,39 +2137,30 @@ Isolate::PerIsolateThreadData::~PerIsolateThreadData() {
21512137
#endif
21522138
}
21532139

2154-
2155-
Isolate::PerIsolateThreadData*
2156-
Isolate::ThreadDataTable::Lookup(Isolate* isolate,
2157-
ThreadId thread_id) {
2158-
for (PerIsolateThreadData* data = list_; data != NULL; data = data->next_) {
2159-
if (data->Matches(isolate, thread_id)) return data;
2160-
}
2161-
return NULL;
2140+
Isolate::PerIsolateThreadData* Isolate::ThreadDataTable::Lookup(
2141+
ThreadId thread_id) {
2142+
auto t = table_.find(thread_id);
2143+
if (t == table_.end()) return nullptr;
2144+
return t->second;
21622145
}
21632146

21642147

21652148
void Isolate::ThreadDataTable::Insert(Isolate::PerIsolateThreadData* data) {
2166-
if (list_ != NULL) list_->prev_ = data;
2167-
data->next_ = list_;
2168-
list_ = data;
2149+
bool inserted = table_.insert(std::make_pair(data->thread_id_, data)).second;
2150+
CHECK(inserted);
21692151
}
21702152

21712153

21722154
void Isolate::ThreadDataTable::Remove(PerIsolateThreadData* data) {
2173-
if (list_ == data) list_ = data->next_;
2174-
if (data->next_ != NULL) data->next_->prev_ = data->prev_;
2175-
if (data->prev_ != NULL) data->prev_->next_ = data->next_;
2155+
table_.erase(data->thread_id_);
21762156
delete data;
21772157
}
21782158

2179-
2180-
void Isolate::ThreadDataTable::RemoveAllThreads(Isolate* isolate) {
2181-
PerIsolateThreadData* data = list_;
2182-
while (data != NULL) {
2183-
PerIsolateThreadData* next = data->next_;
2184-
if (data->isolate() == isolate) Remove(data);
2185-
data = next;
2159+
void Isolate::ThreadDataTable::RemoveAllThreads() {
2160+
for (auto& x : table_) {
2161+
delete x.second;
21862162
}
2163+
table_.clear();
21872164
}
21882165

21892166

@@ -2360,10 +2337,6 @@ Isolate::Isolate(bool enable_serializer)
23602337
wasm_compilation_manager_(new wasm::CompilationManager()),
23612338
abort_on_uncaught_exception_callback_(NULL),
23622339
total_regexp_code_generated_(0) {
2363-
{
2364-
base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer());
2365-
CHECK(thread_data_table_);
2366-
}
23672340
id_ = base::Relaxed_AtomicIncrement(&isolate_counter_, 1);
23682341
TRACE_ISOLATE(constructor);
23692342

@@ -2418,8 +2391,8 @@ void Isolate::TearDown() {
24182391
Deinit();
24192392

24202393
{
2421-
base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer());
2422-
thread_data_table_->RemoveAllThreads(this);
2394+
base::LockGuard<base::Mutex> lock_guard(&thread_data_table_mutex_);
2395+
thread_data_table_.RemoveAllThreads();
24232396
}
24242397

24252398
delete this;
@@ -2429,12 +2402,6 @@ void Isolate::TearDown() {
24292402
}
24302403

24312404

2432-
void Isolate::GlobalTearDown() {
2433-
delete thread_data_table_;
2434-
thread_data_table_ = NULL;
2435-
}
2436-
2437-
24382405
void Isolate::ClearSerializerData() {
24392406
delete external_reference_table_;
24402407
external_reference_table_ = NULL;

deps/v8/src/isolate.h

+18-10
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
#include <cstddef>
99
#include <memory>
1010
#include <queue>
11+
#include <unordered_map>
12+
#include <vector>
1113

1214
#include "include/v8-debug.h"
1315
#include "src/allocation.h"
@@ -243,6 +245,8 @@ class ThreadId {
243245
return *this;
244246
}
245247

248+
bool operator==(const ThreadId& other) const { return Equals(other); }
249+
246250
// Returns ThreadId for current thread.
247251
static ThreadId Current() { return ThreadId(GetCurrentThreadId()); }
248252

@@ -283,7 +287,6 @@ class ThreadId {
283287
friend class Isolate;
284288
};
285289

286-
287290
#define FIELD_ACCESSOR(type, name) \
288291
inline void set_##name(type v) { name##_ = v; } \
289292
inline type name() const { return name##_; }
@@ -557,8 +560,6 @@ class Isolate {
557560

558561
void ReleaseManagedObjects();
559562

560-
static void GlobalTearDown();
561-
562563
void ClearSerializerData();
563564

564565
// Find the PerThread for this particular (isolate, thread) combination
@@ -1334,20 +1335,24 @@ class Isolate {
13341335
void* embedder_data_[Internals::kNumIsolateDataSlots];
13351336
Heap heap_;
13361337

1337-
// The per-process lock should be acquired before the ThreadDataTable is
1338-
// modified.
13391338
class ThreadDataTable {
13401339
public:
13411340
ThreadDataTable();
13421341
~ThreadDataTable();
13431342

1344-
PerIsolateThreadData* Lookup(Isolate* isolate, ThreadId thread_id);
1343+
PerIsolateThreadData* Lookup(ThreadId thread_id);
13451344
void Insert(PerIsolateThreadData* data);
13461345
void Remove(PerIsolateThreadData* data);
1347-
void RemoveAllThreads(Isolate* isolate);
1346+
void RemoveAllThreads();
13481347

13491348
private:
1350-
PerIsolateThreadData* list_;
1349+
struct Hasher {
1350+
std::size_t operator()(const ThreadId& t) const {
1351+
return std::hash<int>()(t.ToInteger());
1352+
}
1353+
};
1354+
1355+
std::unordered_map<ThreadId, PerIsolateThreadData*, Hasher> table_;
13511356
};
13521357

13531358
// These items form a stack synchronously with threads Enter'ing and Exit'ing
@@ -1375,12 +1380,15 @@ class Isolate {
13751380
DISALLOW_COPY_AND_ASSIGN(EntryStackItem);
13761381
};
13771382

1378-
static base::LazyMutex thread_data_table_mutex_;
1383+
// TODO([email protected]): This mutex can be removed if
1384+
// thread_data_table_ is always accessed under the isolate lock. I do not
1385+
// know if this is the case, so I'm preserving it for now.
1386+
base::Mutex thread_data_table_mutex_;
13791387

13801388
static base::Thread::LocalStorageKey per_isolate_thread_data_key_;
13811389
static base::Thread::LocalStorageKey isolate_key_;
13821390
static base::Thread::LocalStorageKey thread_id_key_;
1383-
static ThreadDataTable* thread_data_table_;
1391+
ThreadDataTable thread_data_table_;
13841392

13851393
// A global counter for all generated Isolates, might overflow.
13861394
static base::Atomic32 isolate_counter_;

deps/v8/src/v8.cc

-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ void V8::TearDown() {
4646
Bootstrapper::TearDownExtensions();
4747
ElementsAccessor::TearDown();
4848
RegisteredExtension::UnregisterAll();
49-
Isolate::GlobalTearDown();
5049
sampler::Sampler::TearDown();
5150
FlagList::ResetAllFlags(); // Frees memory held by string arguments.
5251
}

0 commit comments

Comments
 (0)