From 73e420d69d3e5bdd9428af0320407132c4d351c5 Mon Sep 17 00:00:00 2001 From: Troels Bjerre Lund Date: Fri, 27 Jan 2023 14:03:55 +0000 Subject: [PATCH] [K/N] custom-alloc: Fix off-by-one in threshold The existing threshold for determining between medium and large page does not take the dummy block into account, meaning that an allocation on the threshold will lead to a loop that only terminates when the process goes out of memory. Co-authored-by: Troels Lund Merge-request: KOTLIN-MR-618 Merged-by: Alexander Shabalin --- .../custom_alloc/cpp/CustomAllocConstants.hpp | 3 ++- .../src/custom_alloc/cpp/CustomAllocator.cpp | 3 ++- .../custom_alloc/cpp/CustomAllocatorTest.cpp | 27 +++++++++++++++++++ .../src/custom_alloc/cpp/LargePage.cpp | 2 +- 4 files changed, 32 insertions(+), 3 deletions(-) diff --git a/kotlin-native/runtime/src/custom_alloc/cpp/CustomAllocConstants.hpp b/kotlin-native/runtime/src/custom_alloc/cpp/CustomAllocConstants.hpp index f0a720306b6..a046f46f2b9 100644 --- a/kotlin-native/runtime/src/custom_alloc/cpp/CustomAllocConstants.hpp +++ b/kotlin-native/runtime/src/custom_alloc/cpp/CustomAllocConstants.hpp @@ -24,7 +24,8 @@ inline constexpr const size_t MEDIUM_PAGE_SIZE = (256 * KiB); inline constexpr const size_t MEDIUM_PAGE_CELL_COUNT = ((MEDIUM_PAGE_SIZE - sizeof(kotlin::alloc::MediumPage)) / sizeof(kotlin::alloc::Cell)); -inline constexpr const size_t LARGE_PAGE_SIZE_THRESHOLD = (MEDIUM_PAGE_CELL_COUNT - 1); +// MEDIUM_PAGE_CELL_COUNT minus one cell for header minus another for the 0-sized dummy block at cells_[0] +inline constexpr const size_t MEDIUM_PAGE_MAX_BLOCK_SIZE = (MEDIUM_PAGE_CELL_COUNT - 2); inline constexpr const size_t EXTRA_OBJECT_PAGE_SIZE = 64 * KiB; inline constexpr const int EXTRA_OBJECT_COUNT = diff --git a/kotlin-native/runtime/src/custom_alloc/cpp/CustomAllocator.cpp b/kotlin-native/runtime/src/custom_alloc/cpp/CustomAllocator.cpp index c18be2fb9a2..49e9a7e971f 100644 --- a/kotlin-native/runtime/src/custom_alloc/cpp/CustomAllocator.cpp +++ b/kotlin-native/runtime/src/custom_alloc/cpp/CustomAllocator.cpp @@ -12,6 +12,7 @@ #include #include "ConcurrentMarkAndSweep.hpp" +#include "CustomAllocConstants.hpp" #include "CustomLogging.hpp" #include "ExtraObjectData.hpp" #include "ExtraObjectPage.hpp" @@ -126,7 +127,7 @@ uint8_t* CustomAllocator::Allocate(uint64_t size) noexcept { uint8_t* ptr; if (cellCount <= SMALL_PAGE_MAX_BLOCK_SIZE) { ptr = AllocateInSmallPage(cellCount); - } else if (cellCount > LARGE_PAGE_SIZE_THRESHOLD) { + } else if (cellCount > MEDIUM_PAGE_MAX_BLOCK_SIZE) { ptr = AllocateInLargePage(cellCount); } else { ptr = AllocateInMediumPage(cellCount); diff --git a/kotlin-native/runtime/src/custom_alloc/cpp/CustomAllocatorTest.cpp b/kotlin-native/runtime/src/custom_alloc/cpp/CustomAllocatorTest.cpp index f93c6fca8c0..48378da66c2 100644 --- a/kotlin-native/runtime/src/custom_alloc/cpp/CustomAllocatorTest.cpp +++ b/kotlin-native/runtime/src/custom_alloc/cpp/CustomAllocatorTest.cpp @@ -6,6 +6,7 @@ #include #include +#include "CustomAllocConstants.hpp" #include "CustomAllocator.hpp" #include "GCScheduler.hpp" #include "Memory.h" @@ -55,6 +56,32 @@ TEST(CustomAllocTest, SmallAllocSameSmallPage) { } } +TEST(CustomAllocTest, SmallPageThreshold) { + Heap heap; + kotlin::gc::GCSchedulerConfig config; + kotlin::gc::GCSchedulerThreadData schedulerData(config, [](auto&) {}); + CustomAllocator ca(heap, schedulerData); + const int FROM = SMALL_PAGE_MAX_BLOCK_SIZE - 10; + const int TO = SMALL_PAGE_MAX_BLOCK_SIZE + 10; + for (int blocks = FROM; blocks <= TO; ++blocks) { + TypeInfo fakeType = {.instanceSize_ = 8 * blocks, .flags_ = 0}; + ca.CreateObject(&fakeType); + } +} + +TEST(CustomAllocTest, MediumPageThreshold) { + Heap heap; + kotlin::gc::GCSchedulerConfig config; + kotlin::gc::GCSchedulerThreadData schedulerData(config, [](auto&) {}); + CustomAllocator ca(heap, schedulerData); + const int FROM = MEDIUM_PAGE_MAX_BLOCK_SIZE - 10; + const int TO = MEDIUM_PAGE_MAX_BLOCK_SIZE + 10; + for (int blocks = FROM; blocks <= TO; ++blocks) { + TypeInfo fakeType = {.instanceSize_ = 8 * blocks, .flags_ = 0}; + ca.CreateObject(&fakeType); + } +} + TEST(CustomAllocTest, TwoAllocatorsDifferentPages) { for (int blocks = MIN_BLOCK_SIZE; blocks < 2000; ++blocks) { Heap heap; diff --git a/kotlin-native/runtime/src/custom_alloc/cpp/LargePage.cpp b/kotlin-native/runtime/src/custom_alloc/cpp/LargePage.cpp index 6d2e8f5fdff..0b2943b81ff 100644 --- a/kotlin-native/runtime/src/custom_alloc/cpp/LargePage.cpp +++ b/kotlin-native/runtime/src/custom_alloc/cpp/LargePage.cpp @@ -16,7 +16,7 @@ namespace kotlin::alloc { LargePage* LargePage::Create(uint64_t cellCount) noexcept { CustomAllocInfo("LargePage::Create(%" PRIu64 ")", cellCount); - RuntimeAssert(cellCount > LARGE_PAGE_SIZE_THRESHOLD, "blockSize too small for large page"); + RuntimeAssert(cellCount > MEDIUM_PAGE_MAX_BLOCK_SIZE, "blockSize too small for large page"); uint64_t size = sizeof(LargePage) + cellCount * sizeof(uint64_t); return new (SafeAlloc(size)) LargePage(); }