From ad3f9317072697895436af93de71eb13f08c176e Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 19 Jan 2020 13:18:27 +0100 Subject: [PATCH] Kernel: Optimize VM range deallocation a bit Previously, when deallocating a range of VM, we would sort and merge the range list. This was quite slow for large processes. This patch optimizes VM deallocation in the following ways: - Use binary search instead of linear scan to find the place to insert the deallocated range. - Insert at the right place immediately, removing the need to sort. - Merge the inserted range with any adjacent range(s) in-line instead of doing a separate merge pass into a list copy. - Add Traits to inform Vector that Range objects are trivial and can be moved using memmove(). I've also added an assertion that deallocated ranges are actually part of the RangeAllocator's initial address range. I've benchmarked this using g++ to compile Kernel/Process.cpp. With these changes, compilation goes from ~41 sec to ~35 sec. --- AK/BinarySearch.h | 11 +++++-- AK/Vector.h | 8 ++++-- Kernel/VM/RangeAllocator.cpp | 56 +++++++++++++++++++----------------- Kernel/VM/RangeAllocator.h | 9 ++++++ 4 files changed, 53 insertions(+), 31 deletions(-) diff --git a/AK/BinarySearch.h b/AK/BinarySearch.h index 9722b98792e..32e308266c2 100644 --- a/AK/BinarySearch.h +++ b/AK/BinarySearch.h @@ -26,6 +26,7 @@ #pragma once +#include #include namespace AK { @@ -37,7 +38,7 @@ int integral_compare(const T& a, const T& b) } template -T* binary_search(T* haystack, size_t haystack_size, const T& needle, Compare compare = integral_compare) +T* binary_search(T* haystack, size_t haystack_size, const T& needle, Compare compare = integral_compare, int* nearby_index = nullptr) { int low = 0; int high = haystack_size - 1; @@ -48,10 +49,16 @@ T* binary_search(T* haystack, size_t haystack_size, const T& needle, Compare com high = middle - 1; else if (comparison > 0) low = middle + 1; - else + else { + if (nearby_index) + *nearby_index = middle; return &haystack[middle]; + } } + if (nearby_index) + *nearby_index = max(0, min(low, high)); + return nullptr; } diff --git a/AK/Vector.h b/AK/Vector.h index 6cb2cac4e5c..b199dfb77b9 100644 --- a/AK/Vector.h +++ b/AK/Vector.h @@ -359,15 +359,19 @@ public: } template - void insert_before_matching(T&& value, C callback) + void insert_before_matching(T&& value, C callback, int first_index = 0, int* inserted_index = nullptr) { - for (int i = 0; i < size(); ++i) { + for (int i = first_index; i < size(); ++i) { if (callback(at(i))) { insert(i, move(value)); + if (inserted_index) + *inserted_index = i; return; } } append(move(value)); + if (inserted_index) + *inserted_index = size() - 1; } Vector& operator=(const Vector& other) diff --git a/Kernel/VM/RangeAllocator.cpp b/Kernel/VM/RangeAllocator.cpp index 8dd0d7d6d83..7adfe080c4c 100644 --- a/Kernel/VM/RangeAllocator.cpp +++ b/Kernel/VM/RangeAllocator.cpp @@ -24,10 +24,12 @@ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ +#include #include #include #include #include +#include //#define VRA_DEBUG #define VM_GUARD_PAGES @@ -38,6 +40,7 @@ RangeAllocator::RangeAllocator() void RangeAllocator::initialize_with_range(VirtualAddress base, size_t size) { + m_total_range = { base, size }; m_available_ranges.append({ base, size }); #ifdef VRA_DEBUG dump(); @@ -46,6 +49,7 @@ void RangeAllocator::initialize_with_range(VirtualAddress base, size_t size) void RangeAllocator::initialize_from_parent(const RangeAllocator& parent_allocator) { + m_total_range = parent_allocator.m_total_range; m_available_ranges = parent_allocator.m_available_ranges; } @@ -146,42 +150,40 @@ Range RangeAllocator::allocate_specific(VirtualAddress base, size_t size) void RangeAllocator::deallocate(Range range) { + ASSERT(m_total_range.contains(range)); + #ifdef VRA_DEBUG dbgprintf("VRA: Deallocate: %x(%u)\n", range.base().get(), range.size()); dump(); #endif - for (auto& available_range : m_available_ranges) { - if (available_range.end() == range.base()) { - available_range.m_size += range.size(); - goto sort_and_merge; - } - } - m_available_ranges.append(range); + ASSERT(!m_available_ranges.is_empty()); -sort_and_merge: - // FIXME: We don't have to sort if we insert at the right position immediately. - quick_sort(m_available_ranges.begin(), m_available_ranges.end(), [](auto& a, auto& b) { - return a.base() < b.base(); - }); + int nearby_index = 0; + auto* existing_range = binary_search(m_available_ranges.data(), m_available_ranges.size(), range, [](auto& a, auto& b) { + return a.base().get() - b.end().get(); + }, &nearby_index); - Vector merged_ranges; - merged_ranges.ensure_capacity(m_available_ranges.size()); - - for (auto& range : m_available_ranges) { - if (merged_ranges.is_empty()) { - merged_ranges.append(range); - continue; - } - if (range.base() == merged_ranges.last().end()) { - merged_ranges.last().m_size += range.size(); - continue; - } - merged_ranges.append(range); + int inserted_index = 0; + if (existing_range) { + existing_range->m_size += range.size(); + inserted_index = nearby_index; + } else { + m_available_ranges.insert_before_matching(Range(range), [&](auto& entry) { + return entry.base() < range.end(); + }, nearby_index, &inserted_index); } - m_available_ranges = move(merged_ranges); - + if (inserted_index < (m_available_ranges.size() - 1)) { + // We already merged with previous. Try to merge with next. + auto& inserted_range = m_available_ranges[inserted_index]; + auto& next_range = m_available_ranges[inserted_index + 1]; + if (inserted_range.end() == next_range.base()) { + inserted_range.m_size += next_range.size(); + m_available_ranges.remove(inserted_index + 1); + return; + } + } #ifdef VRA_DEBUG dbgprintf("VRA: After deallocate\n"); dump(); diff --git a/Kernel/VM/RangeAllocator.h b/Kernel/VM/RangeAllocator.h index a3f2616d3ad..bc6ce844a6d 100644 --- a/Kernel/VM/RangeAllocator.h +++ b/Kernel/VM/RangeAllocator.h @@ -27,6 +27,7 @@ #pragma once #include +#include #include #include @@ -89,9 +90,17 @@ private: void carve_at_index(int, const Range&); Vector m_available_ranges; + Range m_total_range; }; inline const LogStream& operator<<(const LogStream& stream, const Range& value) { return stream << String::format("Range(%x-%x)", value.base().get(), value.end().get() - 1); } + +namespace AK { +template<> +struct Traits : public GenericTraits { + static constexpr bool is_trivial() { return true; } +}; +}