-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
deps: V8: backport b49206d from upstream
This is the v8.x backport of #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@{#52753} Backport-PR-URL: #21529 PR-URL: #20727 Refs: #20083 Refs: #20083 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
- Loading branch information
Showing
4 changed files
with
45 additions
and
71 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,8 @@ | |
#include <cstddef> | ||
#include <memory> | ||
#include <queue> | ||
#include <unordered_map> | ||
#include <vector> | ||
|
||
#include "include/v8-debug.h" | ||
#include "src/allocation.h" | ||
|
@@ -243,6 +245,8 @@ class ThreadId { | |
return *this; | ||
} | ||
|
||
bool operator==(const ThreadId& other) const { return Equals(other); } | ||
|
||
// Returns ThreadId for current thread. | ||
static ThreadId Current() { return ThreadId(GetCurrentThreadId()); } | ||
|
||
|
@@ -283,7 +287,6 @@ class ThreadId { | |
friend class Isolate; | ||
}; | ||
|
||
|
||
#define FIELD_ACCESSOR(type, name) \ | ||
inline void set_##name(type v) { name##_ = v; } \ | ||
inline type name() const { return name##_; } | ||
|
@@ -557,8 +560,6 @@ class Isolate { | |
|
||
void ReleaseManagedObjects(); | ||
|
||
static void GlobalTearDown(); | ||
|
||
void ClearSerializerData(); | ||
|
||
// Find the PerThread for this particular (isolate, thread) combination | ||
|
@@ -1334,20 +1335,24 @@ class Isolate { | |
void* embedder_data_[Internals::kNumIsolateDataSlots]; | ||
Heap heap_; | ||
|
||
// The per-process lock should be acquired before the ThreadDataTable is | ||
// modified. | ||
class ThreadDataTable { | ||
public: | ||
ThreadDataTable(); | ||
~ThreadDataTable(); | ||
|
||
PerIsolateThreadData* Lookup(Isolate* isolate, ThreadId thread_id); | ||
PerIsolateThreadData* Lookup(ThreadId thread_id); | ||
void Insert(PerIsolateThreadData* data); | ||
void Remove(PerIsolateThreadData* data); | ||
void RemoveAllThreads(Isolate* isolate); | ||
void RemoveAllThreads(); | ||
|
||
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 | ||
|
@@ -1375,12 +1380,15 @@ class Isolate { | |
DISALLOW_COPY_AND_ASSIGN(EntryStackItem); | ||
}; | ||
|
||
static base::LazyMutex thread_data_table_mutex_; | ||
// TODO([email protected]): 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 isolate_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. | ||
static base::Atomic32 isolate_counter_; | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters