AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

Revert "Reland "cppgc, heap: Don't eagerly allocate worklist segments""

Browse Source 
This reverts commit f25cb50a2f.

Reason for revert: Fails compilation on gcc https://ci.chromium.org/p/v8/builders/ci/V8%20Linux64%20gcc%20-%20debug/9026?

Original change's description:
> Reland "cppgc, heap: Don't eagerly allocate worklist segments"
> 
> This is a reland of c99147c65e
> 
> Original change's description:
> > cppgc, heap: Don't eagerly allocate worklist segments
> >
> > Bug: chromium:1056170
> > Change-Id: I75a6b5f52bfe8dd71abc086e5d1e060759ad7fc0
> > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2391254
> > Commit-Queue: Omer Katz <omerkatz@chromium.org>
> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> > Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#69778}
> 
> Bug: chromium:1056170
> Change-Id: I4633da065976a6b2710d2f23b946fd2af0e65c83
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2401425
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Commit-Queue: Omer Katz <omerkatz@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#69806}

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

Change-Id: I004173e2a82518a88e68eae3a6f7e96656c0ad7e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:1056170
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2403249
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69809}
This commit is contained in:
Maya Lekova 2020-09-10 13:03:48 +00:00 committed by Commit Bot
parent 608018e557
commit 68b788caf1
5 changed files with 105 additions and 151 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
BUILD.gn
View File

