Commit f5cdcafc authored by Clemens Hammacher's avatar Clemens Hammacher Committed by Commit Bot

Revert "[heap] Reschedule concurrent marking tasks earlier"

This reverts commit b1c3ca2a.

Reason for revert: TSan issues: https://ci.chromium.org/p/v8/builders/ci/V8%20Linux64%20TSAN/28147

Original change's description:
> [heap] Reschedule concurrent marking tasks earlier
> 
> Currently we reschedule concurrent marking tasks if all tasks finish.
> This is too conservative and we can improve performance by rescheduling
> finished tasks without waiting for all other tasks.
> 
> As a drive-by this also changes task_count_ to total_task_count_.
> 
> Change-Id: If0b3bd45ce6d52f6bcd0065dd8d3efe9ea84184a
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1789142
> Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
> Reviewed-by: Omer Katz <omerkatz@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#63593}

TBR=ulan@chromium.org,omerkatz@chromium.org

Change-Id: I5e6b406a021c8fd4834e346e02388552ee3e0036
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1789287Reviewed-by: 's avatarClemens Hammacher <clemensh@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63594}
parent b1c3ca2a
...@@ -864,7 +864,8 @@ void ConcurrentMarking::ScheduleTasks() { ...@@ -864,7 +864,8 @@ void ConcurrentMarking::ScheduleTasks() {
DCHECK(FLAG_parallel_marking || FLAG_concurrent_marking); DCHECK(FLAG_parallel_marking || FLAG_concurrent_marking);
DCHECK(!heap_->IsTearingDown()); DCHECK(!heap_->IsTearingDown());
base::MutexGuard guard(&pending_lock_); base::MutexGuard guard(&pending_lock_);
if (total_task_count_ == 0) { DCHECK_EQ(0, pending_task_count_);
if (task_count_ == 0) {
static const int num_cores = static const int num_cores =
V8::GetCurrentPlatform()->NumberOfWorkerThreads() + 1; V8::GetCurrentPlatform()->NumberOfWorkerThreads() + 1;
#if defined(V8_OS_MACOSX) #if defined(V8_OS_MACOSX)
...@@ -872,18 +873,15 @@ void ConcurrentMarking::ScheduleTasks() { ...@@ -872,18 +873,15 @@ void ConcurrentMarking::ScheduleTasks() {
// marking on competing hyper-threads (regresses Octane/Splay). As such, // marking on competing hyper-threads (regresses Octane/Splay). As such,
// only use num_cores/2, leaving one of those for the main thread. // only use num_cores/2, leaving one of those for the main thread.
// TODO(ulan): Use all cores on Mac 10.12+. // TODO(ulan): Use all cores on Mac 10.12+.
total_task_count_ = Max(1, Min(kMaxTasks, (num_cores / 2) - 1)); task_count_ = Max(1, Min(kMaxTasks, (num_cores / 2) - 1));
#else // defined(OS_MACOSX) #else // defined(OS_MACOSX)
// On other platforms use all logical cores, leaving one for the main // On other platforms use all logical cores, leaving one for the main
// thread. // thread.
total_task_count_ = Max(1, Min(kMaxTasks, num_cores - 1)); task_count_ = Max(1, Min(kMaxTasks, num_cores - 1));
#endif // defined(OS_MACOSX) #endif // defined(OS_MACOSX)
DCHECK_LE(total_task_count_, kMaxTasks);
// One task is for the main thread.
STATIC_ASSERT(kMaxTasks + 1 <= MarkingWorklist::kMaxNumTasks);
} }
// Task id 0 is for the main thread. // Task id 0 is for the main thread.
for (int i = 1; i <= total_task_count_; i++) { for (int i = 1; i <= task_count_; i++) {
if (!is_pending_[i]) { if (!is_pending_[i]) {
if (FLAG_trace_concurrent_marking) { if (FLAG_trace_concurrent_marking) {
heap_->isolate()->PrintWithTimestamp( heap_->isolate()->PrintWithTimestamp(
...@@ -901,7 +899,7 @@ void ConcurrentMarking::ScheduleTasks() { ...@@ -901,7 +899,7 @@ void ConcurrentMarking::ScheduleTasks() {
V8::GetCurrentPlatform()->CallOnWorkerThread(std::move(task)); V8::GetCurrentPlatform()->CallOnWorkerThread(std::move(task));
} }
} }
DCHECK_EQ(total_task_count_, pending_task_count_); DCHECK_EQ(task_count_, pending_task_count_);
} }
void ConcurrentMarking::RescheduleTasksIfNeeded() { void ConcurrentMarking::RescheduleTasksIfNeeded() {
...@@ -909,11 +907,7 @@ void ConcurrentMarking::RescheduleTasksIfNeeded() { ...@@ -909,11 +907,7 @@ void ConcurrentMarking::RescheduleTasksIfNeeded() {
if (heap_->IsTearingDown()) return; if (heap_->IsTearingDown()) return;
{ {
base::MutexGuard guard(&pending_lock_); base::MutexGuard guard(&pending_lock_);
// The total task count is initialized in ScheduleTasks from if (pending_task_count_ > 0) return;
// NumberOfWorkerThreads of the platform.
if (total_task_count_ > 0 && pending_task_count_ == total_task_count_) {
return;
}
} }
if (!shared_->IsGlobalPoolEmpty() || if (!shared_->IsGlobalPoolEmpty() ||
!weak_objects_->current_ephemerons.IsEmpty() || !weak_objects_->current_ephemerons.IsEmpty() ||
...@@ -931,7 +925,7 @@ bool ConcurrentMarking::Stop(StopRequest stop_request) { ...@@ -931,7 +925,7 @@ bool ConcurrentMarking::Stop(StopRequest stop_request) {
if (stop_request != StopRequest::COMPLETE_TASKS_FOR_TESTING) { if (stop_request != StopRequest::COMPLETE_TASKS_FOR_TESTING) {
CancelableTaskManager* task_manager = CancelableTaskManager* task_manager =
heap_->isolate()->cancelable_task_manager(); heap_->isolate()->cancelable_task_manager();
for (int i = 1; i <= total_task_count_; i++) { for (int i = 1; i <= task_count_; i++) {
if (is_pending_[i]) { if (is_pending_[i]) {
if (task_manager->TryAbort(cancelable_id_[i]) == if (task_manager->TryAbort(cancelable_id_[i]) ==
TryAbortResult::kTaskAborted) { TryAbortResult::kTaskAborted) {
...@@ -946,7 +940,7 @@ bool ConcurrentMarking::Stop(StopRequest stop_request) { ...@@ -946,7 +940,7 @@ bool ConcurrentMarking::Stop(StopRequest stop_request) {
while (pending_task_count_ > 0) { while (pending_task_count_ > 0) {
pending_condition_.Wait(&pending_lock_); pending_condition_.Wait(&pending_lock_);
} }
for (int i = 1; i <= total_task_count_; i++) { for (int i = 1; i <= task_count_; i++) {
DCHECK(!is_pending_[i]); DCHECK(!is_pending_[i]);
} }
return true; return true;
...@@ -962,7 +956,7 @@ bool ConcurrentMarking::IsStopped() { ...@@ -962,7 +956,7 @@ bool ConcurrentMarking::IsStopped() {
void ConcurrentMarking::FlushMemoryChunkData( void ConcurrentMarking::FlushMemoryChunkData(
MajorNonAtomicMarkingState* marking_state) { MajorNonAtomicMarkingState* marking_state) {
DCHECK_EQ(pending_task_count_, 0); DCHECK_EQ(pending_task_count_, 0);
for (int i = 1; i <= total_task_count_; i++) { for (int i = 1; i <= task_count_; i++) {
MemoryChunkDataMap& memory_chunk_data = task_state_[i].memory_chunk_data; MemoryChunkDataMap& memory_chunk_data = task_state_[i].memory_chunk_data;
for (auto& pair : memory_chunk_data) { for (auto& pair : memory_chunk_data) {
// ClearLiveness sets the live bytes to zero. // ClearLiveness sets the live bytes to zero.
...@@ -984,7 +978,7 @@ void ConcurrentMarking::FlushMemoryChunkData( ...@@ -984,7 +978,7 @@ void ConcurrentMarking::FlushMemoryChunkData(
} }
void ConcurrentMarking::ClearMemoryChunkData(MemoryChunk* chunk) { void ConcurrentMarking::ClearMemoryChunkData(MemoryChunk* chunk) {
for (int i = 1; i <= total_task_count_; i++) { for (int i = 1; i <= task_count_; i++) {
auto it = task_state_[i].memory_chunk_data.find(chunk); auto it = task_state_[i].memory_chunk_data.find(chunk);
if (it != task_state_[i].memory_chunk_data.end()) { if (it != task_state_[i].memory_chunk_data.end()) {
it->second.live_bytes = 0; it->second.live_bytes = 0;
...@@ -995,7 +989,7 @@ void ConcurrentMarking::ClearMemoryChunkData(MemoryChunk* chunk) { ...@@ -995,7 +989,7 @@ void ConcurrentMarking::ClearMemoryChunkData(MemoryChunk* chunk) {
size_t ConcurrentMarking::TotalMarkedBytes() { size_t ConcurrentMarking::TotalMarkedBytes() {
size_t result = 0; size_t result = 0;
for (int i = 1; i <= total_task_count_; i++) { for (int i = 1; i <= task_count_; i++) {
result += result +=
base::AsAtomicWord::Relaxed_Load<size_t>(&task_state_[i].marked_bytes); base::AsAtomicWord::Relaxed_Load<size_t>(&task_state_[i].marked_bytes);
} }
......
...@@ -86,6 +86,8 @@ class V8_EXPORT_PRIVATE ConcurrentMarking { ...@@ -86,6 +86,8 @@ class V8_EXPORT_PRIVATE ConcurrentMarking {
// scavenge and is going to be re-used. // scavenge and is going to be re-used.
void ClearMemoryChunkData(MemoryChunk* chunk); void ClearMemoryChunkData(MemoryChunk* chunk);
int TaskCount() { return task_count_; }
// Checks if all threads are stopped. // Checks if all threads are stopped.
bool IsStopped(); bool IsStopped();
...@@ -122,7 +124,7 @@ class V8_EXPORT_PRIVATE ConcurrentMarking { ...@@ -122,7 +124,7 @@ class V8_EXPORT_PRIVATE ConcurrentMarking {
int pending_task_count_ = 0; int pending_task_count_ = 0;
bool is_pending_[kMaxTasks + 1] = {}; bool is_pending_[kMaxTasks + 1] = {};
CancelableTaskManager::Id cancelable_id_[kMaxTasks + 1] = {}; CancelableTaskManager::Id cancelable_id_[kMaxTasks + 1] = {};
int total_task_count_ = 0; int task_count_ = 0;
}; };
} // namespace internal } // namespace internal
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment