From e37a059230266df76555e254c532de243ee4e2a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Fri, 12 Apr 2024 17:16:10 +0200 Subject: [PATCH 1/2] perf: release asset buffers once assets are loaded --- .../src/main/cpp/AndroidManagedBuffer.h | 19 ++++---- package/cpp/FilamentBuffer.h | 9 +++- package/cpp/ManagedBuffer.h | 5 +++ package/example/ios/Podfile.lock | 44 +++++++++---------- package/ios/src/AppleManagedBuffer.h | 5 +++ package/src/hooks/useModel.ts | 1 + package/src/native/FilamentBuffer.ts | 7 ++- 7 files changed, 57 insertions(+), 33 deletions(-) diff --git a/package/android/src/main/cpp/AndroidManagedBuffer.h b/package/android/src/main/cpp/AndroidManagedBuffer.h index b11d231b..bce78321 100644 --- a/package/android/src/main/cpp/AndroidManagedBuffer.h +++ b/package/android/src/main/cpp/AndroidManagedBuffer.h @@ -14,20 +14,21 @@ using namespace facebook; class AndroidManagedBuffer : public ManagedBuffer { public: - explicit AndroidManagedBuffer(const jni::alias_ref& buffer) : _buffer(jni::make_global(buffer)) { + explicit AndroidManagedBuffer(const jni::alias_ref &buffer) + : _buffer(jni::make_global(buffer)) { _buffer->rewind(); } - ~AndroidManagedBuffer() override { - jni::ThreadScope::WithClassLoader([&] { _buffer.reset(); }); - } + ~AndroidManagedBuffer() override { releaseBuffer(); } - const uint8_t* getData() const override { - return _buffer->getDirectBytes(); - } + const uint8_t *getData() const override { return _buffer->getDirectBytes(); } - size_t getSize() const override { - return _buffer->getDirectSize(); + size_t getSize() const override { return _buffer->getDirectSize(); } + + void release() override { releaseBuffer(); } + + void releaseBuffer() { + jni::ThreadScope::WithClassLoader([&] { _buffer.reset(); }); } private: diff --git a/package/cpp/FilamentBuffer.h b/package/cpp/FilamentBuffer.h index b0732eb3..fef166cb 100644 --- a/package/cpp/FilamentBuffer.h +++ b/package/cpp/FilamentBuffer.h @@ -9,12 +9,19 @@ class FilamentBuffer : public HybridObject { public: explicit FilamentBuffer(const std::shared_ptr& buffer) : HybridObject("FilamentBuffer"), _buffer(buffer) {} - void loadHybridMethods() override {} + void loadHybridMethods() override { + registerHybridMethod("release", &FilamentBuffer::release, this); + } const std::shared_ptr& getBuffer() { return _buffer; } + void release() { + Logger::log("FilamentBuffer", "Manually releasing buffer of size %fmb", static_cast(_buffer->getSize()) / 1024 / 1024); + _buffer->release(); + } + private: std::shared_ptr _buffer; }; diff --git a/package/cpp/ManagedBuffer.h b/package/cpp/ManagedBuffer.h index dac77913..931aae46 100644 --- a/package/cpp/ManagedBuffer.h +++ b/package/cpp/ManagedBuffer.h @@ -14,6 +14,11 @@ class ManagedBuffer { virtual ~ManagedBuffer() {} virtual const uint8_t* getData() const = 0; virtual size_t getSize() const = 0; + // Note: in general we don't want to manually release anything. + // However, the reference to the Buffers is managed on the JS side and Hades seldomly + // GCs these buffers as it can't see their true size. Thus we need a mechanism to + // manually release the memory of the buffers once we have loaded an asset. + virtual void release() {} }; } // namespace margelo diff --git a/package/example/ios/Podfile.lock b/package/example/ios/Podfile.lock index f3e59c54..e580abc8 100644 --- a/package/example/ios/Podfile.lock +++ b/package/example/ios/Podfile.lock @@ -886,42 +886,42 @@ PODS: - React-Mapbuffer (0.73.4): - glog - React-debug - - react-native-filament (0.19.2): + - react-native-filament (0.22.0): - glog - RCT-Folly (= 2022.05.16.00) - React-Core - - react-native-filament/camutils (= 0.19.2) - - react-native-filament/filamat (= 0.19.2) - - react-native-filament/filament (= 0.19.2) - - react-native-filament/gltfio_core (= 0.19.2) - - react-native-filament/image (= 0.19.2) - - react-native-filament/ktxreader (= 0.19.2) - - react-native-filament/math (= 0.19.2) - - react-native-filament/tsl (= 0.19.2) - - react-native-filament/uberz (= 0.19.2) - - react-native-filament/utils (= 0.19.2) + - react-native-filament/camutils (= 0.22.0) + - react-native-filament/filamat (= 0.22.0) + - react-native-filament/filament (= 0.22.0) + - react-native-filament/gltfio_core (= 0.22.0) + - react-native-filament/image (= 0.22.0) + - react-native-filament/ktxreader (= 0.22.0) + - react-native-filament/math (= 0.22.0) + - react-native-filament/tsl (= 0.22.0) + - react-native-filament/uberz (= 0.22.0) + - react-native-filament/utils (= 0.22.0) - react-native-worklets-core - - react-native-filament/camutils (0.19.2): + - react-native-filament/camutils (0.22.0): - glog - RCT-Folly (= 2022.05.16.00) - React-Core - react-native-filament/math - react-native-worklets-core - - react-native-filament/filamat (0.19.2): + - react-native-filament/filamat (0.22.0): - glog - RCT-Folly (= 2022.05.16.00) - React-Core - react-native-filament/math - react-native-filament/utils - react-native-worklets-core - - react-native-filament/filament (0.19.2): + - react-native-filament/filament (0.22.0): - glog - RCT-Folly (= 2022.05.16.00) - React-Core - react-native-filament/math - react-native-filament/utils - react-native-worklets-core - - react-native-filament/gltfio_core (0.19.2): + - react-native-filament/gltfio_core (0.22.0): - glog - RCT-Folly (= 2022.05.16.00) - React-Core @@ -929,30 +929,30 @@ PODS: - react-native-filament/ktxreader - react-native-filament/uberz - react-native-worklets-core - - react-native-filament/image (0.19.2): + - react-native-filament/image (0.22.0): - glog - RCT-Folly (= 2022.05.16.00) - React-Core - react-native-filament/filament - react-native-worklets-core - - react-native-filament/ktxreader (0.19.2): + - react-native-filament/ktxreader (0.22.0): - glog - RCT-Folly (= 2022.05.16.00) - React-Core - react-native-filament/filament - react-native-filament/image - react-native-worklets-core - - react-native-filament/math (0.19.2): + - react-native-filament/math (0.22.0): - glog - RCT-Folly (= 2022.05.16.00) - React-Core - react-native-worklets-core - - react-native-filament/tsl (0.19.2): + - react-native-filament/tsl (0.22.0): - glog - RCT-Folly (= 2022.05.16.00) - React-Core - react-native-worklets-core - - react-native-filament/uberz (0.19.2): + - react-native-filament/uberz (0.22.0): - glog - RCT-Folly (= 2022.05.16.00) - React-Core @@ -960,7 +960,7 @@ PODS: - react-native-filament/tsl - react-native-filament/utils - react-native-worklets-core - - react-native-filament/utils (0.19.2): + - react-native-filament/utils (0.22.0): - glog - RCT-Folly (= 2022.05.16.00) - React-Core @@ -1346,7 +1346,7 @@ SPEC CHECKSUMS: React-jsinspector: 9ac353eccf6ab54d1e0a33862ba91221d1e88460 React-logger: 0a57b68dd2aec7ff738195f081f0520724b35dab React-Mapbuffer: 63913773ed7f96b814a2521e13e6d010282096ad - react-native-filament: 237b6f9f7afecfecf3fb0d3887b12db7c26a0f87 + react-native-filament: a5fa3cbbe340d29b8efca8d269b78245fbc81503 react-native-safe-area-context: b97eb6f9e3b7f437806c2ce5983f479f8eb5de4b react-native-worklets-core: 81033d42d22985176f9d1ef97378df02c092232e React-nativeconfig: d7af5bae6da70fa15ce44f045621cf99ed24087c diff --git a/package/ios/src/AppleManagedBuffer.h b/package/ios/src/AppleManagedBuffer.h index a522c3ef..ab3bdd58 100644 --- a/package/ios/src/AppleManagedBuffer.h +++ b/package/ios/src/AppleManagedBuffer.h @@ -23,6 +23,11 @@ class AppleManagedBuffer : public ManagedBuffer { size_t getSize() const override { return _data.length; } + + // Manual release mechanism + void release() override { + _data = nil; + } private: NSData* _data; diff --git a/package/src/hooks/useModel.ts b/package/src/hooks/useModel.ts index 23a1dab2..afd7a1fd 100644 --- a/package/src/hooks/useModel.ts +++ b/package/src/hooks/useModel.ts @@ -70,6 +70,7 @@ export function useModel({ path, engine, shouldReleaseSourceData, autoAddToScene loadedAsset = engine.loadInstancedAsset(assetBuffer, instanceCount) } + assetBuffer.release() return loadedAsset }, context)().then(setAsset) }, [assetBuffer, context, engine, instanceCount]) diff --git a/package/src/native/FilamentBuffer.ts b/package/src/native/FilamentBuffer.ts index 15ce2e7c..9bac148b 100644 --- a/package/src/native/FilamentBuffer.ts +++ b/package/src/native/FilamentBuffer.ts @@ -1,3 +1,8 @@ -export interface FilamentBuffer {} +export interface FilamentBuffer { + /** + * Internal method to release memory early when the buffer is no longer needed. + */ + release(): void +} export type Asset = FilamentBuffer From 500652b2b22363ef6630110f6754aa198b2dcc2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Fri, 12 Apr 2024 18:21:55 +0200 Subject: [PATCH 2/2] check-all --- .../src/main/cpp/AndroidManagedBuffer.h | 19 +++++++++++++------ package/cpp/ManagedBuffer.h | 8 ++++---- package/ios/src/AppleManagedBuffer.h | 10 +++++----- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/package/android/src/main/cpp/AndroidManagedBuffer.h b/package/android/src/main/cpp/AndroidManagedBuffer.h index bce78321..2fb80314 100644 --- a/package/android/src/main/cpp/AndroidManagedBuffer.h +++ b/package/android/src/main/cpp/AndroidManagedBuffer.h @@ -14,18 +14,25 @@ using namespace facebook; class AndroidManagedBuffer : public ManagedBuffer { public: - explicit AndroidManagedBuffer(const jni::alias_ref &buffer) - : _buffer(jni::make_global(buffer)) { + explicit AndroidManagedBuffer(const jni::alias_ref& buffer) : _buffer(jni::make_global(buffer)) { _buffer->rewind(); } - ~AndroidManagedBuffer() override { releaseBuffer(); } + ~AndroidManagedBuffer() override { + releaseBuffer(); + } - const uint8_t *getData() const override { return _buffer->getDirectBytes(); } + const uint8_t* getData() const override { + return _buffer->getDirectBytes(); + } - size_t getSize() const override { return _buffer->getDirectSize(); } + size_t getSize() const override { + return _buffer->getDirectSize(); + } - void release() override { releaseBuffer(); } + void release() override { + releaseBuffer(); + } void releaseBuffer() { jni::ThreadScope::WithClassLoader([&] { _buffer.reset(); }); diff --git a/package/cpp/ManagedBuffer.h b/package/cpp/ManagedBuffer.h index 931aae46..c243efd1 100644 --- a/package/cpp/ManagedBuffer.h +++ b/package/cpp/ManagedBuffer.h @@ -14,10 +14,10 @@ class ManagedBuffer { virtual ~ManagedBuffer() {} virtual const uint8_t* getData() const = 0; virtual size_t getSize() const = 0; - // Note: in general we don't want to manually release anything. - // However, the reference to the Buffers is managed on the JS side and Hades seldomly - // GCs these buffers as it can't see their true size. Thus we need a mechanism to - // manually release the memory of the buffers once we have loaded an asset. + // Note: in general we don't want to manually release anything. + // However, the reference to the Buffers is managed on the JS side and Hades seldomly + // GCs these buffers as it can't see their true size. Thus we need a mechanism to + // manually release the memory of the buffers once we have loaded an asset. virtual void release() {} }; diff --git a/package/ios/src/AppleManagedBuffer.h b/package/ios/src/AppleManagedBuffer.h index ab3bdd58..fd6229e5 100644 --- a/package/ios/src/AppleManagedBuffer.h +++ b/package/ios/src/AppleManagedBuffer.h @@ -23,11 +23,11 @@ class AppleManagedBuffer : public ManagedBuffer { size_t getSize() const override { return _data.length; } - - // Manual release mechanism - void release() override { - _data = nil; - } + + // Manual release mechanism + void release() override { + _data = nil; + } private: NSData* _data;