Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 91e60b0

Browse filesBrowse files
ofrobotsMylesBorins
ofrobots
authored and
MylesBorins
committed
deps: V8: cherry-pick b49206d from upstream
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: #20727 Ref: #20083 Refs: #20083 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent e1ff587 commit 91e60b0
Copy full SHA for 91e60b0

File tree

Expand file treeCollapse file tree

4 files changed

+44
-68
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+44
-68
lines changed
Open diff view settings
Collapse file

‎common.gypi‎

Copy file name to clipboardExpand all lines: common.gypi
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727

2828
# Reset this number to 0 on major V8 upgrades.
2929
# Increment by one for each non-official patch applied to deps/v8.
30-
'v8_embedder_string': '-node.7',
30+
'v8_embedder_string': '-node.8',
3131

3232
# Enable disassembler for `--print-code` v8 options
3333
'v8_enable_disassembler': 1,
Collapse file

‎deps/v8/src/isolate.cc‎

Copy file name to clipboardExpand all lines: deps/v8/src/isolate.cc
+26-56Lines changed: 26 additions & 56 deletions
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/api.h"
1314
#include "src/assembler-inl.h"
@@ -136,8 +137,6 @@ void ThreadLocalTop::Free() {
136137
base::Thread::LocalStorageKey Isolate::isolate_key_;
137138
base::Thread::LocalStorageKey Isolate::thread_id_key_;
138139
base::Thread::LocalStorageKey Isolate::per_isolate_thread_data_key_;
139-
base::LazyMutex Isolate::thread_data_table_mutex_ = LAZY_MUTEX_INITIALIZER;
140-
Isolate::ThreadDataTable* Isolate::thread_data_table_ = nullptr;
141140
base::Atomic32 Isolate::isolate_counter_ = 0;
142141
#if DEBUG
143142
base::Atomic32 Isolate::isolate_key_created_ = 0;
@@ -148,13 +147,13 @@ Isolate::PerIsolateThreadData*
148147
ThreadId thread_id = ThreadId::Current();
149148
PerIsolateThreadData* per_thread = nullptr;
150149
{
151-
base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer());
152-
per_thread = thread_data_table_->Lookup(this, thread_id);
150+
base::LockGuard<base::Mutex> lock_guard(&thread_data_table_mutex_);
151+
per_thread = thread_data_table_.Lookup(thread_id);
153152
if (per_thread == nullptr) {
154153
per_thread = new PerIsolateThreadData(this, thread_id);
155-
thread_data_table_->Insert(per_thread);
154+
thread_data_table_.Insert(per_thread);
156155
}
157-
DCHECK(thread_data_table_->Lookup(this, thread_id) == per_thread);
156+
DCHECK(thread_data_table_.Lookup(thread_id) == per_thread);
158157
}
159158
return per_thread;
160159
}
@@ -165,12 +164,11 @@ void Isolate::DiscardPerThreadDataForThisThread() {
165164
if (thread_id_int) {
166165
ThreadId thread_id = ThreadId(thread_id_int);
167166
DCHECK(!thread_manager_->mutex_owner_.Equals(thread_id));
168-
base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer());
169-
PerIsolateThreadData* per_thread =
170-
thread_data_table_->Lookup(this, thread_id);
167+
base::LockGuard<base::Mutex> lock_guard(&thread_data_table_mutex_);
168+
PerIsolateThreadData* per_thread = thread_data_table_.Lookup(thread_id);
171169
if (per_thread) {
172170
DCHECK(!per_thread->thread_state_);
173-
thread_data_table_->Remove(per_thread);
171+
thread_data_table_.Remove(per_thread);
174172
}
175173
}
176174
}
@@ -186,23 +184,20 @@ Isolate::PerIsolateThreadData* Isolate::FindPerThreadDataForThread(
186184
ThreadId thread_id) {
187185
PerIsolateThreadData* per_thread = nullptr;
188186
{
189-
base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer());
190-
per_thread = thread_data_table_->Lookup(this, thread_id);
187+
base::LockGuard<base::Mutex> lock_guard(&thread_data_table_mutex_);
188+
per_thread = thread_data_table_.Lookup(thread_id);
191189
}
192190
return per_thread;
193191
}
194192

195193

