From 11ecaa2fbf4af31738cb3cf5ea5ad835b48ff1fb Mon Sep 17 00:00:00 2001 From: Benjamin Doherty Date: Fri, 24 May 2024 13:11:10 -0700 Subject: [PATCH] Revert "Metal: implement more accurate buffer tracking (#7839)" This reverts commit 54a800a25dac26021cb001aa3740f4a0d8a0ffaa. --- filament/backend/src/metal/MetalBuffer.h | 108 +++++++++--------- filament/backend/src/metal/MetalBuffer.mm | 20 ++-- filament/backend/src/metal/MetalBufferPool.h | 2 +- filament/backend/src/metal/MetalBufferPool.mm | 3 +- filament/backend/src/metal/MetalDriver.mm | 22 ++-- filament/backend/src/metal/MetalHandles.mm | 4 +- 6 files changed, 77 insertions(+), 82 deletions(-) diff --git a/filament/backend/src/metal/MetalBuffer.h b/filament/backend/src/metal/MetalBuffer.h index c86e08a8c7..081ca6c3fd 100644 --- a/filament/backend/src/metal/MetalBuffer.h +++ b/filament/backend/src/metal/MetalBuffer.h @@ -65,12 +65,9 @@ private: const char* mName; }; -#ifndef FILAMENT_METAL_BUFFER_TRACKING -#define FILAMENT_METAL_BUFFER_TRACKING 0 -#endif - -class MetalBufferTracking { +class TrackedMetalBuffer { public: + static constexpr size_t EXCESS_BUFFER_COUNT = 30000; enum class Type { @@ -94,57 +91,66 @@ public: } } -#if FILAMENT_METAL_BUFFER_TRACKING - static void initialize() { - static dispatch_once_t onceToken; - dispatch_once(&onceToken, ^{ - for (size_t i = 0; i < TypeCount; i++) { - aliveBuffers[i] = [NSHashTable weakObjectsHashTable]; - } - }); - } - - static void setPlatform(MetalPlatform* p) { platform = p; } - - static void track(id buffer, Type type) { + TrackedMetalBuffer() noexcept : mBuffer(nil) {} + TrackedMetalBuffer(nullptr_t) noexcept : mBuffer(nil) {} + TrackedMetalBuffer(id buffer, Type type) : mBuffer(buffer), mType(type) { assert_invariant(type != Type::NONE); - if (UTILS_UNLIKELY(getAliveBuffers() >= EXCESS_BUFFER_COUNT)) { - if (platform && platform->hasDebugUpdateStatFunc()) { - platform->debugUpdateStat("filament.metal.excess_buffers_allocated", - MetalBufferTracking::getAliveBuffers()); + if (buffer) { + aliveBuffers[toIndex(type)]++; + mType = type; + if (getAliveBuffers() >= EXCESS_BUFFER_COUNT) { + if (platform && platform->hasDebugUpdateStatFunc()) { + platform->debugUpdateStat("filament.metal.excess_buffers_allocated", + TrackedMetalBuffer::getAliveBuffers()); + } } } - [aliveBuffers[toIndex(type)] addObject:buffer]; } + ~TrackedMetalBuffer() { + if (mBuffer) { + assert_invariant(mType != Type::NONE); + aliveBuffers[toIndex(mType)]--; + } + } + + TrackedMetalBuffer(TrackedMetalBuffer&&) = delete; + TrackedMetalBuffer(TrackedMetalBuffer const&) = delete; + TrackedMetalBuffer& operator=(TrackedMetalBuffer const&) = delete; + + TrackedMetalBuffer& operator=(TrackedMetalBuffer&& rhs) noexcept { + swap(rhs); + return *this; + } + + id get() const noexcept { return mBuffer; } + operator bool() const noexcept { return bool(mBuffer); } + static uint64_t getAliveBuffers() { uint64_t sum = 0; - for (size_t i = 1; i < TypeCount; i++) { - sum += getAliveBuffers(static_cast(i)); + for (const auto& v : aliveBuffers) { + sum += v; } return sum; } static uint64_t getAliveBuffers(Type type) { assert_invariant(type != Type::NONE); - NSHashTable* hashTable = aliveBuffers[toIndex(type)]; - // Caution! We can't simply use hashTable.count here, which is inaccurate. - // See http://cocoamine.net/blog/2013/12/13/nsmaptable-and-zeroing-weak-references/ - return hashTable.objectEnumerator.allObjects.count; + return aliveBuffers[toIndex(type)]; } -#else - static void initialize() {} - static void setPlatform(MetalPlatform* p) {} - static id track(id buffer, Type type) { return buffer; } - static uint64_t getAliveBuffers() { return 0; } - static uint64_t getAliveBuffers(Type type) { return 0; } -#endif + static void setPlatform(MetalPlatform* p) { platform = p; } private: -#if FILAMENT_METAL_BUFFER_TRACKING - static std::array>*, TypeCount> aliveBuffers; + void swap(TrackedMetalBuffer& other) noexcept { + std::swap(mBuffer, other.mBuffer); + std::swap(mType, other.mType); + } + + id mBuffer; + Type mType = Type::NONE; + static MetalPlatform* platform; -#endif + static std::array aliveBuffers; }; class MetalBuffer { @@ -198,7 +204,7 @@ public: private: - id mBuffer; + TrackedMetalBuffer mBuffer; size_t mBufferSize = 0; void* mCpuBuffer = nullptr; MetalContext& mContext; @@ -247,11 +253,9 @@ public: mBufferOptions(options), mSlotSizeBytes(computeSlotSize(layout)), mSlotCount(slotCount) { - { - ScopedAllocationTimer timer("ring"); - mBuffer = [device newBufferWithLength:mSlotSizeBytes * mSlotCount options:mBufferOptions]; - } - MetalBufferTracking::track(mBuffer, MetalBufferTracking::Type::RING); + ScopedAllocationTimer timer("ring"); + mBuffer = { [device newBufferWithLength:mSlotSizeBytes * mSlotCount options:mBufferOptions], + TrackedMetalBuffer::Type::RING }; assert_invariant(mBuffer); } @@ -271,11 +275,11 @@ public: // finishes executing. { ScopedAllocationTimer timer("ring"); - mAuxBuffer = [mDevice newBufferWithLength:mSlotSizeBytes options:mBufferOptions]; + mAuxBuffer = { [mDevice newBufferWithLength:mSlotSizeBytes options:mBufferOptions], + TrackedMetalBuffer::Type::RING }; } - MetalBufferTracking::track(mAuxBuffer, MetalBufferTracking::Type::RING); assert_invariant(mAuxBuffer); - return { mAuxBuffer, 0 }; + return { mAuxBuffer.get(), 0 }; } mCurrentSlot = (mCurrentSlot + 1) % mSlotCount; mOccupiedSlots->fetch_add(1, std::memory_order_relaxed); @@ -304,9 +308,9 @@ public: */ std::pair, NSUInteger> getCurrentAllocation() const { if (UTILS_UNLIKELY(mAuxBuffer)) { - return { mAuxBuffer, 0 }; + return { mAuxBuffer.get(), 0 }; } - return { mBuffer, mCurrentSlot * mSlotSizeBytes }; + return { mBuffer.get(), mCurrentSlot * mSlotSizeBytes }; } bool canAccomodateLayout(MTLSizeAndAlign layout) const { @@ -315,8 +319,8 @@ public: private: id mDevice; - id mBuffer; - id mAuxBuffer; + TrackedMetalBuffer mBuffer; + TrackedMetalBuffer mAuxBuffer; MTLResourceOptions mBufferOptions; diff --git a/filament/backend/src/metal/MetalBuffer.mm b/filament/backend/src/metal/MetalBuffer.mm index 32804f43f3..5f09a29078 100644 --- a/filament/backend/src/metal/MetalBuffer.mm +++ b/filament/backend/src/metal/MetalBuffer.mm @@ -22,14 +22,10 @@ namespace filament { namespace backend { +std::array TrackedMetalBuffer::aliveBuffers = { 0 }; +MetalPlatform* TrackedMetalBuffer::platform = nullptr; MetalPlatform* ScopedAllocationTimer::platform = nullptr; -#if FILAMENT_METAL_BUFFER_TRACKING -std::array>*, MetalBufferTracking::TypeCount> - MetalBufferTracking::aliveBuffers; -MetalPlatform* MetalBufferTracking::platform = nullptr; -#endif - MetalBuffer::MetalBuffer(MetalContext& context, BufferObjectBinding bindingType, BufferUsage usage, size_t size, bool forceGpuBuffer) : mBufferSize(size), mContext(context) { // If the buffer is less than 4K in size and is updated frequently, we don't use an explicit @@ -45,9 +41,9 @@ MetalBuffer::MetalBuffer(MetalContext& context, BufferObjectBinding bindingType, // Otherwise, we allocate a private GPU buffer. { ScopedAllocationTimer timer("generic"); - mBuffer = [context.device newBufferWithLength:size options:MTLResourceStorageModePrivate]; + mBuffer = { [context.device newBufferWithLength:size options:MTLResourceStorageModePrivate], + TrackedMetalBuffer::Type::GENERIC }; } - MetalBufferTracking::track(mBuffer, MetalBufferTracking::Type::GENERIC); ASSERT_POSTCONDITION(mBuffer, "Could not allocate Metal buffer of size %zu.", size); } @@ -74,7 +70,7 @@ void MetalBuffer::copyIntoBuffer(void* src, size_t size, size_t byteOffset) { // Acquire a staging buffer to hold the contents of this update. MetalBufferPool* bufferPool = mContext.bufferPool; const MetalBufferPoolEntry* const staging = bufferPool->acquireBuffer(size); - memcpy(staging->buffer.contents, src, size); + memcpy(staging->buffer.get().contents, src, size); // The blit below requires that byteOffset be a multiple of 4. ASSERT_PRECONDITION(!(byteOffset & 0x3u), "byteOffset must be a multiple of 4"); @@ -83,9 +79,9 @@ void MetalBuffer::copyIntoBuffer(void* src, size_t size, size_t byteOffset) { id cmdBuffer = getPendingCommandBuffer(&mContext); id blitEncoder = [cmdBuffer blitCommandEncoder]; blitEncoder.label = @"Buffer upload blit"; - [blitEncoder copyFromBuffer:staging->buffer + [blitEncoder copyFromBuffer:staging->buffer.get() sourceOffset:0 - toBuffer:mBuffer + toBuffer:mBuffer.get() destinationOffset:byteOffset size:size]; [blitEncoder endEncoding]; @@ -106,7 +102,7 @@ id MetalBuffer::getGpuBufferForDraw(id cmdBuffer) n return nil; } assert_invariant(mBuffer); - return mBuffer; + return mBuffer.get(); } void MetalBuffer::bindBuffers(id cmdBuffer, id encoder, diff --git a/filament/backend/src/metal/MetalBufferPool.h b/filament/backend/src/metal/MetalBufferPool.h index 2aa7e805a0..03688ab3c4 100644 --- a/filament/backend/src/metal/MetalBufferPool.h +++ b/filament/backend/src/metal/MetalBufferPool.h @@ -32,7 +32,7 @@ struct MetalContext; // Immutable POD representing a shared CPU-GPU buffer. struct MetalBufferPoolEntry { - id buffer; + TrackedMetalBuffer buffer; size_t capacity; mutable uint64_t lastAccessed; mutable uint32_t referenceCount; diff --git a/filament/backend/src/metal/MetalBufferPool.mm b/filament/backend/src/metal/MetalBufferPool.mm index 8c640f7d0a..a1e54a4623 100644 --- a/filament/backend/src/metal/MetalBufferPool.mm +++ b/filament/backend/src/metal/MetalBufferPool.mm @@ -48,10 +48,9 @@ MetalBufferPoolEntry const* MetalBufferPool::acquireBuffer(size_t numBytes) { buffer = [mContext.device newBufferWithLength:numBytes options:MTLResourceStorageModeShared]; } - MetalBufferTracking::track(buffer, MetalBufferTracking::Type::STAGING); ASSERT_POSTCONDITION(buffer, "Could not allocate Metal staging buffer of size %zu.", numBytes); MetalBufferPoolEntry* stage = new MetalBufferPoolEntry { - .buffer = buffer, + .buffer = { buffer, TrackedMetalBuffer::Type::STAGING }, .capacity = numBytes, .lastAccessed = mCurrentFrame, .referenceCount = 1 diff --git a/filament/backend/src/metal/MetalDriver.mm b/filament/backend/src/metal/MetalDriver.mm index 4662b8b953..a9d963e784 100644 --- a/filament/backend/src/metal/MetalDriver.mm +++ b/filament/backend/src/metal/MetalDriver.mm @@ -106,9 +106,8 @@ MetalDriver::MetalDriver(MetalPlatform* platform, const Platform::DriverConfig& mStereoscopicType(driverConfig.stereoscopicType) { mContext->driver = this; + TrackedMetalBuffer::setPlatform(platform); ScopedAllocationTimer::setPlatform(platform); - MetalBufferTracking::initialize(); - MetalBufferTracking::setPlatform(platform); mContext->device = mPlatform.createDevice(); assert_invariant(mContext->device); @@ -203,7 +202,7 @@ MetalDriver::MetalDriver(MetalPlatform* platform, const Platform::DriverConfig& } MetalDriver::~MetalDriver() noexcept { - MetalBufferTracking::setPlatform(nullptr); + TrackedMetalBuffer::setPlatform(nullptr); ScopedAllocationTimer::setPlatform(nullptr); mContext->device = nil; mContext->emptyTexture = nil; @@ -225,16 +224,13 @@ void MetalDriver::beginFrame(int64_t monotonic_clock_ns, os_signpost_interval_begin(mContext->log, mContext->signpostId, "Frame encoding", "%{public}d", frameId); #endif if (mPlatform.hasDebugUpdateStatFunc()) { -#if FILAMENT_METAL_BUFFER_TRACKING - const uint64_t generic = MetalBufferTracking::getAliveBuffers(MetalBufferTracking::Type::GENERIC); - const uint64_t ring = MetalBufferTracking::getAliveBuffers(MetalBufferTracking::Type::RING); - const uint64_t staging = MetalBufferTracking::getAliveBuffers(MetalBufferTracking::Type::STAGING); - const uint64_t total = generic + ring + staging; - mPlatform.debugUpdateStat("filament.metal.alive_buffers", total); - mPlatform.debugUpdateStat("filament.metal.alive_buffers.generic", generic); - mPlatform.debugUpdateStat("filament.metal.alive_buffers.ring", ring); - mPlatform.debugUpdateStat("filament.metal.alive_buffers.staging", staging); -#endif + mPlatform.debugUpdateStat("filament.metal.alive_buffers", TrackedMetalBuffer::getAliveBuffers()); + mPlatform.debugUpdateStat("filament.metal.alive_buffers.generic", + TrackedMetalBuffer::getAliveBuffers(TrackedMetalBuffer::Type::GENERIC)); + mPlatform.debugUpdateStat("filament.metal.alive_buffers.ring", + TrackedMetalBuffer::getAliveBuffers(TrackedMetalBuffer::Type::RING)); + mPlatform.debugUpdateStat("filament.metal.alive_buffers.staging", + TrackedMetalBuffer::getAliveBuffers(TrackedMetalBuffer::Type::STAGING)); } } diff --git a/filament/backend/src/metal/MetalHandles.mm b/filament/backend/src/metal/MetalHandles.mm index 2b770241ab..dcf22137fb 100644 --- a/filament/backend/src/metal/MetalHandles.mm +++ b/filament/backend/src/metal/MetalHandles.mm @@ -800,13 +800,13 @@ void MetalTexture::loadWithCopyBuffer(uint32_t level, uint32_t slice, MTLRegion PixelBufferDescriptor const& data, const PixelBufferShape& shape) { const size_t stagingBufferSize = shape.totalBytes; auto entry = context.bufferPool->acquireBuffer(stagingBufferSize); - memcpy(entry->buffer.contents, + memcpy(entry->buffer.get().contents, static_cast(data.buffer) + shape.sourceOffset, stagingBufferSize); id blitCommandBuffer = getPendingCommandBuffer(&context); id blitCommandEncoder = [blitCommandBuffer blitCommandEncoder]; blitCommandEncoder.label = @"Texture upload buffer blit"; - [blitCommandEncoder copyFromBuffer:entry->buffer + [blitCommandEncoder copyFromBuffer:entry->buffer.get() sourceOffset:0 sourceBytesPerRow:shape.bytesPerRow sourceBytesPerImage:shape.bytesPerSlice