diff --git a/include/cppgc/heap.h b/include/cppgc/heap.h index 811679ed2b..d24b63e517 100644 --- a/include/cppgc/heap.h +++ b/include/cppgc/heap.h @@ -10,6 +10,7 @@ #include "cppgc/common.h" #include "cppgc/custom-space.h" +#include "cppgc/platform.h" #include "v8config.h" // NOLINT(build/include_directory) /** @@ -51,10 +52,12 @@ class V8_EXPORT Heap { /** * Creates a new heap that can be used for object allocation. * + * \param platform implemented and provided by the embedder. * \param options HeapOptions specifying various properties for the Heap. * \returns a new Heap instance. */ static std::unique_ptr Create( + std::shared_ptr platform, HeapOptions options = HeapOptions::Default()); virtual ~Heap() = default; diff --git a/include/cppgc/platform.h b/include/cppgc/platform.h index 5a5634b876..c52b73c8e7 100644 --- a/include/cppgc/platform.h +++ b/include/cppgc/platform.h @@ -10,19 +10,112 @@ namespace cppgc { -// TODO(v8:10346): Put PageAllocator and Platform in a non-V8 include header to -// avoid depending on namespace v8. +// TODO(v8:10346): Create separate includes for concepts that are not +// V8-specific. +using JobHandle = v8::JobHandle; +using JobTask = v8::JobTask; using PageAllocator = v8::PageAllocator; -using Platform = v8::Platform; +using TaskPriority = v8::TaskPriority; +using TaskRunner = v8::TaskRunner; -// Initializes the garbage collector with the provided platform. Must be called -// before creating a Heap. -V8_EXPORT void InitializePlatform(Platform* platform); +/** + * Platform interface used by Heap. Contains allocators and executors. + */ +class V8_EXPORT Platform { + public: + virtual ~Platform() = default; -V8_EXPORT Platform* GetPlatform(); + /** + * Returns the allocator used by cppgc to allocate its heap and various + * support structures. + */ + virtual PageAllocator* GetPageAllocator() = 0; -// Must be called after destroying the last used heap. -V8_EXPORT void ShutdownPlatform(); + /** + * Monotonically increasing time in seconds from an arbitrary fixed point in + * the past. This function is expected to return at least + * millisecond-precision values. For this reason, + * it is recommended that the fixed point be no further in the past than + * the epoch. + **/ + virtual double MonotonicallyIncreasingTime() = 0; + + /** + * Foreground task runner that should be used by a Heap. + */ + virtual std::shared_ptr GetForegroundTaskRunner() { + return nullptr; + } + + /** + * Posts |job_task| to run in parallel. Returns a JobHandle associated with + * the Job, which can be joined or canceled. + * This avoids degenerate cases: + * - Calling CallOnWorkerThread() for each work item, causing significant + * overhead. + * - Fixed number of CallOnWorkerThread() calls that split the work and might + * run for a long time. This is problematic when many components post + * "num cores" tasks and all expect to use all the cores. In these cases, + * the scheduler lacks context to be fair to multiple same-priority requests + * and/or ability to request lower priority work to yield when high priority + * work comes in. + * A canonical implementation of |job_task| looks like: + * class MyJobTask : public JobTask { + * public: + * MyJobTask(...) : worker_queue_(...) {} + * // JobTask: + * void Run(JobDelegate* delegate) override { + * while (!delegate->ShouldYield()) { + * // Smallest unit of work. + * auto work_item = worker_queue_.TakeWorkItem(); // Thread safe. + * if (!work_item) return; + * ProcessWork(work_item); + * } + * } + * + * size_t GetMaxConcurrency() const override { + * return worker_queue_.GetSize(); // Thread safe. + * } + * }; + * auto handle = PostJob(TaskPriority::kUserVisible, + * std::make_unique(...)); + * handle->Join(); + * + * PostJob() and methods of the returned JobHandle/JobDelegate, must never be + * called while holding a lock that could be acquired by JobTask::Run or + * JobTask::GetMaxConcurrency -- that could result in a deadlock. This is + * because [1] JobTask::GetMaxConcurrency may be invoked while holding + * internal lock (A), hence JobTask::GetMaxConcurrency can only use a lock (B) + * if that lock is *never* held while calling back into JobHandle from any + * thread (A=>B/B=>A deadlock) and [2] JobTask::Run or + * JobTask::GetMaxConcurrency may be invoked synchronously from JobHandle + * (B=>JobHandle::foo=>B deadlock). + * + * A sufficient PostJob() implementation that uses the default Job provided in + * libplatform looks like: + * std::unique_ptr PostJob( + * TaskPriority priority, std::unique_ptr job_task) override { + * return std::make_unique( + * std::make_shared( + * this, std::move(job_task), kNumThreads)); + * } + */ + virtual std::unique_ptr PostJob( + TaskPriority priority, std::unique_ptr job_task) { + return nullptr; + } +}; + +/** + * Process-global initialization of the garbage collector. Must be called before + * creating a Heap. + */ +V8_EXPORT void InitializeProcess(PageAllocator*); + +/** + * Must be called after destroying the last used heap. + */ +V8_EXPORT void ShutdownProcess(); namespace internal { diff --git a/src/heap/cppgc/heap.cc b/src/heap/cppgc/heap.cc index 692025e997..571009b448 100644 --- a/src/heap/cppgc/heap.cc +++ b/src/heap/cppgc/heap.cc @@ -31,9 +31,12 @@ void VerifyCustomSpaces( } // namespace -std::unique_ptr Heap::Create(cppgc::Heap::HeapOptions options) { +std::unique_ptr Heap::Create(std::shared_ptr platform, + cppgc::Heap::HeapOptions options) { + DCHECK(platform.get()); VerifyCustomSpaces(options.custom_spaces); - return std::make_unique(options.custom_spaces.size()); + return std::make_unique(std::move(platform), + options.custom_spaces.size()); } void Heap::ForceGarbageCollectionSlow(const char* source, const char* reason, @@ -81,11 +84,13 @@ cppgc::LivenessBroker LivenessBrokerFactory::Create() { return cppgc::LivenessBroker(); } -Heap::Heap(size_t custom_spaces) +Heap::Heap(std::shared_ptr platform, size_t custom_spaces) : raw_heap_(this, custom_spaces), - page_backend_(std::make_unique(&system_allocator_)), + platform_(std::move(platform)), + page_backend_( + std::make_unique(platform_->GetPageAllocator())), object_allocator_(&raw_heap_), - sweeper_(&raw_heap_), + sweeper_(&raw_heap_, platform_.get()), stack_(std::make_unique(v8::base::Stack::GetStackStart())), prefinalizer_handler_(std::make_unique()) {} diff --git a/src/heap/cppgc/heap.h b/src/heap/cppgc/heap.h index 879e52df55..1704bff027 100644 --- a/src/heap/cppgc/heap.h +++ b/src/heap/cppgc/heap.h @@ -80,7 +80,8 @@ class V8_EXPORT_PRIVATE Heap final : public cppgc::Heap { static Heap* From(cppgc::Heap* heap) { return static_cast(heap); } - explicit Heap(size_t custom_spaces); + explicit Heap(std::shared_ptr platform, + size_t custom_spaces); ~Heap() final; inline void* Allocate(size_t size, GCInfoIndex index); @@ -126,7 +127,7 @@ class V8_EXPORT_PRIVATE Heap final : public cppgc::Heap { RawHeap raw_heap_; - v8::base::PageAllocator system_allocator_; + std::shared_ptr platform_; std::unique_ptr page_backend_; ObjectAllocator object_allocator_; Sweeper sweeper_; diff --git a/src/heap/cppgc/platform.cc b/src/heap/cppgc/platform.cc index 2f8ed708d6..e96d69b225 100644 --- a/src/heap/cppgc/platform.cc +++ b/src/heap/cppgc/platform.cc @@ -8,20 +8,12 @@ #include "src/heap/cppgc/gc-info-table.h" namespace cppgc { -namespace internal { -static Platform* g_platform; - -} // namespace internal - -void InitializePlatform(Platform* platform) { - internal::g_platform = platform; - internal::GlobalGCInfoTable::Create(internal::g_platform->GetPageAllocator()); +void InitializeProcess(PageAllocator* page_allocator) { + internal::GlobalGCInfoTable::Create(page_allocator); } -Platform* GetPlatform() { return internal::g_platform; } - -void ShutdownPlatform() { internal::g_platform = nullptr; } +void ShutdownProcess() {} namespace internal { diff --git a/src/heap/cppgc/sweeper.cc b/src/heap/cppgc/sweeper.cc index 4c1f3249b5..2660af563a 100644 --- a/src/heap/cppgc/sweeper.cc +++ b/src/heap/cppgc/sweeper.cc @@ -225,7 +225,7 @@ typename FinalizationBuilder::ResultType SweepNormalPage(NormalPage* page) { // - merges freelists to the space's freelist. class SweepFinalizer final { public: - explicit SweepFinalizer(v8::Platform* platform) : platform_(platform) {} + explicit SweepFinalizer(cppgc::Platform* platform) : platform_(platform) {} void FinalizeHeap(SpaceStates* space_states) { for (SpaceState& space_state : *space_states) { @@ -292,14 +292,14 @@ class SweepFinalizer final { } private: - v8::Platform* platform_; + cppgc::Platform* platform_; }; class MutatorThreadSweeper final : private HeapVisitor { friend class HeapVisitor; public: - explicit MutatorThreadSweeper(SpaceStates* states, v8::Platform* platform) + explicit MutatorThreadSweeper(SpaceStates* states, cppgc::Platform* platform) : states_(states), platform_(platform) {} void Sweep() { @@ -374,7 +374,7 @@ class MutatorThreadSweeper final : private HeapVisitor { } SpaceStates* states_; - v8::Platform* platform_; + cppgc::Platform* platform_; }; class ConcurrentSweepTask final : public v8::JobTask, @@ -466,13 +466,11 @@ class PrepareForSweepVisitor final class Sweeper::SweeperImpl final { public: - explicit SweeperImpl(RawHeap* heap) + explicit SweeperImpl(RawHeap* heap, cppgc::Platform* platform) : heap_(heap), space_states_(heap->size()), - platform_(cppgc::GetPlatform()), - foreground_task_runner_(platform_->GetForegroundTaskRunner(nullptr)) { - // TODO(chromium:1056170): support Isolate independent task runner. - } + platform_(platform), + foreground_task_runner_(platform_->GetForegroundTaskRunner()) {} ~SweeperImpl() { CancelSweepers(); } @@ -600,14 +598,15 @@ class Sweeper::SweeperImpl final { RawHeap* heap_; SpaceStates space_states_; - v8::Platform* platform_; + cppgc::Platform* platform_; std::shared_ptr foreground_task_runner_; IncrementalSweepTask::Handle incremental_sweeper_handle_; std::unique_ptr concurrent_sweeper_handle_; bool is_in_progress_ = false; }; -Sweeper::Sweeper(RawHeap* heap) : impl_(std::make_unique(heap)) {} +Sweeper::Sweeper(RawHeap* heap, cppgc::Platform* platform) + : impl_(std::make_unique(heap, platform)) {} Sweeper::~Sweeper() = default; diff --git a/src/heap/cppgc/sweeper.h b/src/heap/cppgc/sweeper.h index 82fb4b051e..b19ee1eda3 100644 --- a/src/heap/cppgc/sweeper.h +++ b/src/heap/cppgc/sweeper.h @@ -10,6 +10,9 @@ #include "src/base/macros.h" namespace cppgc { + +class Platform; + namespace internal { class RawHeap; @@ -18,7 +21,7 @@ class V8_EXPORT_PRIVATE Sweeper final { public: enum class Config { kAtomic, kIncrementalAndConcurrent }; - explicit Sweeper(RawHeap*); + explicit Sweeper(RawHeap*, cppgc::Platform*); ~Sweeper(); Sweeper(const Sweeper&) = delete; diff --git a/src/heap/sweeper.h b/src/heap/sweeper.h index 13428ec31c..7cd1bafd4f 100644 --- a/src/heap/sweeper.h +++ b/src/heap/sweeper.h @@ -47,7 +47,7 @@ class Sweeper { // after exiting this scope. class FilterSweepingPagesScope final { public: - explicit FilterSweepingPagesScope( + FilterSweepingPagesScope( Sweeper* sweeper, const PauseOrCompleteScope& pause_or_complete_scope); ~FilterSweepingPagesScope(); diff --git a/test/unittests/heap/cppgc/concurrent-sweeper-unittest.cc b/test/unittests/heap/cppgc/concurrent-sweeper-unittest.cc index 77fb04842f..6a4aa20373 100644 --- a/test/unittests/heap/cppgc/concurrent-sweeper-unittest.cc +++ b/test/unittests/heap/cppgc/concurrent-sweeper-unittest.cc @@ -275,7 +275,7 @@ TEST_F(ConcurrentSweeperTest, IncrementalSweeping) { testing::TestPlatform::DisableBackgroundTasksScope disable_concurrent_sweeper( &GetPlatform()); - auto task_runner = GetPlatform().GetForegroundTaskRunner(nullptr); + auto task_runner = GetPlatform().GetForegroundTaskRunner(); // Create two unmarked objects. MakeGarbageCollected(GetHeap()); diff --git a/test/unittests/heap/cppgc/custom-spaces-unittest.cc b/test/unittests/heap/cppgc/custom-spaces-unittest.cc index 432c89efa4..63db50e4b0 100644 --- a/test/unittests/heap/cppgc/custom-spaces-unittest.cc +++ b/test/unittests/heap/cppgc/custom-spaces-unittest.cc @@ -32,7 +32,7 @@ class TestWithHeapWithCustomSpaces : public testing::TestWithPlatform { Heap::HeapOptions options; options.custom_spaces.emplace_back(std::make_unique()); options.custom_spaces.emplace_back(std::make_unique()); - heap_ = Heap::Create(std::move(options)); + heap_ = Heap::Create(platform_, std::move(options)); g_destructor_callcount = 0; } diff --git a/test/unittests/heap/cppgc/test-platform.cc b/test/unittests/heap/cppgc/test-platform.cc index 5f4f0b46cf..140e9c1589 100644 --- a/test/unittests/heap/cppgc/test-platform.cc +++ b/test/unittests/heap/cppgc/test-platform.cc @@ -84,20 +84,6 @@ TestPlatform::TestPlatform() TestPlatform::~TestPlatform() V8_NOEXCEPT { WaitAllBackgroundTasks(); } -void TestPlatform::CallOnWorkerThread(std::unique_ptr task) { - if (AreBackgroundTasksDisabled()) return; - - auto thread = std::make_unique(std::move(task)); - if (thread->Start()) { - threads_.push_back(std::move(thread)); - } -} - -void TestPlatform::CallDelayedOnWorkerThread(std::unique_ptr task, - double) { - CallOnWorkerThread(std::move(task)); -} - std::unique_ptr TestPlatform::PostJob( v8::TaskPriority, std::unique_ptr job_task) { if (AreBackgroundTasksDisabled()) return {}; @@ -112,23 +98,15 @@ double TestPlatform::MonotonicallyIncreasingTime() { static_cast(v8::base::Time::kMicrosecondsPerSecond); } -double TestPlatform::CurrentClockTimeMillis() { - return v8::base::OS::TimeCurrentMillis(); -} - void TestPlatform::WaitAllForegroundTasks() { foreground_task_runner_->RunUntilIdle(); } void TestPlatform::WaitAllBackgroundTasks() { - for (auto& thread : threads_) { - thread->Join(); - } - threads_.clear(); for (auto& thread : job_threads_) { thread->Join(); } - threads_.clear(); + job_threads_.clear(); } TestPlatform::DisableBackgroundTasksScope::DisableBackgroundTasksScope( diff --git a/test/unittests/heap/cppgc/test-platform.h b/test/unittests/heap/cppgc/test-platform.h index 2c7688fb92..474afaed0f 100644 --- a/test/unittests/heap/cppgc/test-platform.h +++ b/test/unittests/heap/cppgc/test-platform.h @@ -53,30 +53,14 @@ class TestPlatform : public Platform { PageAllocator* GetPageAllocator() override { return &page_allocator_; } - int NumberOfWorkerThreads() override { - return static_cast(threads_.size()); - } - - std::shared_ptr GetForegroundTaskRunner( - v8::Isolate*) override { + std::shared_ptr GetForegroundTaskRunner() override { return foreground_task_runner_; } - void CallOnWorkerThread(std::unique_ptr task) override; - void CallDelayedOnWorkerThread(std::unique_ptr task, - double) override; - - bool IdleTasksEnabled(v8::Isolate*) override { return true; } - std::unique_ptr PostJob( v8::TaskPriority, std::unique_ptr job_task) override; double MonotonicallyIncreasingTime() override; - double CurrentClockTimeMillis() override; - - v8::TracingController* GetTracingController() override { - return &tracing_controller_; - } void WaitAllForegroundTasks(); void WaitAllBackgroundTasks(); @@ -122,9 +106,7 @@ class TestPlatform : public Platform { v8::base::PageAllocator page_allocator_; std::shared_ptr foreground_task_runner_; - std::vector> threads_; std::vector> job_threads_; - v8::TracingController tracing_controller_; size_t disabled_background_tasks_ = 0; }; diff --git a/test/unittests/heap/cppgc/tests.cc b/test/unittests/heap/cppgc/tests.cc index f0edb1bcc9..b03ea19be6 100644 --- a/test/unittests/heap/cppgc/tests.cc +++ b/test/unittests/heap/cppgc/tests.cc @@ -13,21 +13,21 @@ namespace internal { namespace testing { // static -std::unique_ptr TestWithPlatform::platform_; +std::shared_ptr TestWithPlatform::platform_; // static void TestWithPlatform::SetUpTestSuite() { platform_ = std::make_unique(); - cppgc::InitializePlatform(platform_.get()); + cppgc::InitializeProcess(platform_->GetPageAllocator()); } // static void TestWithPlatform::TearDownTestSuite() { - cppgc::ShutdownPlatform(); + cppgc::ShutdownProcess(); platform_.reset(); } -TestWithHeap::TestWithHeap() : heap_(Heap::Create()) {} +TestWithHeap::TestWithHeap() : heap_(Heap::Create(platform_)) {} TestSupportingAllocationOnly::TestSupportingAllocationOnly() : no_gc_scope_(internal::Heap::From(GetHeap())) {} diff --git a/test/unittests/heap/cppgc/tests.h b/test/unittests/heap/cppgc/tests.h index e341944187..12946461a0 100644 --- a/test/unittests/heap/cppgc/tests.h +++ b/test/unittests/heap/cppgc/tests.h @@ -22,8 +22,8 @@ class TestWithPlatform : public ::testing::Test { TestPlatform& GetPlatform() const { return *platform_; } - private: - static std::unique_ptr platform_; + protected: + static std::shared_ptr platform_; }; class TestWithHeap : public TestWithPlatform {