196194
void Isolate::InitializeOncePerProcess() {
197-
base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer());
198-
CHECK_NULL(thread_data_table_);
199195
isolate_key_ = base::Thread::CreateThreadLocalKey();
200196
#if DEBUG
201197
base::Relaxed_Store(&isolate_key_created_, 1);
202198
#endif
203199
thread_id_key_ = base::Thread::CreateThreadLocalKey();
204200
per_isolate_thread_data_key_ = base::Thread::CreateThreadLocalKey();
205-
thread_data_table_ = new Isolate::ThreadDataTable();
206201
}
207202

208203
Address Isolate::get_address_from_id(IsolateAddressId id) {
@@ -2240,14 +2235,9 @@ char* Isolate::RestoreThread(char* from) {
22402235
return from + sizeof(ThreadLocalTop);
22412236
}
22422237

2243-
Isolate::ThreadDataTable::ThreadDataTable() : list_(nullptr) {}
2238+
Isolate::ThreadDataTable::ThreadDataTable() : table_() {}
22442239

2245-
Isolate::ThreadDataTable::~ThreadDataTable() {
2246-
// TODO(svenpanne) The assertion below would fire if an embedder does not
2247-
// cleanly dispose all Isolates before disposing v8, so we are conservative
2248-
// and leave it out for now.
2249-
// DCHECK_NULL(list_);
2250-
}
2240+
Isolate::ThreadDataTable::~ThreadDataTable() {}
22512241

22522242
void Isolate::ReleaseManagedObjects() {
22532243
Isolate::ManagedObjectFinalizer* current =
@@ -2294,40 +2284,30 @@ Isolate::PerIsolateThreadData::~PerIsolateThreadData() {
22942284
#endif
22952285
}
22962286

2297-
2298-
Isolate::PerIsolateThreadData*
2299-
Isolate::ThreadDataTable::Lookup(Isolate* isolate,
2300-
ThreadId thread_id) {
2301-
for (PerIsolateThreadData* data = list_; data != nullptr;
2302-
data = data->next_) {
2303-
if (data->Matches(isolate, thread_id)) return data;
2304-
}
2305-
return nullptr;
2287+
Isolate::PerIsolateThreadData* Isolate::ThreadDataTable::Lookup(
2288+
ThreadId thread_id) {
2289+
auto t = table_.find(thread_id);
2290+
if (t == table_.end()) return nullptr;
2291+
return t->second;
23062292
}
23072293

23082294

23092295
void Isolate::ThreadDataTable::Insert(Isolate::PerIsolateThreadData* data) {
2310-
if (list_ != nullptr) list_->prev_ = data;
2311-
data->next_ = list_;
2312-
list_ = data;
2296+
bool inserted = table_.insert(std::make_pair(data->thread_id_, data)).second;
2297+
CHECK(inserted);
23132298
}
23142299

23152300

23162301
void Isolate::ThreadDataTable::Remove(PerIsolateThreadData* data) {
2317-
if (list_ == data) list_ = data->next_;
2318-
if (data->next_ != nullptr) data->next_->prev_ = data->prev_;
2319-
if (data->prev_ != nullptr) data->prev_->next_ = data->next_;
2302+
table_.erase(data->thread_id_);
23202303
delete data;
23212304
}
23222305

2323-
2324-
void Isolate::ThreadDataTable::RemoveAllThreads(Isolate* isolate) {
2325-
PerIsolateThreadData* data = list_;
2326-
while (data != nullptr) {
2327-
PerIsolateThreadData* next = data->next_;
2328-
if (data->isolate() == isolate) Remove(data);
2329-
data = next;
2306+
void Isolate::ThreadDataTable::RemoveAllThreads() {
2307+
for (auto& x : table_) {
2308+
delete x.second;
23302309
}
2310+
table_.clear();
23312311
}
23322312

23332313

@@ -2502,10 +2482,6 @@ Isolate::Isolate(bool enable_serializer)
25022482
cancelable_task_manager_(new CancelableTaskManager()),
25032483
abort_on_uncaught_exception_callback_(nullptr),
25042484
total_regexp_code_generated_(0) {
2505-
{
2506-
base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer());
2507-
CHECK(thread_data_table_);
2508-
}
25092485
id_ = base::Relaxed_AtomicIncrement(&isolate_counter_, 1);
25102486
TRACE_ISOLATE(constructor);
25112487

@@ -2563,8 +2539,8 @@ void Isolate::TearDown() {
25632539
Deinit();
25642540

25652541
{
2566-
base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer());
2567-
thread_data_table_->RemoveAllThreads(this);
2542+
base::LockGuard<base::Mutex> lock_guard(&thread_data_table_mutex_);
2543+
thread_data_table_.RemoveAllThreads();
25682544
}
25692545

25702546
#ifdef DEBUG
@@ -2578,12 +2554,6 @@ void Isolate::TearDown() {
25782554
}
25792555

25802556

2581-
void Isolate::GlobalTearDown() {
2582-
delete thread_data_table_;
2583-
thread_data_table_ = nullptr;
2584-
}
2585-
2586-
25872557
void Isolate::ClearSerializerData() {
25882558
delete external_reference_table_;
25892559
external_reference_table_ = nullptr;
Collapse file

‎deps/v8/src/isolate.h‎

Copy file name to clipboardExpand all lines: deps/v8/src/isolate.h
+17-10Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <cstddef>
99
#include <memory>
1010
#include <queue>
11+
#include <unordered_map>
1112
#include <vector>
1213

1314
#include "include/v8.h"
@@ -247,6 +248,8 @@ class ThreadId {
247248
return *this;
248249
}
249250

251+
bool operator==(const ThreadId& other) const { return Equals(other); }
252+
250253
// Returns ThreadId for current thread.
251254
static ThreadId Current() { return ThreadId(GetCurrentThreadId()); }
252255

@@ -287,7 +290,6 @@ class ThreadId {
287290
friend class Isolate;
288291
};
289292

290-
291293
#define FIELD_ACCESSOR(type, name) \
292294
inline void set_##name(type v) { name##_ = v; } \
293295
inline type name() const { return name##_; }
@@ -550,8 +552,6 @@ class Isolate {
550552

551553
void ReleaseManagedObjects();
552554

553-
static void GlobalTearDown();
554-
555555
void ClearSerializerData();
556556

557557
// Find the PerThread for this particular (isolate, thread) combination
@@ -1371,20 +1371,24 @@ class Isolate {
13711371
void* embedder_data_[Internals::kNumIsolateDataSlots];
13721372
Heap heap_;
13731373

1374-
// The per-process lock should be acquired before the ThreadDataTable is
1375-
// modified.
13761374
class ThreadDataTable {
13771375
public:
13781376
ThreadDataTable();
13791377
~ThreadDataTable();
13801378

1381-
PerIsolateThreadData* Lookup(Isolate* isolate, ThreadId thread_id);
1379+
PerIsolateThreadData* Lookup(ThreadId thread_id);
13821380
void Insert(PerIsolateThreadData* data);
13831381
void Remove(PerIsolateThreadData* data);
1384-
void RemoveAllThreads(Isolate* isolate);
1382+
void RemoveAllThreads();
13851383

13861384
private:
1387-
PerIsolateThreadData* list_;
1385+
struct Hasher {
1386+
std::size_t operator()(const ThreadId& t) const {
1387+
return std::hash<int>()(t.ToInteger());
1388+
}
1389+
};
1390+
1391+
std::unordered_map<ThreadId, PerIsolateThreadData*, Hasher> table_;
13881392
};
13891393

13901394
// These items form a stack synchronously with threads Enter'ing and Exit'ing
@@ -1412,12 +1416,15 @@ class Isolate {
14121416
DISALLOW_COPY_AND_ASSIGN(EntryStackItem);
14131417
};
14141418

1415-
static base::LazyMutex thread_data_table_mutex_;
1419+
// TODO(kenton@cloudflare.com): This mutex can be removed if
1420+
// thread_data_table_ is always accessed under the isolate lock. I do not
1421+
// know if this is the case, so I'm preserving it for now.
1422+
base::Mutex thread_data_table_mutex_;
14161423

14171424
static base::Thread::LocalStorageKey per_isolate_thread_data_key_;
14181425
static base::Thread::LocalStorageKey isolate_key_;
14191426
static base::Thread::LocalStorageKey thread_id_key_;
1420-
static ThreadDataTable* thread_data_table_;
1427+
ThreadDataTable thread_data_table_;
14211428

14221429
// A global counter for all generated Isolates, might overflow.
14231430
static base::Atomic32 isolate_counter_;
Collapse file

‎deps/v8/src/v8.cc‎

Copy file name to clipboardExpand all lines: deps/v8/src/v8.cc
-1Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ void V8::TearDown() {
4949
Bootstrapper::TearDownExtensions();
5050
ElementsAccessor::TearDown();
5151
RegisteredExtension::UnregisterAll();
52-
Isolate::GlobalTearDown();
5352
sampler::Sampler::TearDown();
5453
FlagList::ResetAllFlags(); // Frees memory held by string arguments.
5554
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.