From 392b5c3b19af8efea46f5a0f7d5ee7e29277e6f4 Mon Sep 17 00:00:00 2001 From: Ali Mohammad Pur Date: Sun, 9 Jul 2023 20:05:02 +0330 Subject: [PATCH] LibJS: Resolve a circular include problem between HeapBlock and Cell Cell::heap() and Cell::vm() needed to access member functions from HeapBlock, and wanted to be inline, so they were moved to VM.h. That approach will no longer work with VM.h not being included in every file (starting from the next commit), so this commit fixes that circular import issue by introducing secondary base classes to host the references to Heap and VM, respectively. --- Userland/Libraries/LibJS/Heap/Cell.h | 5 +- Userland/Libraries/LibJS/Heap/Heap.cpp | 2 +- Userland/Libraries/LibJS/Heap/Heap.h | 7 +-- Userland/Libraries/LibJS/Heap/HeapBlock.cpp | 2 +- Userland/Libraries/LibJS/Heap/HeapBlock.h | 10 ++-- Userland/Libraries/LibJS/Heap/Internals.h | 53 +++++++++++++++++++++ Userland/Libraries/LibJS/Runtime/VM.h | 10 ---- 7 files changed, 64 insertions(+), 25 deletions(-) create mode 100644 Userland/Libraries/LibJS/Heap/Internals.h diff --git a/Userland/Libraries/LibJS/Heap/Cell.h b/Userland/Libraries/LibJS/Heap/Cell.h index 6070949786..db1a7e4252 100644 --- a/Userland/Libraries/LibJS/Heap/Cell.h +++ b/Userland/Libraries/LibJS/Heap/Cell.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -105,8 +106,8 @@ public: bool overrides_must_survive_garbage_collection(Badge) const { return m_overrides_must_survive_garbage_collection; } - Heap& heap() const; - VM& vm() const; + ALWAYS_INLINE Heap& heap() const { return HeapBlockBase::from_cell(this)->heap(); } + ALWAYS_INLINE VM& vm() const { return bit_cast(&heap())->vm(); } protected: Cell() = default; diff --git a/Userland/Libraries/LibJS/Heap/Heap.cpp b/Userland/Libraries/LibJS/Heap/Heap.cpp index 659582f766..732647843a 100644 --- a/Userland/Libraries/LibJS/Heap/Heap.cpp +++ b/Userland/Libraries/LibJS/Heap/Heap.cpp @@ -39,7 +39,7 @@ static int gc_perf_string_id; static __thread HashMap* s_custom_ranges_for_conservative_scan = nullptr; Heap::Heap(VM& vm) - : m_vm(vm) + : HeapBase(vm) { #ifdef AK_OS_SERENITY auto gc_signpost_string = "Garbage collection"sv; diff --git a/Userland/Libraries/LibJS/Heap/Heap.h b/Userland/Libraries/LibJS/Heap/Heap.h index 613a007aaa..4e7d763361 100644 --- a/Userland/Libraries/LibJS/Heap/Heap.h +++ b/Userland/Libraries/LibJS/Heap/Heap.h @@ -19,12 +19,13 @@ #include #include #include +#include #include #include namespace JS { -class Heap { +class Heap : public HeapBase { AK_MAKE_NONCOPYABLE(Heap); AK_MAKE_NONMOVABLE(Heap); @@ -57,8 +58,6 @@ public: void collect_garbage(CollectionType = CollectionType::CollectGarbage, bool print_report = false); - VM& vm() { return m_vm; } - bool should_collect_on_every_allocation() const { return m_should_collect_on_every_allocation; } void set_should_collect_on_every_allocation(bool b) { m_should_collect_on_every_allocation = b; } @@ -106,8 +105,6 @@ private: bool m_should_collect_on_every_allocation { false }; - VM& m_vm; - Vector> m_allocators; HandleImpl::List m_handles; diff --git a/Userland/Libraries/LibJS/Heap/HeapBlock.cpp b/Userland/Libraries/LibJS/Heap/HeapBlock.cpp index 6fe8c6e4e4..59a2292f07 100644 --- a/Userland/Libraries/LibJS/Heap/HeapBlock.cpp +++ b/Userland/Libraries/LibJS/Heap/HeapBlock.cpp @@ -32,7 +32,7 @@ NonnullOwnPtr HeapBlock::create_with_cell_size(Heap& heap, size_t cel } HeapBlock::HeapBlock(Heap& heap, size_t cell_size) - : m_heap(heap) + : HeapBlockBase(heap) , m_cell_size(cell_size) { VERIFY(cell_size >= sizeof(FreelistEntry)); diff --git a/Userland/Libraries/LibJS/Heap/HeapBlock.h b/Userland/Libraries/LibJS/Heap/HeapBlock.h index ffa42b174c..f852166f84 100644 --- a/Userland/Libraries/LibJS/Heap/HeapBlock.h +++ b/Userland/Libraries/LibJS/Heap/HeapBlock.h @@ -12,6 +12,7 @@ #include #include #include +#include #ifdef HAS_ADDRESS_SANITIZER # include @@ -19,12 +20,12 @@ namespace JS { -class HeapBlock { +class HeapBlock : public HeapBlockBase { AK_MAKE_NONCOPYABLE(HeapBlock); AK_MAKE_NONMOVABLE(HeapBlock); public: - static constexpr size_t block_size = 16 * KiB; + using HeapBlockBase::block_size; static NonnullOwnPtr create_with_cell_size(Heap&, size_t); size_t cell_size() const { return m_cell_size; } @@ -66,11 +67,9 @@ public: }); } - Heap& heap() { return m_heap; } - static HeapBlock* from_cell(Cell const* cell) { - return reinterpret_cast((FlatPtr)cell & ~(block_size - 1)); + return static_cast(HeapBlockBase::from_cell(cell)); } Cell* cell_from_possible_pointer(FlatPtr pointer) @@ -107,7 +106,6 @@ private: return reinterpret_cast(&m_storage[index * cell_size()]); } - Heap& m_heap; size_t m_cell_size { 0 }; size_t m_next_lazy_freelist_index { 0 }; GCPtr m_freelist; diff --git a/Userland/Libraries/LibJS/Heap/Internals.h b/Userland/Libraries/LibJS/Heap/Internals.h new file mode 100644 index 0000000000..2242dee77e --- /dev/null +++ b/Userland/Libraries/LibJS/Heap/Internals.h @@ -0,0 +1,53 @@ +/* + * Copyright (c) 2020, Andreas Kling + * Copyright (c) 2020-2023, the SerenityOS developers. + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include +#include + +namespace JS { + +class HeapBase { + AK_MAKE_NONCOPYABLE(HeapBase); + AK_MAKE_NONMOVABLE(HeapBase); + +public: + VM& vm() { return m_vm; } + +protected: + HeapBase(VM& vm) + : m_vm(vm) + { + } + + VM& m_vm; +}; + +class HeapBlockBase { + AK_MAKE_NONMOVABLE(HeapBlockBase); + AK_MAKE_NONCOPYABLE(HeapBlockBase); + +public: + static constexpr auto block_size = 16 * KiB; + static HeapBlockBase* from_cell(Cell const* cell) + { + return reinterpret_cast(bit_cast(cell) & ~(HeapBlockBase::block_size - 1)); + } + + Heap& heap() { return m_heap; } + +protected: + HeapBlockBase(Heap& heap) + : m_heap(heap) + { + } + + Heap& m_heap; +}; + +} diff --git a/Userland/Libraries/LibJS/Runtime/VM.h b/Userland/Libraries/LibJS/Runtime/VM.h index bea1c5fbf0..ec5dc552d2 100644 --- a/Userland/Libraries/LibJS/Runtime/VM.h +++ b/Userland/Libraries/LibJS/Runtime/VM.h @@ -342,14 +342,4 @@ private: OwnPtr m_bytecode_interpreter; }; -ALWAYS_INLINE Heap& Cell::heap() const -{ - return HeapBlock::from_cell(this)->heap(); -} - -ALWAYS_INLINE VM& Cell::vm() const -{ - return heap().vm(); -} - }