From d57802633c26524d51311cc4c5df2c6ef02d7aff Mon Sep 17 00:00:00 2001 From: Alexander Shabalin Date: Wed, 13 Jan 2021 14:37:41 +0300 Subject: [PATCH] Use Alloc.h everywhere (#4611) --- kotlin-native/codestyle/cpp/README.md | 4 + .../runtime/src/legacymm/cpp/Memory.cpp | 2 +- kotlin-native/runtime/src/main/cpp/Alloc.h | 38 +++++ .../runtime/src/main/cpp/AllocTest.cpp | 133 ++++++++++++++++++ .../runtime/src/main/cpp/CleanerTest.cpp | 6 +- .../runtime/src/main/cpp/CppSupport.hpp | 6 - .../runtime/src/main/cpp/MultiSourceQueue.hpp | 23 +-- .../src/main/cpp/MultiSourceQueueTest.cpp | 19 +-- .../runtime/src/main/cpp/SingleLockList.hpp | 97 +++++++++---- .../src/main/cpp/SingleLockListTest.cpp | 83 ++++++++--- .../main/cpp/TestSupportCompilerGenerated.hpp | 5 +- kotlin-native/runtime/src/main/cpp/Types.h | 10 ++ kotlin-native/runtime/src/main/cpp/Worker.cpp | 2 +- .../runtime/src/mm/cpp/ExtraObjectData.hpp | 3 +- .../runtime/src/mm/cpp/GlobalsRegistry.hpp | 2 +- .../runtime/src/mm/cpp/ObjectFactory.hpp | 27 ++-- .../runtime/src/mm/cpp/ObjectFactoryTest.cpp | 39 ++--- .../runtime/src/mm/cpp/StableRefRegistry.hpp | 2 +- .../runtime/src/mm/cpp/ThreadLocalStorage.hpp | 11 +- .../src/mm/cpp/ThreadLocalStorageTest.cpp | 20 +-- 20 files changed, 403 insertions(+), 129 deletions(-) create mode 100644 kotlin-native/runtime/src/main/cpp/AllocTest.cpp diff --git a/kotlin-native/codestyle/cpp/README.md b/kotlin-native/codestyle/cpp/README.md index c855ad8a30a..bcefcadee6e 100644 --- a/kotlin-native/codestyle/cpp/README.md +++ b/kotlin-native/codestyle/cpp/README.md @@ -21,6 +21,10 @@ * Put implementation details inside `.h`/`.hpp` into a nested `namespace internal` (e.g. implementation details of module `mm` go into `namespace kotlin { namespace mm { namespace internal { ... } } }`) * Put implementation details inside `.cpp`/`.mm` into a global anonymous `namespace` * For `extern "C"` declarations emulate namespaces with `Kotlin_[module_name]_` prefixes. +* To mark type as move-only, privately inherit from `kotlin::MoveOnly` +* To mark type unmovable and uncopyable, privately inherit from `kotlin::Pinned` +* All heap-allocated classes should publicly inherit from `KonanAllocatorAware` +* Use `KStd*` containers and smart pointers instead of `std::*` ones. ## Naming diff --git a/kotlin-native/runtime/src/legacymm/cpp/Memory.cpp b/kotlin-native/runtime/src/legacymm/cpp/Memory.cpp index 4a1f63ec95a..28e8b008bf7 100644 --- a/kotlin-native/runtime/src/legacymm/cpp/Memory.cpp +++ b/kotlin-native/runtime/src/legacymm/cpp/Memory.cpp @@ -192,7 +192,7 @@ struct CycleDetectorRootset { KStdVector heldRefs; }; -class CycleDetector : private kotlin::Pinned { +class CycleDetector : private kotlin::Pinned, public KonanAllocatorAware { public: static void insertCandidateIfNeeded(KRef object) { if (canBeACandidate(object)) diff --git a/kotlin-native/runtime/src/main/cpp/Alloc.h b/kotlin-native/runtime/src/main/cpp/Alloc.h index 8c853edf93f..7f9c6042ddc 100644 --- a/kotlin-native/runtime/src/main/cpp/Alloc.h +++ b/kotlin-native/runtime/src/main/cpp/Alloc.h @@ -119,4 +119,42 @@ bool operator!=( return !(x == y); } +template +class KonanDeleter { +public: + void operator()(T* instance) noexcept { konanDestructInstance(instance); } +}; + +// Force a class to be heap-allocated using `konanAllocMemory`. Does not prevent stack allocation, or +// allocation as part of another object. +// Usage: +// class A : public KonanAllocatorAware { +// ... +// }; +class KonanAllocatorAware { +public: + static void* operator new(size_t count) noexcept { return konanAllocMemory(count); } + static void* operator new[](size_t count) noexcept { return konanAllocMemory(count); } + + static void* operator new(size_t count, void* ptr) noexcept { return ptr; } + static void* operator new[](size_t count, void* ptr) noexcept { return ptr; } + + static void operator delete(void* ptr) noexcept { konanFreeMemory(ptr); } + static void operator delete[](void* ptr) noexcept { konanFreeMemory(ptr); } + +protected: + // Hide constructors, assignments and destructor to discourage operating on instance of `KonanAllocatorAware` + KonanAllocatorAware() = default; + + KonanAllocatorAware(const KonanAllocatorAware&) = default; + KonanAllocatorAware(KonanAllocatorAware&&) = default; + + KonanAllocatorAware& operator=(const KonanAllocatorAware&) = default; + KonanAllocatorAware& operator=(KonanAllocatorAware&&) = default; + + // Not virtual by design. Since this class hides this destructor, no one can destroy an + // instance of `KonanAllocatorAware` directly, so this destructor is never called in a virtual manner. + ~KonanAllocatorAware() = default; +}; + #endif // RUNTIME_ALLOC_H diff --git a/kotlin-native/runtime/src/main/cpp/AllocTest.cpp b/kotlin-native/runtime/src/main/cpp/AllocTest.cpp new file mode 100644 index 00000000000..7ba0fb9be48 --- /dev/null +++ b/kotlin-native/runtime/src/main/cpp/AllocTest.cpp @@ -0,0 +1,133 @@ +/* + * Copyright 2010-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license + * that can be found in the LICENSE file. + */ + +#include "Alloc.h" + +#include + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +#include "Types.h" + +namespace { + +class A : public KonanAllocatorAware { +public: + using DestructorHook = testing::StrictMock>; + + static thread_local DestructorHook* destructorHook; + + explicit A(int value = -1) : value_(value) {} + + ~A() { destructorHook->Call(value_); } + + int value() const { return value_; } + + bool operator==(const A& rhs) const { return value_ == rhs.value_; } + +private: + int value_; +}; + +// static +thread_local A::DestructorHook* A::destructorHook = nullptr; + +struct B { + explicit B(int value) : a(value) {} + + A a; +}; + +} // namespace + +class KonanAllocatorAwareTest : public testing::Test { +public: + KStdUniquePtr destructorHook; + + void SetUp() override { + Test::SetUp(); + + destructorHook = make_unique(); + A::destructorHook = destructorHook.get(); + } + + void TearDown() override { + A::destructorHook = nullptr; + destructorHook.reset(); + + Test::TearDown(); + } +}; + +TEST_F(KonanAllocatorAwareTest, AllocatedOnStack) { + A a(42); + EXPECT_THAT(a.value(), 42); + EXPECT_CALL(*destructorHook, Call(42)); +} + +TEST_F(KonanAllocatorAwareTest, AllocatedInAnotherObject) { + // We do not control how `B` is allocated. + B* b = new B(42); + EXPECT_THAT(b->a.value(), 42); + EXPECT_CALL(*destructorHook, Call(42)); + delete b; +} + +TEST_F(KonanAllocatorAwareTest, AllocatedByItself) { + A* a = new A(42); + EXPECT_THAT(a->value(), 42); + EXPECT_CALL(*destructorHook, Call(42)); + delete a; +} + +TEST_F(KonanAllocatorAwareTest, AllocateArray) { + constexpr size_t kCount = 5; + A* as = new A[kCount]; + + std::vector actual; + for (A* a = as; a != as + kCount; ++a) { + actual.push_back(a->value()); + } + std::array expected; + for (int& element : expected) { + element = -1; + } + EXPECT_THAT(actual, testing::ElementsAreArray(expected)); + + EXPECT_CALL(*destructorHook, Call(-1)).Times(kCount); + delete[] as; +} + +TEST_F(KonanAllocatorAwareTest, PlacementAllocated) { + std::array buffer; + A* a = new (buffer.data()) A(42); + EXPECT_THAT(a->value(), 42); + EXPECT_CALL(*destructorHook, Call(42)); + a->~A(); + testing::Mock::VerifyAndClearExpectations(destructorHook.get()); +} + +TEST_F(KonanAllocatorAwareTest, PlacementConstructedArray) { + constexpr size_t kCount = 5; + std::array buffer; + A* as = new (buffer.data()) A[kCount]; + + std::vector actual; + for (A* a = as; a != as + kCount; ++a) { + actual.push_back(a->value()); + } + std::array expected; + for (int& element : expected) { + element = -1; + } + EXPECT_THAT(actual, testing::ElementsAreArray(expected)); + + EXPECT_CALL(*destructorHook, Call(-1)).Times(kCount); + for (A* a = as; a != as + kCount; ++a) { + a->~A(); + } + testing::Mock::VerifyAndClearExpectations(destructorHook.get()); +} diff --git a/kotlin-native/runtime/src/main/cpp/CleanerTest.cpp b/kotlin-native/runtime/src/main/cpp/CleanerTest.cpp index ca36bcd109a..fa5746142b7 100644 --- a/kotlin-native/runtime/src/main/cpp/CleanerTest.cpp +++ b/kotlin-native/runtime/src/main/cpp/CleanerTest.cpp @@ -6,7 +6,6 @@ #include "Cleaner.h" #include -#include #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -14,6 +13,7 @@ #include "Atomic.h" #include "TestSupport.hpp" #include "TestSupportCompilerGenerated.hpp" +#include "Types.h" using testing::_; @@ -31,7 +31,7 @@ TEST(CleanerTest, ConcurrentCreation) { int startedThreads = 0; bool allowRunning = false; - std::vector> futures; + KStdVector> futures; for (int i = 0; i < threadCount; ++i) { auto future = std::async(std::launch::async, [&startedThreads, &allowRunning]() { atomicAdd(&startedThreads, 1); @@ -44,7 +44,7 @@ TEST(CleanerTest, ConcurrentCreation) { while (atomicGet(&startedThreads) != threadCount) { } atomicSet(&allowRunning, true); - std::vector values; + KStdVector values; for (auto& future : futures) { values.push_back(future.get()); } diff --git a/kotlin-native/runtime/src/main/cpp/CppSupport.hpp b/kotlin-native/runtime/src/main/cpp/CppSupport.hpp index 9f04f544c41..e44f9fdc14b 100644 --- a/kotlin-native/runtime/src/main/cpp/CppSupport.hpp +++ b/kotlin-native/runtime/src/main/cpp/CppSupport.hpp @@ -7,7 +7,6 @@ #define RUNTIME_CPP_SUPPORT_H #include -#include // A collection of backported utilities from future C++ versions. @@ -16,11 +15,6 @@ namespace std_support { ////////////////////////// C++14 ////////////////////////// -template -std::unique_ptr make_unique(Args&&... args) { - return std::unique_ptr(new T(std::forward(args)...)); -} - template using make_unsigned_t = typename std::make_unsigned::type; diff --git a/kotlin-native/runtime/src/main/cpp/MultiSourceQueue.hpp b/kotlin-native/runtime/src/main/cpp/MultiSourceQueue.hpp index 9352006a424..31aecaf7e71 100644 --- a/kotlin-native/runtime/src/main/cpp/MultiSourceQueue.hpp +++ b/kotlin-native/runtime/src/main/cpp/MultiSourceQueue.hpp @@ -11,6 +11,7 @@ #include #include "Mutex.hpp" +#include "Types.h" namespace kotlin { @@ -20,9 +21,9 @@ class MultiSourceQueue { public: class Producer; - // TODO: Consider switching from `std::list` to `SingleLockList` to hide the constructor + // TODO: Consider switching from `KStdList` to `SingleLockList` to hide the constructor // and to not store the iterator. - class Node : private Pinned { + class Node : private Pinned, public KonanAllocatorAware { public: Node(const T& value, Producer* owner) noexcept : value_(value), owner_(owner) {} @@ -33,7 +34,7 @@ public: T value_; std::atomic owner_; // `nullptr` signifies that `MultiSourceQueue` owns it. - typename std::list::iterator position_; + typename KStdList::iterator position_; }; class Producer { @@ -72,8 +73,8 @@ public: private: MultiSourceQueue& owner_; // weak - std::list queue_; - std::list deletionQueue_; + KStdList queue_; + KStdList deletionQueue_; }; class Iterator { @@ -92,9 +93,9 @@ public: private: friend class MultiSourceQueue; - explicit Iterator(const typename std::list::iterator& position) noexcept : position_(position) {} + explicit Iterator(const typename KStdList::iterator& position) noexcept : position_(position) {} - typename std::list::iterator position_; + typename KStdList::iterator position_; }; class Iterable : MoveOnly { @@ -118,7 +119,7 @@ public: // Lock `MultiSourceQueue` and apply deletions. Only deletes elements that were published. void ApplyDeletions() noexcept { std::lock_guard guard(mutex_); - std::list remainingDeletions; + KStdList remainingDeletions; auto it = deletionQueue_.begin(); while (it != deletionQueue_.end()) { @@ -139,10 +140,10 @@ public: } private: - // Using `std::list` as it allows to implement `Collect` without memory allocations, + // Using `KStdList` as it allows to implement `Collect` without memory allocations, // which is important for GC mark phase. - std::list queue_; - std::list deletionQueue_; + KStdList queue_; + KStdList deletionQueue_; SpinLock mutex_; }; diff --git a/kotlin-native/runtime/src/main/cpp/MultiSourceQueueTest.cpp b/kotlin-native/runtime/src/main/cpp/MultiSourceQueueTest.cpp index 62310bf19ee..7f3e7a2c7af 100644 --- a/kotlin-native/runtime/src/main/cpp/MultiSourceQueueTest.cpp +++ b/kotlin-native/runtime/src/main/cpp/MultiSourceQueueTest.cpp @@ -12,14 +12,15 @@ #include "gtest/gtest.h" #include "TestSupport.hpp" +#include "Types.h" using namespace kotlin; namespace { template -std::vector Collect(MultiSourceQueue& queue) { - std::vector result; +KStdVector Collect(MultiSourceQueue& queue) { + KStdVector result; for (const auto& element : queue.Iter()) { result.push_back(element); } @@ -192,8 +193,8 @@ TEST(MultiSourceQueueTest, ConcurrentPublish) { constexpr int kThreadCount = kDefaultThreadCount; std::atomic canStart(false); std::atomic readyCount(0); - std::vector threads; - std::vector expected; + KStdVector threads; + KStdVector expected; for (int i = 0; i < kThreadCount; ++i) { expected.push_back(i); @@ -223,8 +224,8 @@ TEST(MultiSourceQueueTest, IterWhileConcurrentPublish) { constexpr int kStartCount = 50; constexpr int kThreadCount = kDefaultThreadCount; - std::vector expectedBefore; - std::vector expectedAfter; + KStdVector expectedBefore; + KStdVector expectedAfter; IntQueue::Producer producer(queue); for (int i = 0; i < kStartCount; ++i) { expectedBefore.push_back(i); @@ -236,7 +237,7 @@ TEST(MultiSourceQueueTest, IterWhileConcurrentPublish) { std::atomic canStart(false); std::atomic readyCount(0); std::atomic startedCount(0); - std::vector threads; + KStdVector threads; for (int i = 0; i < kThreadCount; ++i) { int j = i + kStartCount; expectedAfter.push_back(j); @@ -251,7 +252,7 @@ TEST(MultiSourceQueueTest, IterWhileConcurrentPublish) { }); } - std::vector actualBefore; + KStdVector actualBefore; { auto iter = queue.Iter(); while (readyCount < kThreadCount) { @@ -282,7 +283,7 @@ TEST(MultiSourceQueueTest, ConcurrentPublishAndApplyDeletions) { std::atomic canStart(false); std::atomic readyCount(0); std::atomic startedCount(0); - std::vector threads; + KStdVector threads; for (int i = 0; i < kThreadCount; ++i) { threads.emplace_back([&queue, i, &canStart, &readyCount, &startedCount]() { IntQueue::Producer producer(queue); diff --git a/kotlin-native/runtime/src/main/cpp/SingleLockList.hpp b/kotlin-native/runtime/src/main/cpp/SingleLockList.hpp index cebf5c033d2..4f6508943c6 100644 --- a/kotlin-native/runtime/src/main/cpp/SingleLockList.hpp +++ b/kotlin-native/runtime/src/main/cpp/SingleLockList.hpp @@ -10,8 +10,9 @@ #include #include -#include "CppSupport.hpp" +#include "Alloc.h" #include "Mutex.hpp" +#include "Types.h" #include "Utils.hpp" namespace kotlin { @@ -20,29 +21,43 @@ namespace kotlin { template class SingleLockList : private Pinned { public: - class Node : Pinned { + class Node; + +private: + class NodeDeleter { public: - Value* Get() noexcept { return &value; } + void operator()(Node* node) const { delete node; } + }; + + using NodeOwner = std::unique_ptr; + +public: + class Node : private Pinned, public KonanAllocatorAware { + public: + Value* Get() noexcept { return &value_; } private: friend class SingleLockList; template - Node(Args... args) noexcept : value(args...) {} + Node(Args&&... args) noexcept : value_(std::forward(args)...) {} - Value value; - std::unique_ptr next; - Node* previous = nullptr; // weak + // Make sure `Node` can only be deleted by `SingleLockList` itself. + ~Node() = default; + + Value value_; + NodeOwner next_; + Node* previous_ = nullptr; // weak }; class Iterator { public: explicit Iterator(Node* node) noexcept : node_(node) {} - Value& operator*() noexcept { return node_->value; } + Value& operator*() noexcept { return node_->value_; } Iterator& operator++() noexcept { - node_ = node_->next.get(); + node_ = node_->next_.get(); return *this; } @@ -67,36 +82,56 @@ public: std::unique_lock guard_; }; - template - Node* Emplace(Args... args) noexcept { - auto* nodePtr = new Node(args...); - std::unique_ptr node(nodePtr); - std::lock_guard guard(mutex_); - if (root_) { - root_->previous = node.get(); + ~SingleLockList() { + AssertCorrectUnsafe(); + // Make sure not to blow up the stack by nested `~Node` calls. + for (auto node = std::move(root_); node != nullptr; node = std::move(node->next_)) { } - node->next = std::move(root_); + last_ = nullptr; + AssertCorrectUnsafe(); + } + + // TODO: Consider making `Emplace` append to `last_`. + template + Node* Emplace(Args&&... args) noexcept { + auto* nodePtr = new Node(std::forward(args)...); + NodeOwner node(nodePtr); + std::lock_guard guard(mutex_); + AssertCorrectUnsafe(); + if (root_) { + root_->previous_ = node.get(); + } else { + last_ = nodePtr; + } + node->next_ = std::move(root_); root_ = std::move(node); + AssertCorrectUnsafe(); return nodePtr; } // Using `node` including its referred `Value` after `Erase` is undefined behaviour. void Erase(Node* node) noexcept { std::lock_guard guard(mutex_); + AssertCorrectUnsafe(); + if (last_ == node) { + last_ = node->previous_; + } if (root_.get() == node) { - root_ = std::move(node->next); + root_ = std::move(node->next_); if (root_) { - root_->previous = nullptr; + root_->previous_ = nullptr; } + AssertCorrectUnsafe(); return; } - auto* previous = node->previous; + auto* previous = node->previous_; RuntimeAssert(previous != nullptr, "Only the root node doesn't have the previous node"); - auto ownedNode = std::move(previous->next); - previous->next = std::move(node->next); - if (auto& next = previous->next) { - next->previous = previous; + auto ownedNode = std::move(previous->next_); + previous->next_ = std::move(node->next_); + if (auto& next = previous->next_) { + next->previous_ = previous; } + AssertCorrectUnsafe(); } // Returned value locks `this` to perform safe iteration. `this` unlocks when @@ -109,7 +144,19 @@ public: Iterable Iter() noexcept { return Iterable(this); } private: - std::unique_ptr root_; + // Expects `mutex_` to be held by the current thread. + ALWAYS_INLINE void AssertCorrectUnsafe() const noexcept { + if (root_ == nullptr) { + RuntimeAssert(last_ == nullptr, "last_ must be null"); + } else { + RuntimeAssert(root_->previous_ == nullptr, "root_ must not have previous_"); + RuntimeAssert(last_ != nullptr, "last_ must not be null"); + RuntimeAssert(last_->next_ == nullptr, "last_ must not have next_"); + } + } + + NodeOwner root_; + Node* last_ = nullptr; Mutex mutex_; }; diff --git a/kotlin-native/runtime/src/main/cpp/SingleLockListTest.cpp b/kotlin-native/runtime/src/main/cpp/SingleLockListTest.cpp index 480056ac12f..0b74e32ac90 100644 --- a/kotlin-native/runtime/src/main/cpp/SingleLockListTest.cpp +++ b/kotlin-native/runtime/src/main/cpp/SingleLockListTest.cpp @@ -6,13 +6,14 @@ #include "SingleLockList.hpp" #include -#include +#include #include #include "gmock/gmock.h" #include "gtest/gtest.h" #include "TestSupport.hpp" +#include "Types.h" using namespace kotlin; @@ -47,7 +48,7 @@ TEST(SingleLockListTest, EmplaceAndIter) { list.Emplace(kSecond); list.Emplace(kThird); - std::vector actual; + KStdVector actual; for (int element : list.Iter()) { actual.push_back(element); } @@ -65,7 +66,7 @@ TEST(SingleLockListTest, EmplaceEraseAndIter) { list.Emplace(kThird); list.Erase(secondNode); - std::vector actual; + KStdVector actual; for (int element : list.Iter()) { actual.push_back(element); } @@ -76,7 +77,7 @@ TEST(SingleLockListTest, EmplaceEraseAndIter) { TEST(SingleLockListTest, IterEmpty) { IntList list; - std::vector actual; + KStdVector actual; for (int element : list.Iter()) { actual.push_back(element); } @@ -97,7 +98,7 @@ TEST(SingleLockListTest, EraseToEmptyEmplaceAndIter) { list.Emplace(kThird); list.Emplace(kFourth); - std::vector actual; + KStdVector actual; for (int element : list.Iter()) { actual.push_back(element); } @@ -110,8 +111,8 @@ TEST(SingleLockListTest, ConcurrentEmplace) { constexpr int kThreadCount = kDefaultThreadCount; std::atomic canStart(false); std::atomic readyCount(0); - std::vector threads; - std::vector expected; + KStdVector threads; + KStdVector expected; for (int i = 0; i < kThreadCount; ++i) { expected.push_back(i); threads.emplace_back([i, &list, &canStart, &readyCount]() { @@ -129,7 +130,7 @@ TEST(SingleLockListTest, ConcurrentEmplace) { t.join(); } - std::vector actual; + KStdVector actual; for (int element : list.Iter()) { actual.push_back(element); } @@ -140,14 +141,14 @@ TEST(SingleLockListTest, ConcurrentEmplace) { TEST(SingleLockListTest, ConcurrentErase) { IntList list; constexpr int kThreadCount = kDefaultThreadCount; - std::vector items; + KStdVector items; for (int i = 0; i < kThreadCount; ++i) { items.push_back(list.Emplace(i)); } std::atomic canStart(false); std::atomic readyCount(0); - std::vector threads; + KStdVector threads; for (auto* item : items) { threads.emplace_back([item, &list, &canStart, &readyCount]() { ++readyCount; @@ -164,7 +165,7 @@ TEST(SingleLockListTest, ConcurrentErase) { t.join(); } - std::vector actual; + KStdVector actual; for (int element : list.Iter()) { actual.push_back(element); } @@ -177,8 +178,8 @@ TEST(SingleLockListTest, IterWhileConcurrentEmplace) { constexpr int kStartCount = 50; constexpr int kThreadCount = kDefaultThreadCount; - std::deque expectedBefore; - std::vector expectedAfter; + KStdDeque expectedBefore; + KStdVector expectedAfter; for (int i = 0; i < kStartCount; ++i) { expectedBefore.push_front(i); expectedAfter.push_back(i); @@ -187,7 +188,7 @@ TEST(SingleLockListTest, IterWhileConcurrentEmplace) { std::atomic canStart(false); std::atomic startedCount(0); - std::vector threads; + KStdVector threads; for (int i = 0; i < kThreadCount; ++i) { int j = i + kStartCount; expectedAfter.push_back(j); @@ -199,7 +200,7 @@ TEST(SingleLockListTest, IterWhileConcurrentEmplace) { }); } - std::vector actualBefore; + KStdVector actualBefore; { auto iter = list.Iter(); canStart = true; @@ -217,7 +218,7 @@ TEST(SingleLockListTest, IterWhileConcurrentEmplace) { EXPECT_THAT(actualBefore, testing::ElementsAreArray(expectedBefore)); - std::vector actualAfter; + KStdVector actualAfter; for (int element : list.Iter()) { actualAfter.push_back(element); } @@ -229,8 +230,8 @@ TEST(SingleLockListTest, IterWhileConcurrentErase) { IntList list; constexpr int kThreadCount = kDefaultThreadCount; - std::deque expectedBefore; - std::vector items; + KStdDeque expectedBefore; + KStdVector items; for (int i = 0; i < kThreadCount; ++i) { expectedBefore.push_front(i); items.push_back(list.Emplace(i)); @@ -238,7 +239,7 @@ TEST(SingleLockListTest, IterWhileConcurrentErase) { std::atomic canStart(false); std::atomic startedCount(0); - std::vector threads; + KStdVector threads; for (auto* item : items) { threads.emplace_back([item, &list, &canStart, &startedCount]() { while (!canStart) { @@ -248,7 +249,7 @@ TEST(SingleLockListTest, IterWhileConcurrentErase) { }); } - std::vector actualBefore; + KStdVector actualBefore; { auto iter = list.Iter(); canStart = true; @@ -266,7 +267,7 @@ TEST(SingleLockListTest, IterWhileConcurrentErase) { EXPECT_THAT(actualBefore, testing::ElementsAreArray(expectedBefore)); - std::vector actualAfter; + KStdVector actualAfter; for (int element : list.Iter()) { actualAfter.push_back(element); } @@ -298,10 +299,48 @@ TEST(SingleLockListTest, PinnedType) { list.Erase(itemNode); - std::vector actualAfter; + KStdVector actualAfter; for (auto& element : list.Iter()) { actualAfter.push_back(&element); } EXPECT_THAT(actualAfter, testing::IsEmpty()); } + +namespace { + +class WithDestructorHook; + +using DestructorHook = void(WithDestructorHook*); + +class WithDestructorHook : private Pinned { +public: + explicit WithDestructorHook(std::function hook) : hook_(std::move(hook)) {} + + ~WithDestructorHook() { hook_(this); } + +private: + std::function hook_; +}; + +} // namespace + +TEST(SingleLockListTest, Destructor) { + testing::StrictMock> hook; + { + SingleLockList list; + auto* first = list.Emplace(hook.AsStdFunction())->Get(); + auto* second = list.Emplace(hook.AsStdFunction())->Get(); + auto* third = list.Emplace(hook.AsStdFunction())->Get(); + { + testing::InSequence seq; + // `list` is `third`->`second`->`first`. If destruction + // were to cause recursion, the order of destructors + // would've been backwards. + EXPECT_CALL(hook, Call(third)); + EXPECT_CALL(hook, Call(second)); + EXPECT_CALL(hook, Call(first)); + } + } + testing::Mock::VerifyAndClear(&hook); +} diff --git a/kotlin-native/runtime/src/main/cpp/TestSupportCompilerGenerated.hpp b/kotlin-native/runtime/src/main/cpp/TestSupportCompilerGenerated.hpp index 26fccdb0838..17c295154c6 100644 --- a/kotlin-native/runtime/src/main/cpp/TestSupportCompilerGenerated.hpp +++ b/kotlin-native/runtime/src/main/cpp/TestSupportCompilerGenerated.hpp @@ -9,7 +9,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -#include "CppSupport.hpp" #include "Types.h" #include "Utils.hpp" @@ -21,7 +20,7 @@ public: explicit ScopedStrictMockFunction(Mock** globalMockLocation) : globalMockLocation_(globalMockLocation) { RuntimeCheck(globalMockLocation != nullptr, "ScopedStrictMockFunction needs non-null global mock location"); RuntimeCheck(*globalMockLocation == nullptr, "ScopedStrictMockFunction needs null global mock"); - mock_ = kotlin::std_support::make_unique(); + mock_ = make_unique(); *globalMockLocation_ = mock_.get(); } @@ -57,7 +56,7 @@ public: private: // Can be null if moved-out of. Mock** globalMockLocation_; - std::unique_ptr mock_; + KStdUniquePtr mock_; }; ScopedStrictMockFunction ScopedCreateCleanerWorkerMock(); diff --git a/kotlin-native/runtime/src/main/cpp/Types.h b/kotlin-native/runtime/src/main/cpp/Types.h index d380584d206..64300cc0af1 100644 --- a/kotlin-native/runtime/src/main/cpp/Types.h +++ b/kotlin-native/runtime/src/main/cpp/Types.h @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -60,6 +61,8 @@ typedef ObjHeader* KRef; typedef const ObjHeader* KConstRef; typedef const ArrayHeader* KString; +// TODO: Consider moving these into `kotlin::std_support` namespace keeping STL names. + // Definitions of STL classes used inside Konan runtime. typedef std::basic_string, KonanAllocator> KStdString; @@ -81,6 +84,13 @@ template using KStdVector = std::vector>; template using KStdList = std::list>; +template +using KStdUniquePtr = std::unique_ptr>; + +template +KStdUniquePtr make_unique(Args&&... args) noexcept { + return KStdUniquePtr(konanConstructInstance(std::forward(args)...)); +} #ifdef __cplusplus extern "C" { diff --git a/kotlin-native/runtime/src/main/cpp/Worker.cpp b/kotlin-native/runtime/src/main/cpp/Worker.cpp index 9c3a76f05a5..50888740b17 100644 --- a/kotlin-native/runtime/src/main/cpp/Worker.cpp +++ b/kotlin-native/runtime/src/main/cpp/Worker.cpp @@ -476,7 +476,7 @@ class State { template void waitNativeWorkersTerminationUnlocked(bool checkLeaks, F waitForWorker) { - std::vector> workersToWait; + KStdVector> workersToWait; { Locker locker(&lock_); diff --git a/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.hpp b/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.hpp index f88524af2ee..05ab8e25940 100644 --- a/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.hpp +++ b/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.hpp @@ -9,6 +9,7 @@ #include #include +#include "Alloc.h" #include "Memory.h" #include "TypeInfo.h" #include "Utils.hpp" @@ -17,7 +18,7 @@ namespace kotlin { namespace mm { // Optional data that's lazily allocated only for objects that need it. -class ExtraObjectData : private Pinned { +class ExtraObjectData : private Pinned, public KonanAllocatorAware { public: MetaObjHeader* AsMetaObjHeader() noexcept { return reinterpret_cast(this); } static ExtraObjectData& FromMetaObjHeader(MetaObjHeader* header) noexcept { return *reinterpret_cast(header); } diff --git a/kotlin-native/runtime/src/mm/cpp/GlobalsRegistry.hpp b/kotlin-native/runtime/src/mm/cpp/GlobalsRegistry.hpp index 83d3ceac83f..b4f4c254929 100644 --- a/kotlin-native/runtime/src/mm/cpp/GlobalsRegistry.hpp +++ b/kotlin-native/runtime/src/mm/cpp/GlobalsRegistry.hpp @@ -35,7 +35,7 @@ public: void ProcessThread(mm::ThreadData* threadData) noexcept; // Lock registry for safe iteration. - // TODO: Iteration over `globals_` will be slow, because it's `std::list` collected at different times from + // TODO: Iteration over `globals_` will be slow, because it's `KStdList` collected at different times from // different threads, and so the nodes are all over the memory. Use metrics to understand how // much of a problem is it. Iterable Iter() noexcept { return globals_.Iter(); } diff --git a/kotlin-native/runtime/src/mm/cpp/ObjectFactory.hpp b/kotlin-native/runtime/src/mm/cpp/ObjectFactory.hpp index cb5968e9831..99fc269c7d5 100644 --- a/kotlin-native/runtime/src/mm/cpp/ObjectFactory.hpp +++ b/kotlin-native/runtime/src/mm/cpp/ObjectFactory.hpp @@ -15,6 +15,7 @@ #include "CppSupport.hpp" #include "Memory.h" #include "Mutex.hpp" +#include "Types.h" #include "Utils.hpp" namespace kotlin { @@ -31,15 +32,14 @@ class ObjectFactoryStorage : private Pinned { static_assert(IsValidAlignment(DataAlignment), "DataAlignment is not a valid alignment"); public: - // This class does not know its size at compile-time. + // This class does not know its size at compile-time. Does not inherit from `KonanAllocatorAware` because + // in `KonanAllocatorAware::operator new(size_t size, KonanAllocTag)` `size` would be incorrect. class Node : private Pinned { constexpr static size_t DataOffset() noexcept { return AlignUp(sizeof(Node), DataAlignment); } public: ~Node() = default; - static void operator delete(void* ptr) noexcept { konanFreeMemory(ptr); } - // Note: This can only be trivially destructible data, as nobody can invoke its destructor. void* Data() noexcept { constexpr size_t kDataOffset = DataOffset(); @@ -59,7 +59,7 @@ public: Node() noexcept = default; - static void* operator new(size_t size, size_t dataSize) noexcept { + static KStdUniquePtr Create(size_t dataSize) noexcept { size_t dataSizeAligned = AlignUp(dataSize, DataAlignment); size_t totalAlignment = std::max(alignof(Node), DataAlignment); size_t totalSize = AlignUp(sizeof(Node) + dataSizeAligned, totalAlignment); @@ -73,10 +73,10 @@ public: konan::abort(); } RuntimeAssert(IsAligned(ptr, totalAlignment), "Allocator returned unaligned to %zu pointer %p", totalAlignment, ptr); - return ptr; + return KStdUniquePtr(new (ptr) Node()); } - std::unique_ptr next_; + KStdUniquePtr next_; // There's some more data of an unknown (at compile-time) size here, but it cannot be represented // with C++ members. }; @@ -89,13 +89,11 @@ public: Node& Insert(size_t dataSize) noexcept { AssertCorrect(); - auto* nodePtr = new (dataSize) Node(); - std::unique_ptr node(nodePtr); + auto node = Node::Create(dataSize); + auto* nodePtr = node.get(); if (!root_) { - RuntimeAssert(last_ == nullptr, "Unsynchronized root_ and last_"); root_ = std::move(node); } else { - RuntimeAssert(last_ != nullptr, "Unsynchronized root_ and last_"); last_->next_ = std::move(node); } @@ -155,7 +153,7 @@ public: } ObjectFactoryStorage& owner_; // weak - std::unique_ptr root_; + KStdUniquePtr root_; Node* last_ = nullptr; }; @@ -197,6 +195,11 @@ public: std::unique_lock guard_; }; + ~ObjectFactoryStorage() { + // Make sure not to blow up the stack by nested `~Node` calls. + for (auto node = std::move(root_); node != nullptr; node = std::move(node->next_)) {} + } + // Lock `ObjectFactoryStorage` for safe iteration. Iterable Iter() noexcept { return Iterable(*this); } @@ -236,7 +239,7 @@ private: } } - std::unique_ptr root_; + KStdUniquePtr root_; Node* last_ = nullptr; SpinLock mutex_; }; diff --git a/kotlin-native/runtime/src/mm/cpp/ObjectFactoryTest.cpp b/kotlin-native/runtime/src/mm/cpp/ObjectFactoryTest.cpp index f3c4a6a0e54..3eca0b91496 100644 --- a/kotlin-native/runtime/src/mm/cpp/ObjectFactoryTest.cpp +++ b/kotlin-native/runtime/src/mm/cpp/ObjectFactoryTest.cpp @@ -13,6 +13,7 @@ #include "CppSupport.hpp" #include "TestSupport.hpp" +#include "Types.h" using namespace kotlin; @@ -24,8 +25,8 @@ using ObjectFactoryStorageRegular = ObjectFactoryStorage; namespace { template -std::vector Collect(ObjectFactoryStorage& storage) { - std::vector result; +KStdVector Collect(ObjectFactoryStorage& storage) { + KStdVector result; for (auto& node : storage.Iter()) { result.push_back(node.Data()); } @@ -33,8 +34,8 @@ std::vector Collect(ObjectFactoryStorage& storage) { } template -std::vector Collect(ObjectFactoryStorage& storage) { - std::vector result; +KStdVector Collect(ObjectFactoryStorage& storage) { + KStdVector result; for (auto& node : storage.Iter()) { result.push_back(*static_cast(node.Data())); } @@ -300,8 +301,8 @@ TEST(ObjectFactoryStorageTest, ConcurrentPublish) { constexpr int kThreadCount = kDefaultThreadCount; std::atomic canStart(false); std::atomic readyCount(0); - std::vector threads; - std::vector expected; + KStdVector threads; + KStdVector expected; for (int i = 0; i < kThreadCount; ++i) { expected.push_back(i); @@ -332,8 +333,8 @@ TEST(ObjectFactoryStorageTest, IterWhileConcurrentPublish) { constexpr int kStartCount = 50; constexpr int kThreadCount = kDefaultThreadCount; - std::vector expectedBefore; - std::vector expectedAfter; + KStdVector expectedBefore; + KStdVector expectedAfter; ObjectFactoryStorageRegular::Producer producer(storage); for (int i = 0; i < kStartCount; ++i) { expectedBefore.push_back(i); @@ -345,7 +346,7 @@ TEST(ObjectFactoryStorageTest, IterWhileConcurrentPublish) { std::atomic canStart(false); std::atomic readyCount(0); std::atomic startedCount(0); - std::vector threads; + KStdVector threads; for (int i = 0; i < kThreadCount; ++i) { int j = i + kStartCount; expectedAfter.push_back(j); @@ -360,7 +361,7 @@ TEST(ObjectFactoryStorageTest, IterWhileConcurrentPublish) { }); } - std::vector actualBefore; + KStdVector actualBefore; { auto iter = storage.Iter(); while (readyCount < kThreadCount) { @@ -391,7 +392,7 @@ TEST(ObjectFactoryStorageTest, EraseWhileConcurrentPublish) { constexpr int kStartCount = 50; constexpr int kThreadCount = kDefaultThreadCount; - std::vector expectedAfter; + KStdVector expectedAfter; ObjectFactoryStorageRegular::Producer producer(storage); for (int i = 0; i < kStartCount; ++i) { if (i % 2 == 0) { @@ -404,7 +405,7 @@ TEST(ObjectFactoryStorageTest, EraseWhileConcurrentPublish) { std::atomic canStart(false); std::atomic readyCount(0); std::atomic startedCount(0); - std::vector threads; + KStdVector threads; for (int i = 0; i < kThreadCount; ++i) { int j = i + kStartCount; expectedAfter.push_back(j); @@ -449,15 +450,15 @@ using mm::ObjectFactory; namespace { -std::unique_ptr MakeObjectTypeInfo(int32_t size) { - auto typeInfo = std_support::make_unique(); +KStdUniquePtr MakeObjectTypeInfo(int32_t size) { + auto typeInfo = make_unique(); typeInfo->typeInfo_ = typeInfo.get(); typeInfo->instanceSize_ = size; return typeInfo; } -std::unique_ptr MakeArrayTypeInfo(int32_t elementSize) { - auto typeInfo = std_support::make_unique(); +KStdUniquePtr MakeArrayTypeInfo(int32_t elementSize) { + auto typeInfo = make_unique(); typeInfo->typeInfo_ = typeInfo.get(); typeInfo->instanceSize_ = -elementSize; return typeInfo; @@ -537,9 +538,9 @@ TEST(ObjectFactoryTest, ConcurrentPublish) { constexpr int kThreadCount = kDefaultThreadCount; std::atomic canStart(false); std::atomic readyCount(0); - std::vector threads; + KStdVector threads; std::mutex expectedMutex; - std::vector expected; + KStdVector expected; for (int i = 0; i < kThreadCount; ++i) { threads.emplace_back([&typeInfo, &objectFactory, &canStart, &readyCount, &expected, &expectedMutex]() { @@ -564,7 +565,7 @@ TEST(ObjectFactoryTest, ConcurrentPublish) { } auto iter = objectFactory.Iter(); - std::vector actual; + KStdVector actual; for (auto it = iter.begin(); it != iter.end(); ++it) { actual.push_back(it.GetObjHeader()); } diff --git a/kotlin-native/runtime/src/mm/cpp/StableRefRegistry.hpp b/kotlin-native/runtime/src/mm/cpp/StableRefRegistry.hpp index dc2410e4cc6..149ea10cd29 100644 --- a/kotlin-native/runtime/src/mm/cpp/StableRefRegistry.hpp +++ b/kotlin-native/runtime/src/mm/cpp/StableRefRegistry.hpp @@ -40,7 +40,7 @@ public: void ProcessDeletions() noexcept; // Lock registry for safe iteration. - // TODO: Iteration over `stableRefs_` will be slow, because it's `std::list` collected at different times from + // TODO: Iteration over `stableRefs_` will be slow, because it's `KStdList` collected at different times from // different threads, and so the nodes are all over the memory. Use metrics to understand how // much of a problem is it. Iterable Iter() noexcept { return stableRefs_.Iter(); } diff --git a/kotlin-native/runtime/src/mm/cpp/ThreadLocalStorage.hpp b/kotlin-native/runtime/src/mm/cpp/ThreadLocalStorage.hpp index 40de796846e..6bbe328270c 100644 --- a/kotlin-native/runtime/src/mm/cpp/ThreadLocalStorage.hpp +++ b/kotlin-native/runtime/src/mm/cpp/ThreadLocalStorage.hpp @@ -11,6 +11,7 @@ #include #include "Memory.h" +#include "Types.h" #include "Utils.hpp" namespace kotlin { @@ -22,7 +23,7 @@ public: class Iterator { public: - explicit Iterator(std::vector::iterator iterator) : iterator_(iterator) {} + explicit Iterator(KStdVector::iterator iterator) : iterator_(iterator) {} ObjHeader** operator*() noexcept { return &*iterator_; } @@ -35,7 +36,7 @@ public: bool operator!=(const Iterator& rhs) const noexcept { return iterator_ != rhs.iterator_; } private: - std::vector::iterator iterator_; + KStdVector::iterator iterator_; }; // Add TLS record. Can only be called before `Commit`. @@ -64,9 +65,9 @@ private: ObjHeader** Lookup(Entry entry, int index) noexcept; - std::vector storage_; - // TODO: `std::unordered_map` is probably the wrong container here. - std::unordered_map map_; + KStdVector storage_; + // TODO: `KStdUnorderedMap` is probably the wrong container here. + KStdUnorderedMap map_; State state_ = State::kBuilding; int size_ = 0; // Only used in `State::kBuilding` std::pair lastKeyAndEntry_; diff --git a/kotlin-native/runtime/src/mm/cpp/ThreadLocalStorageTest.cpp b/kotlin-native/runtime/src/mm/cpp/ThreadLocalStorageTest.cpp index f9543a0c3b7..2b5aec3bed0 100644 --- a/kotlin-native/runtime/src/mm/cpp/ThreadLocalStorageTest.cpp +++ b/kotlin-native/runtime/src/mm/cpp/ThreadLocalStorageTest.cpp @@ -8,6 +8,8 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "Types.h" + using namespace kotlin; namespace { @@ -54,12 +56,12 @@ TEST(ThreadLocalStorageTest, Iterate) { tls.AddRecord(&key2, 2); tls.Commit(); - std::vector expected; + KStdVector expected; expected.push_back(tls.Lookup(&key1, 0)); expected.push_back(tls.Lookup(&key2, 0)); expected.push_back(tls.Lookup(&key2, 1)); - std::vector actual; + KStdVector actual; for (auto item : tls) { actual.push_back(item); } @@ -78,12 +80,12 @@ TEST(ThreadLocalStorageTest, AddRecordEmpty) { tls.AddRecord(&key3, 2); tls.Commit(); - std::vector expected; + KStdVector expected; expected.push_back(tls.Lookup(&key1, 0)); expected.push_back(tls.Lookup(&key3, 0)); expected.push_back(tls.Lookup(&key3, 1)); - std::vector actual; + KStdVector actual; for (auto item : tls) { actual.push_back(item); } @@ -99,10 +101,10 @@ TEST(ThreadLocalStorageTest, AddRecordSameSize) { tls.AddRecord(&key1, 1); tls.Commit(); - std::vector expected; + KStdVector expected; expected.push_back(tls.Lookup(&key1, 0)); - std::vector actual; + KStdVector actual; for (auto item : tls) { actual.push_back(item); } @@ -115,7 +117,7 @@ TEST(ThreadLocalStorageTest, NoRecords) { tls.Commit(); - std::vector actual; + KStdVector actual; for (auto item : tls) { actual.push_back(item); } @@ -130,7 +132,7 @@ TEST(ThreadLocalStorageTest, ClearEmpty) { tls.Clear(); - std::vector actual; + KStdVector actual; for (auto item : tls) { actual.push_back(item); } @@ -147,7 +149,7 @@ TEST(ThreadLocalStorageTest, ClearNonEmpty) { tls.Clear(); - std::vector actual; + KStdVector actual; for (auto item : tls) { actual.push_back(item); }