From 0ecd7516de73773b140ed4e9c7dc073972360ff2 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Tue, 28 Feb 2023 13:48:17 -0500 Subject: [PATCH] Package Kit: Use an object_cache in kernel mode for decompression buffers. The kernel heap only uses object caches for objects up to size 8192. Larger allocations have to go through the raw allocator. That can get pretty expensive. Adding instrumentation around the malloc/free calls in this function showed that on my machine, some 596ms during boot were spent on *malloc/free alone*, all else aside. After this change, we are at around 110ms, or a >5x improvement. Running an fgrep -R on /system/ after boot increased the cumulative time in memory functions to over 5 seconds, while after this change it is "only" 1170ms. Honestly, it seems like the object depots should be able to be faster than that, even if this function is called thousands of times. But that is a problem for a different investigation. It would be even faster for every consumer of this data in packagefs just allocated one set of buffers up front, or at least for a single "read session", but plumbing that all the way through the myriad abstractions of the Package Kit will not be easy, and is left for another time, as well. --- .../hpkg/PackageFileHeapAccessorBase.h | 3 ++ .../packagefs/kernel_interface.cpp | 13 +++++ .../hpkg/PackageFileHeapAccessorBase.cpp | 49 +++++++++++++++++-- 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/headers/private/package/hpkg/PackageFileHeapAccessorBase.h b/headers/private/package/hpkg/PackageFileHeapAccessorBase.h index 5af32a092c..62817e2e01 100644 --- a/headers/private/package/hpkg/PackageFileHeapAccessorBase.h +++ b/headers/private/package/hpkg/PackageFileHeapAccessorBase.h @@ -98,6 +98,9 @@ public: public: static const size_t kChunkSize = 64 * 1024; +#if defined(_KERNEL_MODE) + static void* sChunkCache; +#endif protected: virtual status_t ReadAndDecompressChunk(size_t chunkIndex, diff --git a/src/add-ons/kernel/file_systems/packagefs/kernel_interface.cpp b/src/add-ons/kernel/file_systems/packagefs/kernel_interface.cpp index 9798ffb2c2..5166405005 100644 --- a/src/add-ons/kernel/file_systems/packagefs/kernel_interface.cpp +++ b/src/add-ons/kernel/file_systems/packagefs/kernel_interface.cpp @@ -14,9 +14,12 @@ #include #include #include +#include #include +#include + #include "AttributeCookie.h" #include "AttributeDirectoryCookie.h" #include "DebugSupport.h" @@ -1055,6 +1058,8 @@ packagefs_rewind_query(fs_volume* fsVolume, void* cookie) static status_t packagefs_std_ops(int32 op, ...) { + using BPackageKit::BHPKG::BPrivate::PackageFileHeapAccessorBase; + switch (op) { case B_MODULE_INIT: { @@ -1075,6 +1080,12 @@ packagefs_std_ops(int32 op, ...) return error; } + PackageFileHeapAccessorBase::sChunkCache = + create_object_cache_etc("packagefs heap buffers", + PackageFileHeapAccessorBase::kChunkSize, sizeof(void*), + 0, /* magazine capacity, count */ 2, 1, 0, NULL, + NULL, NULL, NULL); + error = PackageFSRoot::GlobalInit(); if (error != B_OK) { ERROR("Failed to init PackageFSRoot\n"); @@ -1091,6 +1102,8 @@ packagefs_std_ops(int32 op, ...) { PRINT("package_std_ops(): B_MODULE_UNINIT\n"); PackageFSRoot::GlobalUninit(); + delete_object_cache((object_cache*) + PackageFileHeapAccessorBase::sChunkCache); StringConstants::Cleanup(); StringPool::Cleanup(); exit_debugging(); diff --git a/src/kits/package/hpkg/PackageFileHeapAccessorBase.cpp b/src/kits/package/hpkg/PackageFileHeapAccessorBase.cpp index a93663690e..46c091d056 100644 --- a/src/kits/package/hpkg/PackageFileHeapAccessorBase.cpp +++ b/src/kits/package/hpkg/PackageFileHeapAccessorBase.cpp @@ -11,6 +11,9 @@ #include #include +#ifdef _KERNEL_MODE +#include +#endif #include #include @@ -27,6 +30,11 @@ namespace BHPKG { namespace BPrivate { +#if defined(_KERNEL_MODE) +void* PackageFileHeapAccessorBase::sChunkCache = NULL; +#endif + + // #pragma mark - OffsetArray @@ -209,10 +217,43 @@ PackageFileHeapAccessorBase::ReadDataToOutput(off_t offset, size_t size, } // allocate buffers for compressed and uncompressed data - uint16* compressedDataBuffer = (uint16*)malloc(kChunkSize); - uint16* uncompressedDataBuffer = (uint16*)malloc(kChunkSize); - MemoryDeleter compressedDataBufferDeleter(compressedDataBuffer); - MemoryDeleter uncompressedDataBufferDeleter(uncompressedDataBuffer); + uint16* compressedDataBuffer, *uncompressedDataBuffer; + MemoryDeleter compressedMemoryDeleter, uncompressedMemoryDeleter; + +#if defined(_KERNEL_MODE) && !defined(_BOOT_MODE) + struct ObjectCacheDeleter { + object_cache* cache; + void* object; + + ObjectCacheDeleter(object_cache* c) + : cache(c) + , object(NULL) + { + } + + ~ObjectCacheDeleter() + { + if (cache != NULL && object != NULL) + object_cache_free(cache, object, 0); + } + }; + + ObjectCacheDeleter compressedCacheDeleter((object_cache*)sChunkCache), + uncompressedCacheDeleter((object_cache*)sChunkCache); + if (sChunkCache != NULL) { + compressedDataBuffer = (uint16*)object_cache_alloc((object_cache*)sChunkCache, 0); + uncompressedDataBuffer = (uint16*)object_cache_alloc((object_cache*)sChunkCache, 0); + compressedCacheDeleter.object = compressedDataBuffer; + uncompressedCacheDeleter.object = uncompressedDataBuffer; + } else +#endif + { + compressedDataBuffer = (uint16*)malloc(kChunkSize); + uncompressedDataBuffer = (uint16*)malloc(kChunkSize); + compressedMemoryDeleter.SetTo(compressedDataBuffer); + uncompressedMemoryDeleter.SetTo(uncompressedDataBuffer); + } + if (compressedDataBuffer == NULL || uncompressedDataBuffer == NULL) return B_NO_MEMORY;