diff --git a/BUILD.gn b/BUILD.gn index 7ab4e81949..dfd64d3bd5 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -4256,6 +4256,7 @@ 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", ] diff --git a/src/heap/base/worklist.cc b/src/heap/base/worklist.cc new file mode 100644 index 0000000000..f4360d7d82 --- /dev/null +++ b/src/heap/base/worklist.cc @@ -0,0 +1,13 @@ +// 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 diff --git a/src/heap/base/worklist.h b/src/heap/base/worklist.h index 490bbee3a7..2c5feb09b1 100644 --- a/src/heap/base/worklist.h +++ b/src/heap/base/worklist.h @@ -16,11 +16,29 @@ 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 +template class Worklist { public: static const int kSegmentSize = SegmentSize; @@ -61,34 +79,33 @@ class Worklist { std::atomic size_{0}; }; -template +template void Worklist::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 +template bool Worklist::Pop(Segment** segment) { v8::base::MutexGuard guard(&lock_); - 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; + if (top_ == nullptr) return false; + DCHECK_LT(0U, size_); + size_.fetch_sub(1, std::memory_order_relaxed); + *segment = top_; + set_top(top_->next()); + return true; } -template +template bool Worklist::IsEmpty() { return v8::base::AsAtomicPtr(&top_)->load(std::memory_order_relaxed) == nullptr; } -template +template size_t Worklist::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 @@ -96,7 +113,7 @@ size_t Worklist::Size() { return size_.load(std::memory_order_relaxed); } -template +template void Worklist::Clear() { v8::base::MutexGuard guard(&lock_); size_.store(0, std::memory_order_relaxed); @@ -109,7 +126,7 @@ void Worklist::Clear() { set_top(nullptr); } -template +template template void Worklist::Update(Callback callback) { v8::base::MutexGuard guard(&lock_); @@ -137,7 +154,7 @@ void Worklist::Update(Callback callback) { size_.fetch_sub(num_deleted, std::memory_order_relaxed); } -template +template template void Worklist::Iterate(Callback callback) { v8::base::MutexGuard guard(&lock_); @@ -146,7 +163,7 @@ void Worklist::Iterate(Callback callback) { } } -template +template void Worklist::Merge( Worklist* other) { Segment* top = nullptr; @@ -173,19 +190,13 @@ void Worklist::Merge( } } -template -class Worklist::Segment { +template +class Worklist::Segment : public internal::SegmentBase { public: - static const size_t kSize = SegmentSize; + static const uint16_t kSize = SegmentSize; - 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; } + void Push(EntryType entry); + void Pop(EntryType* entry); template void Update(Callback callback); @@ -196,26 +207,36 @@ class Worklist::Segment { void set_next(Segment* segment) { next_ = segment; } private: + Segment() : internal::SegmentBase(kSize) {} + Segment* next_ = nullptr; - size_t index_ = 0; EntryType entries_[kSize]; + + friend class Worklist::Local; + + FRIEND_TEST(CppgcWorkListTest, SegmentCreate); + FRIEND_TEST(CppgcWorkListTest, SegmentPush); + FRIEND_TEST(CppgcWorkListTest, SegmentPushPop); + FRIEND_TEST(CppgcWorkListTest, SegmentIsEmpty); + FRIEND_TEST(CppgcWorkListTest, SegmentIsFull); + FRIEND_TEST(CppgcWorkListTest, SegmentClear); + FRIEND_TEST(CppgcWorkListTest, SegmentUpdateFalse); + FRIEND_TEST(CppgcWorkListTest, SegmentUpdate); }; -template -bool Worklist::Segment::Push(EntryType entry) { - if (IsFull()) return false; +template +void Worklist::Segment::Push(EntryType entry) { + DCHECK(!IsFull()); entries_[index_++] = entry; - return true; } -template -bool Worklist::Segment::Pop(EntryType* entry) { - if (IsEmpty()) return false; +template +void Worklist::Segment::Pop(EntryType* entry) { + DCHECK(!IsEmpty()); *entry = entries_[--index_]; - return true; } -template +template template void Worklist::Segment::Update(Callback callback) { size_t new_index = 0; @@ -227,7 +248,7 @@ void Worklist::Segment::Update(Callback callback) { index_ = new_index; } -template +template template void Worklist::Segment::Iterate( Callback callback) const { @@ -237,7 +258,7 @@ void Worklist::Segment::Iterate( } // A thread-local view of the marking worklist. -template +template class Worklist::Local { public: using ItemType = EntryType; @@ -270,32 +291,55 @@ class Worklist::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); + } + + inline Segment* push_segment() { + DCHECK_NE(&internal::SegmentBase::kSentinelSegment, push_segment_); + return static_cast(push_segment_); + } + inline const Segment* push_segment() const { + DCHECK_NE(&internal::SegmentBase::kSentinelSegment, push_segment_); + return static_cast(push_segment_); + } + + inline Segment* pop_segment() { + DCHECK_NE(&internal::SegmentBase::kSentinelSegment, pop_segment_); + return static_cast(pop_segment_); + } + inline const Segment* pop_segment() const { + DCHECK_NE(&internal::SegmentBase::kSentinelSegment, pop_segment_); + return static_cast(pop_segment_); + } Worklist* worklist_ = nullptr; - Segment* push_segment_ = nullptr; - Segment* pop_segment_ = nullptr; + internal::SegmentBase* push_segment_ = nullptr; + internal::SegmentBase* pop_segment_ = nullptr; }; -template +template Worklist::Local::Local( Worklist* worklist) : worklist_(worklist), - push_segment_(NewSegment()), - pop_segment_(NewSegment()) {} + push_segment_(&internal::SegmentBase::kSentinelSegment), + pop_segment_(&internal::SegmentBase::kSentinelSegment) {} -template +template Worklist::Local::~Local() { CHECK_IMPLIES(push_segment_, push_segment_->IsEmpty()); CHECK_IMPLIES(pop_segment_, pop_segment_->IsEmpty()); - delete push_segment_; - delete pop_segment_; + DeleteSegment(push_segment_); + DeleteSegment(pop_segment_); } -template +template Worklist::Local::Local( Worklist::Local&& other) V8_NOEXCEPT { worklist_ = other.worklist_; @@ -306,7 +350,7 @@ Worklist::Local::Local( other.pop_segment_ = nullptr; } -template +template typename Worklist::Local& Worklist::Local::operator=( Worklist::Local&& other) V8_NOEXCEPT { @@ -324,81 +368,75 @@ Worklist::Local::operator=( return *this; } -template +template void Worklist::Local::Push(EntryType entry) { - if (V8_UNLIKELY(!push_segment_->Push(entry))) { + if (V8_UNLIKELY(push_segment_->IsFull())) { PublishPushSegment(); - bool success = push_segment_->Push(entry); - USE(success); - DCHECK(success); } + push_segment()->Push(entry); } -template +template bool Worklist::Local::Pop(EntryType* entry) { - if (!pop_segment_->Pop(entry)) { + if (pop_segment_->IsEmpty()) { 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 +template bool Worklist::Local::IsLocalAndGlobalEmpty() const { return IsLocalEmpty() && IsGlobalEmpty(); } -template +template bool Worklist::Local::IsLocalEmpty() const { return push_segment_->IsEmpty() && pop_segment_->IsEmpty(); } -template +template bool Worklist::Local::IsGlobalEmpty() const { return worklist_->IsEmpty(); } -template +template void Worklist::Local::Publish() { - if (!push_segment_->IsEmpty()) { - PublishPushSegment(); - } - if (!pop_segment_->IsEmpty()) { - PublishPopSegment(); - } + if (!push_segment_->IsEmpty()) PublishPushSegment(); + if (!pop_segment_->IsEmpty()) PublishPopSegment(); } -template +template void Worklist::Local::Merge( Worklist::Local* other) { other->Publish(); worklist_->Merge(other->worklist_); } -template +template void Worklist::Local::PublishPushSegment() { - worklist_->Push(push_segment_); + if (push_segment_ != &internal::SegmentBase::kSentinelSegment) + worklist_->Push(push_segment()); push_segment_ = NewSegment(); } -template +template void Worklist::Local::PublishPopSegment() { - worklist_->Push(pop_segment_); + if (pop_segment_ != &internal::SegmentBase::kSentinelSegment) + worklist_->Push(pop_segment()); pop_segment_ = NewSegment(); } -template +template bool Worklist::Local::StealPopSegment() { if (worklist_->IsEmpty()) return false; Segment* new_segment = nullptr; if (worklist_->Pop(&new_segment)) { - delete pop_segment_; + DeleteSegment(pop_segment_); pop_segment_ = new_segment; return true; } diff --git a/test/unittests/BUILD.gn b/test/unittests/BUILD.gn index 6511f4984d..5b2cc7b289 100644 --- a/test/unittests/BUILD.gn +++ b/test/unittests/BUILD.gn @@ -69,6 +69,7 @@ v8_executable("cppgc_unittests") { deps = [ ":cppgc_unittests_sources", + ":v8_cppgc_shared_unittests_sources", "../..:cppgc_for_testing", "//testing/gmock", "//testing/gtest", @@ -136,6 +137,7 @@ v8_executable("unittests") { deps = [ ":cppgc_unittests_sources", ":unittests_sources", + ":v8_cppgc_shared_unittests_sources", "../..:v8_for_testing", "../..:v8_libbase", "../..:v8_libplatform", diff --git a/test/unittests/heap/base/worklist-unittest.cc b/test/unittests/heap/base/worklist-unittest.cc index 35ca545be1..ae737a7aa3 100644 --- a/test/unittests/heap/base/worklist-unittest.cc +++ b/test/unittests/heap/base/worklist-unittest.cc @@ -4,7 +4,7 @@ #include "src/heap/base/worklist.h" -#include "test/unittests/heap/cppgc/tests.h" +#include "testing/gtest/include/gtest/gtest.h" namespace heap { namespace base { @@ -23,17 +23,17 @@ TEST(CppgcWorkListTest, SegmentCreate) { TEST(CppgcWorkListTest, SegmentPush) { TestWorklist::Segment segment; EXPECT_EQ(0u, segment.Size()); - EXPECT_TRUE(segment.Push(nullptr)); + segment.Push(nullptr); EXPECT_EQ(1u, segment.Size()); } TEST(CppgcWorkListTest, SegmentPushPop) { TestWorklist::Segment segment; - EXPECT_TRUE(segment.Push(nullptr)); + segment.Push(nullptr); EXPECT_EQ(1u, segment.Size()); SomeObject dummy; SomeObject* object = &dummy; - EXPECT_TRUE(segment.Pop(&object)); + 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()); - EXPECT_TRUE(segment.Push(nullptr)); + segment.Push(nullptr); EXPECT_FALSE(segment.IsEmpty()); } @@ -49,44 +49,27 @@ TEST(CppgcWorkListTest, SegmentIsFull) { TestWorklist::Segment segment; EXPECT_FALSE(segment.IsFull()); for (size_t i = 0; i < TestWorklist::Segment::kSize; i++) { - EXPECT_TRUE(segment.Push(nullptr)); + segment.Push(nullptr); } EXPECT_TRUE(segment.IsFull()); } TEST(CppgcWorkListTest, SegmentClear) { TestWorklist::Segment segment; - EXPECT_TRUE(segment.Push(nullptr)); + segment.Push(nullptr); EXPECT_FALSE(segment.IsEmpty()); segment.Clear(); EXPECT_TRUE(segment.IsEmpty()); for (size_t i = 0; i < TestWorklist::Segment::kSize; i++) { - EXPECT_TRUE(segment.Push(nullptr)); + 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(&object); - EXPECT_TRUE(segment.Push(object)); + segment.Push(object); segment.Update([](SomeObject* object, SomeObject** out) { return false; }); EXPECT_TRUE(segment.IsEmpty()); } @@ -97,13 +80,13 @@ TEST(CppgcWorkListTest, SegmentUpdate) { objectA = reinterpret_cast(&objectA); SomeObject* objectB; objectB = reinterpret_cast(&objectB); - EXPECT_TRUE(segment.Push(objectA)); + segment.Push(objectA); segment.Update([objectB](SomeObject* object, SomeObject** out) { *out = objectB; return true; }); SomeObject* object; - EXPECT_TRUE(segment.Pop(&object)); + segment.Pop(&object); EXPECT_EQ(object, objectB); }