@ -4256,7 +4256,6 @@ v8_source_set("v8_cppgc_shared") {
sources = [
"src/heap/base/stack.cc",
"src/heap/base/stack.h",
"src/heap/base/worklist.cc",
"src/heap/base/worklist.h",
]

13
src/heap/base/worklist.cc
View File

@ -1,13 +0,0 @@
// Copyright 2020 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "src/heap/base/worklist.h"
namespace heap {
namespace base {
namespace internal {
SegmentBase SegmentBase::kSentinelSegment(0);
} // namespace internal
} // namespace base
} // namespace heap

170
src/heap/base/worklist.h
View File

@ -16,29 +16,11 @@
namespace heap {
namespace base {
namespace internal {
class V8_EXPORT_PRIVATE SegmentBase {
public:
static SegmentBase kSentinelSegment;
explicit SegmentBase(uint16_t capacity) : capacity_(capacity) {}
size_t Size() const { return index_; }
bool IsEmpty() const { return index_ == 0; }
bool IsFull() const { return index_ == capacity_; }
void Clear() { index_ = 0; }
protected:
const uint16_t capacity_;
uint16_t index_ = 0;
};
} // namespace internal
// A global marking worklist that is similar the existing Worklist
// but does not reserve space and keep track of the local segments.
// Eventually this will replace Worklist after all its current uses
// are migrated.
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
class Worklist {
public:
static const int kSegmentSize = SegmentSize;
@ -79,33 +61,34 @@ class Worklist {
std::atomic<size_t> size_{0};
};
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
void Worklist<EntryType, SegmentSize>::Push(Segment* segment) {
DCHECK(!segment->IsEmpty());
v8::base::MutexGuard guard(&lock_);
segment->set_next(top_);
set_top(segment);
size_.fetch_add(1, std::memory_order_relaxed);
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
bool Worklist<EntryType, SegmentSize>::Pop(Segment** segment) {
v8::base::MutexGuard guard(&lock_);
if (top_ == nullptr) return false;
if (top_ != nullptr) {
DCHECK_LT(0U, size_);
size_.fetch_sub(1, std::memory_order_relaxed);
*segment = top_;
set_top(top_->next());
return true;
}
return false;
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
bool Worklist<EntryType, SegmentSize>::IsEmpty() {
return v8::base::AsAtomicPtr(&top_)->load(std::memory_order_relaxed) ==
nullptr;
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
size_t Worklist<EntryType, SegmentSize>::Size() {
// It is safe to read |size_| without a lock since this variable is
// atomic, keeping in mind that threads may not immediately see the new
@ -113,7 +96,7 @@ size_t Worklist<EntryType, SegmentSize>::Size() {
return size_.load(std::memory_order_relaxed);
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
void Worklist<EntryType, SegmentSize>::Clear() {
v8::base::MutexGuard guard(&lock_);
size_.store(0, std::memory_order_relaxed);
@ -126,7 +109,7 @@ void Worklist<EntryType, SegmentSize>::Clear() {
set_top(nullptr);
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
template <typename Callback>
void Worklist<EntryType, SegmentSize>::Update(Callback callback) {
v8::base::MutexGuard guard(&lock_);
@ -154,7 +137,7 @@ void Worklist<EntryType, SegmentSize>::Update(Callback callback) {
size_.fetch_sub(num_deleted, std::memory_order_relaxed);
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
template <typename Callback>
void Worklist<EntryType, SegmentSize>::Iterate(Callback callback) {
v8::base::MutexGuard guard(&lock_);
@ -163,7 +146,7 @@ void Worklist<EntryType, SegmentSize>::Iterate(Callback callback) {
}
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
void Worklist<EntryType, SegmentSize>::Merge(
Worklist<EntryType, SegmentSize>* other) {
Segment* top = nullptr;
@ -190,14 +173,19 @@ void Worklist<EntryType, SegmentSize>::Merge(
}
}
template <typename EntryType, uint16_t SegmentSize>
class Worklist<EntryType, SegmentSize>::Segment : public internal::SegmentBase {
template <typename EntryType, int SegmentSize>
class Worklist<EntryType, SegmentSize>::Segment {
public:
static const uint16_t kSize = SegmentSize;
static const size_t kSize = SegmentSize;
Segment() : internal::SegmentBase(kSize) {}
void Push(EntryType entry);
void Pop(EntryType* entry);
Segment() = default;
bool Push(EntryType entry);
bool Pop(EntryType* entry);
size_t Size() const { return index_; }
bool IsEmpty() const { return index_ == 0; }
bool IsFull() const { return index_ == kSize; }
void Clear() { index_ = 0; }
template <typename Callback>
void Update(Callback callback);
@ -209,22 +197,25 @@ class Worklist<EntryType, SegmentSize>::Segment : public internal::SegmentBase {
private:
Segment* next_ = nullptr;
size_t index_ = 0;
EntryType entries_[kSize];
};
template <typename EntryType, uint16_t SegmentSize>
void Worklist<EntryType, SegmentSize>::Segment::Push(EntryType entry) {
DCHECK(!IsFull());
template <typename EntryType, int SegmentSize>
bool Worklist<EntryType, SegmentSize>::Segment::Push(EntryType entry) {
if (IsFull()) return false;
entries_[index_++] = entry;
return true;
}
template <typename EntryType, uint16_t SegmentSize>
void Worklist<EntryType, SegmentSize>::Segment::Pop(EntryType* entry) {
DCHECK(!IsEmpty());
template <typename EntryType, int SegmentSize>
bool Worklist<EntryType, SegmentSize>::Segment::Pop(EntryType* entry) {
if (IsEmpty()) return false;
*entry = entries_[--index_];
return true;
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
template <typename Callback>
void Worklist<EntryType, SegmentSize>::Segment::Update(Callback callback) {
size_t new_index = 0;
@ -236,7 +227,7 @@ void Worklist<EntryType, SegmentSize>::Segment::Update(Callback callback) {
index_ = new_index;
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
template <typename Callback>
void Worklist<EntryType, SegmentSize>::Segment::Iterate(
Callback callback) const {
@ -246,7 +237,7 @@ void Worklist<EntryType, SegmentSize>::Segment::Iterate(
}
// A thread-local view of the marking worklist.
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
class Worklist<EntryType, SegmentSize>::Local {
public:
using ItemType = EntryType;
@ -279,55 +270,32 @@ class Worklist<EntryType, SegmentSize>::Local {
void PublishPushSegment();
void PublishPopSegment();
bool StealPopSegment();
Segment* NewSegment() const {
// Bottleneck for filtering in crash dumps.
return new Segment();
}
void DeleteSegment(internal::SegmentBase* segment) const {
if (segment == &internal::SegmentBase::kSentinelSegment) return;
delete static_cast<Segment*>(segment);
}
inline Segment* push_segment() {
DCHECK_NE(&internal::SegmentBase::kSentinelSegment, push_segment_);
return static_cast<Segment*>(push_segment_);
}
inline const Segment* push_segment() const {
DCHECK_NE(&internal::SegmentBase::kSentinelSegment, push_segment_);
return static_cast<const Segment*>(push_segment_);
}
inline Segment* pop_segment() {
DCHECK_NE(&internal::SegmentBase::kSentinelSegment, pop_segment_);
return static_cast<Segment*>(pop_segment_);
}
inline const Segment* pop_segment() const {
DCHECK_NE(&internal::SegmentBase::kSentinelSegment, pop_segment_);
return static_cast<const Segment*>(pop_segment_);
}
Worklist<EntryType, SegmentSize>* worklist_ = nullptr;
internal::SegmentBase* push_segment_ = nullptr;
internal::SegmentBase* pop_segment_ = nullptr;
Segment* push_segment_ = nullptr;
Segment* pop_segment_ = nullptr;
};
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
Worklist<EntryType, SegmentSize>::Local::Local(
Worklist<EntryType, SegmentSize>* worklist)
: worklist_(worklist),
push_segment_(&internal::SegmentBase::kSentinelSegment),
pop_segment_(&internal::SegmentBase::kSentinelSegment) {}
push_segment_(NewSegment()),
pop_segment_(NewSegment()) {}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
Worklist<EntryType, SegmentSize>::Local::~Local() {
CHECK_IMPLIES(push_segment_, push_segment_->IsEmpty());
CHECK_IMPLIES(pop_segment_, pop_segment_->IsEmpty());
DeleteSegment(push_segment_);
DeleteSegment(pop_segment_);
delete push_segment_;
delete pop_segment_;
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
Worklist<EntryType, SegmentSize>::Local::Local(
Worklist<EntryType, SegmentSize>::Local&& other) V8_NOEXCEPT {
worklist_ = other.worklist_;
@ -338,7 +306,7 @@ Worklist<EntryType, SegmentSize>::Local::Local(
other.pop_segment_ = nullptr;
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
typename Worklist<EntryType, SegmentSize>::Local&
Worklist<EntryType, SegmentSize>::Local::operator=(
Worklist<EntryType, SegmentSize>::Local&& other) V8_NOEXCEPT {
@ -356,75 +324,81 @@ Worklist<EntryType, SegmentSize>::Local::operator=(
return *this;
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
void Worklist<EntryType, SegmentSize>::Local::Push(EntryType entry) {
if (V8_UNLIKELY(push_segment_->IsFull())) {
if (V8_UNLIKELY(!push_segment_->Push(entry))) {
PublishPushSegment();
bool success = push_segment_->Push(entry);
USE(success);
DCHECK(success);
}
push_segment()->Push(entry);
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
bool Worklist<EntryType, SegmentSize>::Local::Pop(EntryType* entry) {
if (pop_segment_->IsEmpty()) {
if (!pop_segment_->Pop(entry)) {
if (!push_segment_->IsEmpty()) {
std::swap(push_segment_, pop_segment_);
} else if (!StealPopSegment()) {
return false;
}
bool success = pop_segment_->Pop(entry);
USE(success);
DCHECK(success);
}
pop_segment()->Pop(entry);
return true;
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
bool Worklist<EntryType, SegmentSize>::Local::IsLocalAndGlobalEmpty() const {
return IsLocalEmpty() && IsGlobalEmpty();
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
bool Worklist<EntryType, SegmentSize>::Local::IsLocalEmpty() const {
return push_segment_->IsEmpty() && pop_segment_->IsEmpty();
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
bool Worklist<EntryType, SegmentSize>::Local::IsGlobalEmpty() const {
return worklist_->IsEmpty();
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
void Worklist<EntryType, SegmentSize>::Local::Publish() {
if (!push_segment_->IsEmpty()) PublishPushSegment();
if (!pop_segment_->IsEmpty()) PublishPopSegment();
if (!push_segment_->IsEmpty()) {
PublishPushSegment();
}
if (!pop_segment_->IsEmpty()) {
PublishPopSegment();
}
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
void Worklist<EntryType, SegmentSize>::Local::Merge(
Worklist<EntryType, SegmentSize>::Local* other) {
other->Publish();
worklist_->Merge(other->worklist_);
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
void Worklist<EntryType, SegmentSize>::Local::PublishPushSegment() {
if (push_segment_ != &internal::SegmentBase::kSentinelSegment)
worklist_->Push(push_segment());
worklist_->Push(push_segment_);
push_segment_ = NewSegment();
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
void Worklist<EntryType, SegmentSize>::Local::PublishPopSegment() {
if (pop_segment_ != &internal::SegmentBase::kSentinelSegment)
worklist_->Push(pop_segment());
worklist_->Push(pop_segment_);
pop_segment_ = NewSegment();
}
template <typename EntryType, uint16_t SegmentSize>
template <typename EntryType, int SegmentSize>
bool Worklist<EntryType, SegmentSize>::Local::StealPopSegment() {
if (worklist_->IsEmpty()) return false;
Segment* new_segment = nullptr;
if (worklist_->Pop(&new_segment)) {
DeleteSegment(pop_segment_);
delete pop_segment_;
pop_segment_ = new_segment;
return true;
}

2
test/unittests/BUILD.gn
View File

@ -69,7 +69,6 @@ v8_executable("cppgc_unittests") {
deps = [
":cppgc_unittests_sources",
":v8_cppgc_shared_unittests_sources",
"../..:cppgc_for_testing",
"//testing/gmock",
"//testing/gtest",
@ -137,7 +136,6 @@ v8_executable("unittests") {
deps = [
":cppgc_unittests_sources",
":unittests_sources",
":v8_cppgc_shared_unittests_sources",
"../..:v8_for_testing",
"../..:v8_libbase",
"../..:v8_libplatform",

60
test/unittests/heap/base/worklist-unittest.cc
View File

@ -4,7 +4,7 @@
#include "src/heap/base/worklist.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "test/unittests/heap/cppgc/tests.h"
namespace heap {
namespace base {
@ -23,17 +23,17 @@ TEST(CppgcWorkListTest, SegmentCreate) {
TEST(CppgcWorkListTest, SegmentPush) {
TestWorklist::Segment segment;
EXPECT_EQ(0u, segment.Size());
segment.Push(nullptr);
EXPECT_TRUE(segment.Push(nullptr));
EXPECT_EQ(1u, segment.Size());
}
TEST(CppgcWorkListTest, SegmentPushPop) {
TestWorklist::Segment segment;
segment.Push(nullptr);
EXPECT_TRUE(segment.Push(nullptr));
EXPECT_EQ(1u, segment.Size());
SomeObject dummy;
SomeObject* object = &dummy;
segment.Pop(&object);
EXPECT_TRUE(segment.Pop(&object));
EXPECT_EQ(0u, segment.Size());
EXPECT_EQ(nullptr, object);
}
@ -41,7 +41,7 @@ TEST(CppgcWorkListTest, SegmentPushPop) {
TEST(CppgcWorkListTest, SegmentIsEmpty) {
TestWorklist::Segment segment;
EXPECT_TRUE(segment.IsEmpty());
segment.Push(nullptr);
EXPECT_TRUE(segment.Push(nullptr));
EXPECT_FALSE(segment.IsEmpty());
}
@ -49,27 +49,44 @@ TEST(CppgcWorkListTest, SegmentIsFull) {
TestWorklist::Segment segment;
EXPECT_FALSE(segment.IsFull());
for (size_t i = 0; i < TestWorklist::Segment::kSize; i++) {
segment.Push(nullptr);
EXPECT_TRUE(segment.Push(nullptr));
}
EXPECT_TRUE(segment.IsFull());
}
TEST(CppgcWorkListTest, SegmentClear) {
TestWorklist::Segment segment;
segment.Push(nullptr);
EXPECT_TRUE(segment.Push(nullptr));
EXPECT_FALSE(segment.IsEmpty());
segment.Clear();
EXPECT_TRUE(segment.IsEmpty());
for (size_t i = 0; i < TestWorklist::Segment::kSize; i++) {
segment.Push(nullptr);
EXPECT_TRUE(segment.Push(nullptr));
}
}
TEST(CppgcWorkListTest, SegmentFullPushFails) {
TestWorklist::Segment segment;
EXPECT_FALSE(segment.IsFull());
for (size_t i = 0; i < TestWorklist::Segment::kSize; i++) {
EXPECT_TRUE(segment.Push(nullptr));
}
EXPECT_TRUE(segment.IsFull());
EXPECT_FALSE(segment.Push(nullptr));
}
TEST(CppgcWorkListTest, SegmentEmptyPopFails) {
TestWorklist::Segment segment;
EXPECT_TRUE(segment.IsEmpty());
SomeObject* object;
EXPECT_FALSE(segment.Pop(&object));
}
TEST(CppgcWorkListTest, SegmentUpdateFalse) {
TestWorklist::Segment segment;
SomeObject* object;
object = reinterpret_cast<SomeObject*>(&object);
segment.Push(object);
EXPECT_TRUE(segment.Push(object));
segment.Update([](SomeObject* object, SomeObject** out) { return false; });
EXPECT_TRUE(segment.IsEmpty());
}
@ -80,13 +97,13 @@ TEST(CppgcWorkListTest, SegmentUpdate) {
objectA = reinterpret_cast<SomeObject*>(&objectA);
SomeObject* objectB;
objectB = reinterpret_cast<SomeObject*>(&objectB);
segment.Push(objectA);
EXPECT_TRUE(segment.Push(objectA));
segment.Update([objectB](SomeObject* object, SomeObject** out) {
*out = objectB;
return true;
});
SomeObject* object;
segment.Pop(&object);
EXPECT_TRUE(segment.Pop(&object));
EXPECT_EQ(object, objectB);
}
@ -307,26 +324,5 @@ TEST(CppgcWorkListTest, MergeGlobalPool) {
EXPECT_TRUE(worklist2.IsEmpty());
}
#ifdef DEBUG
TEST(CppgcWorkListDeathTest, DiesOnPushToFullSegment) {
TestWorklist::Segment segment;
EXPECT_FALSE(segment.IsFull());
for (size_t i = 0; i < TestWorklist::Segment::kSize; i++) {
segment.Push(nullptr);
}
EXPECT_TRUE(segment.IsFull());
EXPECT_DEATH_IF_SUPPORTED(segment.Push(nullptr), "");
}
TEST(CppgcWorkListDeathTest, DiesOnPopFromEmptySegment) {
TestWorklist::Segment segment;
EXPECT_TRUE(segment.IsEmpty());
SomeObject* object;
EXPECT_DEATH_IF_SUPPORTED(segment.Pop(&object), "");
}
#endif // DEBUG
} // namespace base
} // namespace heap