From d640ba853bb06e98cfdb4a441b400a6be96cf5eb Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Mon, 5 Feb 2024 23:32:14 -0800 Subject: [PATCH] rework how we size the HandleAllocator's pools - update the pools sizes for metal and vulkan, which were very outdated. - add debug code on all backends to print the size of each handle (with a compile time switch) The most important change is that now the 3 pools of HandleAllocator are sized so that each can accommodate about the same amount of handles. This makes it easier to reason about. The total amount of handles is three times that, since there are 3 pools. We also try to allocate the buckets so that handles are evenly distributed, however, that's very hand wavy. With the current setup the number of handles per pool is as follows: - GL : 3240 / pool / MiB - VK : 1820 / pool / MiB - MTL: 1310 / pool / MiB --- .../include/private/backend/HandleAllocator.h | 12 ++++--- filament/backend/src/HandleAllocator.cpp | 25 ++++++++----- filament/backend/src/metal/MetalDriver.mm | 34 ++++++++++++++++++ filament/backend/src/opengl/OpenGLDriver.cpp | 22 ++++++------ filament/backend/src/vulkan/VulkanDriver.cpp | 35 +++++++++++++++++++ libs/utils/include/utils/Allocator.h | 4 +++ 6 files changed, 109 insertions(+), 23 deletions(-) diff --git a/filament/backend/include/private/backend/HandleAllocator.h b/filament/backend/include/private/backend/HandleAllocator.h index 3a336e8d6e..2e7c8d1530 100644 --- a/filament/backend/include/private/backend/HandleAllocator.h +++ b/filament/backend/include/private/backend/HandleAllocator.h @@ -40,19 +40,23 @@ # define HANDLE_TYPE_SAFETY 0 #endif -#define HandleAllocatorGL HandleAllocator<16, 64, 208> -#define HandleAllocatorVK HandleAllocator<16, 64, 880> -#define HandleAllocatorMTL HandleAllocator<16, 64, 584> +#define HandleAllocatorGL HandleAllocator<16, 64, 208> // ~3640 / pool / MiB +#define HandleAllocatorVK HandleAllocator<80, 176, 320> // ~1820 / pool / MiB +#define HandleAllocatorMTL HandleAllocator<48, 160, 592> // ~1310 / pool / MiB namespace filament::backend { /* * A utility class to efficiently allocate and manage Handle<> */ -template +template class HandleAllocator { public: + static_assert(P0 % 16 == 0, "HandleAllocator Pools must be multiple of 16 bytes"); + static_assert(P1 % 16 == 0, "HandleAllocator Pools must be multiple of 16 bytes"); + static_assert(P2 % 16 == 0, "HandleAllocator Pools must be multiple of 16 bytes"); + HandleAllocator(const char* name, size_t size) noexcept; HandleAllocator(HandleAllocator const& rhs) = delete; HandleAllocator& operator=(HandleAllocator const& rhs) = delete; diff --git a/filament/backend/src/HandleAllocator.cpp b/filament/backend/src/HandleAllocator.cpp index 3257e4e2c9..d1b568af19 100644 --- a/filament/backend/src/HandleAllocator.cpp +++ b/filament/backend/src/HandleAllocator.cpp @@ -16,10 +16,17 @@ #include "private/backend/HandleAllocator.h" +#include + +#include +#include +#include #include #include +#include + namespace filament::backend { using namespace utils; @@ -28,14 +35,16 @@ template UTILS_NOINLINE HandleAllocator::Allocator::Allocator(AreaPolicy::HeapArea const& area) : mArea(area) { - // TODO: we probably need a better way to set the size of these pools - const size_t unit = area.size() / 32; - const size_t offsetPool1 = unit; - const size_t offsetPool2 = 16 * unit; - char* const p = (char*)area.begin(); - mPool0 = PoolAllocator< P0, 16>(p, p + offsetPool1); - mPool1 = PoolAllocator< P1, 16>(p + offsetPool1, p + offsetPool2); - mPool2 = PoolAllocator< P2, 16>(p + offsetPool2, area.end()); + + // size the different pools so that they can all contain the same number of handles + size_t const count = area.size() / (P0 + P1 + P2); + char* const p0 = static_cast(area.begin()); + char* const p1 = p0 + count * P0; + char* const p2 = p1 + count * P1; + + mPool0 = PoolAllocator< P0, 16>(p0, count * P0); + mPool1 = PoolAllocator< P1, 16>(p1 + count * P0, count * P1); + mPool2 = PoolAllocator< P2, 16>(p2 + count * P0, count * P2); } // ------------------------------------------------------------------------------------------------ diff --git a/filament/backend/src/metal/MetalDriver.mm b/filament/backend/src/metal/MetalDriver.mm index ef5c35e108..b1e3d7574f 100644 --- a/filament/backend/src/metal/MetalDriver.mm +++ b/filament/backend/src/metal/MetalDriver.mm @@ -43,6 +43,40 @@ namespace filament { namespace backend { Driver* MetalDriverFactory::create(MetalPlatform* const platform, const Platform::DriverConfig& driverConfig) { +#if 0 + // this is useful for development, but too verbose even for debug builds + // For reference on a 64-bits machine in Release mode: + // MetalTimerQuery : 16 few + // HwStream : 24 few + // MetalIndexBuffer : 40 moderate + // MetalFence : 48 few + // MetalBufferObject : 48 many + // -- less than or equal 48 bytes + // MetalSamplerGroup : 112 few + // MetalProgram : 144 moderate + // MetalTexture : 152 moderate + // MetalVertexBuffer : 152 moderate + // -- less than or equal 160 bytes + // MetalSwapChain : 184 few + // MetalRenderTarget : 272 few + // MetalRenderPrimitive : 584 many + // -- less than or equal to 592 bytes + + utils::slog.d + << "\nMetalSwapChain: " << sizeof(MetalSwapChain) + << "\nMetalBufferObject: " << sizeof(MetalBufferObject) + << "\nMetalVertexBuffer: " << sizeof(MetalVertexBuffer) + << "\nMetalIndexBuffer: " << sizeof(MetalIndexBuffer) + << "\nMetalSamplerGroup: " << sizeof(MetalSamplerGroup) + << "\nMetalRenderPrimitive: " << sizeof(MetalRenderPrimitive) + << "\nMetalTexture: " << sizeof(MetalTexture) + << "\nMetalTimerQuery: " << sizeof(MetalTimerQuery) + << "\nHwStream: " << sizeof(HwStream) + << "\nMetalRenderTarget: " << sizeof(MetalRenderTarget) + << "\nMetalFence: " << sizeof(MetalFence) + << "\nMetalProgram: " << sizeof(MetalProgram) + << utils::io::endl; +#endif return MetalDriver::create(platform, driverConfig); } diff --git a/filament/backend/src/opengl/OpenGLDriver.cpp b/filament/backend/src/opengl/OpenGLDriver.cpp index 1d3e06282c..047f28383d 100644 --- a/filament/backend/src/opengl/OpenGLDriver.cpp +++ b/filament/backend/src/opengl/OpenGLDriver.cpp @@ -90,24 +90,24 @@ Driver* OpenGLDriver::create(OpenGLPlatform* const platform, #if 0 // this is useful for development, but too verbose even for debug builds // For reference on a 64-bits machine in Release mode: - // GLFence : 8 few // GLIndexBuffer : 8 moderate - // GLSamplerGroup : 8 few + // GLSamplerGroup : 16 few + // GLSwapChain : 16 few + // GLTimerQuery : 16 few // -- less than or equal 16 bytes - // GLBufferObject : 24 many - // GLSync : 24 few - // GLTimerQuery : 32 few - // OpenGLProgram : 32 moderate - // GLRenderPrimitive : 48 many + // GLFence : 24 few + // GLBufferObject : 32 many + // GLRenderPrimitive : 40 many + // OpenGLProgram : 56 moderate + // GLTexture : 64 moderate // -- less than or equal 64 bytes - // GLTexture : 72 moderate + // GLStream : 104 few // GLRenderTarget : 112 few - // GLStream : 184 few // GLVertexBuffer : 200 moderate // -- less than or equal to 208 bytes slog.d - << "HwFence: " << sizeof(HwFence) + << "\nGLSwapChain: " << sizeof(GLSwapChain) << "\nGLBufferObject: " << sizeof(GLBufferObject) << "\nGLVertexBuffer: " << sizeof(GLVertexBuffer) << "\nGLIndexBuffer: " << sizeof(GLIndexBuffer) @@ -117,7 +117,7 @@ Driver* OpenGLDriver::create(OpenGLPlatform* const platform, << "\nGLTimerQuery: " << sizeof(GLTimerQuery) << "\nGLStream: " << sizeof(GLStream) << "\nGLRenderTarget: " << sizeof(GLRenderTarget) - << "\nGLSync: " << sizeof(GLSync) + << "\nGLFence: " << sizeof(GLFence) << "\nOpenGLProgram: " << sizeof(OpenGLProgram) << io::endl; #endif diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 9680320f92..3f48b44ad1 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -213,6 +213,41 @@ VulkanDriver::~VulkanDriver() noexcept = default; UTILS_NOINLINE Driver* VulkanDriver::create(VulkanPlatform* platform, VulkanContext const& context, Platform::DriverConfig const& driverConfig) noexcept { +#if 0 + // this is useful for development, but too verbose even for debug builds + // For reference on a 64-bits machine in Release mode: + // VulkanSamplerGroup : 24 few + // HwStream : 24 few + // VulkanFence : 40 few + // VulkanProgram : 40 moderate + // VulkanIndexBuffer : 72 moderate + // VulkanBufferObject : 72 many + // -- less than or equal 80 bytes + // VulkanRenderPrimitive : 104 many + // VulkanSwapChain : 112 few + // VulkanTimerQuery : 168 few + // -- less than or equal 176 bytes + // VulkanTexture : 232 moderate + // VulkanVertexBuffer : 312 moderate + // VulkanRenderTarget : 320 few + // -- less than or equal to 320 bytes + + utils::slog.d + << "\nVulkanSwapChain: " << sizeof(VulkanSwapChain) + << "\nVulkanBufferObject: " << sizeof(VulkanBufferObject) + << "\nVulkanVertexBuffer: " << sizeof(VulkanVertexBuffer) + << "\nVulkanIndexBuffer: " << sizeof(VulkanIndexBuffer) + << "\nVulkanSamplerGroup: " << sizeof(VulkanSamplerGroup) + << "\nVulkanRenderPrimitive: " << sizeof(VulkanRenderPrimitive) + << "\nVulkanTexture: " << sizeof(VulkanTexture) + << "\nVulkanTimerQuery: " << sizeof(VulkanTimerQuery) + << "\nHwStream: " << sizeof(HwStream) + << "\nVulkanRenderTarget: " << sizeof(VulkanRenderTarget) + << "\nVulkanFence: " << sizeof(VulkanFence) + << "\nVulkanProgram: " << sizeof(VulkanProgram) + << utils::io::endl; +#endif + assert_invariant(platform); size_t defaultSize = FVK_HANDLE_ARENA_SIZE_IN_MB * 1024U * 1024U; Platform::DriverConfig validConfig {driverConfig}; diff --git a/libs/utils/include/utils/Allocator.h b/libs/utils/include/utils/Allocator.h index 02479bb49a..2b02eb4cc0 100644 --- a/libs/utils/include/utils/Allocator.h +++ b/libs/utils/include/utils/Allocator.h @@ -393,6 +393,10 @@ public: : mFreeList(begin, end, ELEMENT_SIZE, ALIGNMENT, OFFSET) { } + PoolAllocator(void* begin, size_t size) noexcept + : mFreeList(begin, static_cast(begin) + size, ELEMENT_SIZE, ALIGNMENT, OFFSET) { + } + template explicit PoolAllocator(const AREA& area) noexcept : PoolAllocator(area.begin(), area.end()) {