From: Amos Jeffries Date: Wed, 13 Dec 2023 18:51:47 +0000 (+0000) Subject: Stop zeroing huge memAllocBuf() buffers (#1592) X-Git-Tag: SQUID_7_0_1~255 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=250fd42dff395fd003b122fe6d1b3ab87ff50b93;p=thirdparty%2Fsquid.git Stop zeroing huge memAllocBuf() buffers (#1592) memAllocBuf() buffers smaller than 64KB were not zeroed before this change. Larger buffers (a.k.a. "huge buffers") were zeroed with xcalloc(). We believe it is safe to stop zeroing those huge buffers because all known code that allocates huge buffers also allocates smaller ones for the same purpose. If some of that code relied on zeroing for years, we would expect to see problems with smaller buffers. Removing xcalloc() allows removal of mem*Rigid(), memBufStats() and all string pools API complications. The cache manager mgr:mem report section for string statistics is also removed. --- diff --git a/src/esi/VarState.cc b/src/esi/VarState.cc index 43786a4083..2f482723c4 100644 --- a/src/esi/VarState.cc +++ b/src/esi/VarState.cc @@ -176,8 +176,8 @@ ESIVariableQuery::ESIVariableQuery(char const *uri) : query (nullptr), query_sz ++query_pos; } - query = (_query_elem *)memReallocBuf(query, query_elements * sizeof (struct _query_elem), - &query_sz); + query = static_cast<_query_elem *>(memAllocBuf(query_elements * sizeof(struct _query_elem), &query_sz)); + memset(query, 0, query_sz); query_pos = query_start + 1; n = 0; diff --git a/src/mem/PoolingAllocator.h b/src/mem/PoolingAllocator.h index 85f17abb27..f15f1042b3 100644 --- a/src/mem/PoolingAllocator.h +++ b/src/mem/PoolingAllocator.h @@ -22,8 +22,8 @@ public: using value_type = Value; PoolingAllocator() noexcept {} template PoolingAllocator(const PoolingAllocator &) noexcept {} - value_type *allocate(std::size_t n) { return static_cast(memAllocRigid(n*sizeof(value_type))); } - void deallocate(value_type *vp, std::size_t n) noexcept { memFreeRigid(vp, n*sizeof(value_type)); } + value_type *allocate(std::size_t n) { return static_cast(memAllocBuf(n*sizeof(value_type), nullptr)); } + void deallocate(value_type *vp, std::size_t n) noexcept { memFreeBuf(n*sizeof(value_type), vp); } template struct rebind { diff --git a/src/mem/forward.h b/src/mem/forward.h index 82edae4de5..f128804cf5 100644 --- a/src/mem/forward.h +++ b/src/mem/forward.h @@ -65,15 +65,12 @@ void memConfigure(void); /// Allocate one element from the typed pool void *memAllocate(mem_type); void *memAllocBuf(size_t net_size, size_t * gross_size); -void *memAllocRigid(size_t net_size); void *memReallocBuf(void *buf, size_t net_size, size_t * gross_size); /// Free a element allocated by memAllocate() void memFree(void *, int type); void memFreeBuf(size_t size, void *); -void memFreeRigid(void *, size_t net_size); FREE *memFreeBufFunc(size_t size); int memInUse(mem_type); -size_t memStringCount(); #endif /* _SQUID_SRC_MEM_FORWARD_H */ diff --git a/src/mem/minimal.cc b/src/mem/minimal.cc index 2e27804334..7c7263ed55 100644 --- a/src/mem/minimal.cc +++ b/src/mem/minimal.cc @@ -45,8 +45,9 @@ Mem::AllocatorProxy::getStats(PoolStats &) void * memAllocBuf(const size_t netSize, size_t * const grossSize) { - *grossSize = netSize; - return xcalloc(1, netSize); + if (grossSize) + *grossSize = netSize; + return xmalloc(netSize); } void * @@ -62,18 +63,6 @@ memFree(void *memory, int) xfree(memory); } -void * -memAllocRigid(const size_t netSize) -{ - return xmalloc(netSize); -} - -void -memFreeRigid(void * const buf, size_t) -{ - xfree(buf); -} - void memFreeBuf(size_t, void * const buf) { diff --git a/src/mem/old_api.cc b/src/mem/old_api.cc index b551ad6397..461be27d02 100644 --- a/src/mem/old_api.cc +++ b/src/mem/old_api.cc @@ -40,24 +40,15 @@ static void memFree16K(void *); static void memFree32K(void *); static void memFree64K(void *); -/* local prototypes */ -static void memStringStats(std::ostream &); - /* module locals */ static double xm_time = 0; static double xm_deltat = 0; -/* string pools */ -#define mem_str_pool_count 6 - struct PoolMeta { const char *name; size_t obj_size; }; -static Mem::Meter StrCountMeter; -static Mem::Meter StrVolumeMeter; - static Mem::Meter HugeBufCountMeter; static Mem::Meter HugeBufVolumeMeter; @@ -82,96 +73,16 @@ GetPool(size_t type) return pools[type]; } -static Mem::Allocator & -GetStrPool(size_t type) -{ - static Mem::Allocator *strPools[mem_str_pool_count]; - static bool initialized = false; - - static const PoolMeta PoolAttrs[mem_str_pool_count] = { - {"Short Strings", Mem::Allocator::RoundedSize(36)}, /* to fit rfc1123 and similar */ - {"Medium Strings", Mem::Allocator::RoundedSize(128)}, /* to fit most urls */ - {"Long Strings", Mem::Allocator::RoundedSize(512)}, - {"1KB Strings", Mem::Allocator::RoundedSize(1024)}, - {"4KB Strings", Mem::Allocator::RoundedSize(4*1024)}, - {"16KB Strings", Mem::Allocator::RoundedSize(16*1024)} - }; - - if (!initialized) { - memset(strPools, '\0', sizeof(strPools)); - - /** Lastly init the string pools. */ - for (int i = 0; i < mem_str_pool_count; ++i) { - strPools[i] = memPoolCreate(PoolAttrs[i].name, PoolAttrs[i].obj_size); - strPools[i]->zeroBlocks(false); - - if (strPools[i]->objectSize != PoolAttrs[i].obj_size) - debugs(13, DBG_IMPORTANT, "WARNING: " << PoolAttrs[i].name << - " is " << strPools[i]->objectSize << - " bytes instead of requested " << - PoolAttrs[i].obj_size << " bytes"); - } - - initialized = true; - } - - return *strPools[type]; -} - -/// \returns the best-fit string pool or nil -static Mem::Allocator * -memFindStringPool(size_t net_size) -{ - for (unsigned int i = 0; i < mem_str_pool_count; ++i) { - auto &pool = GetStrPool(i); - if (net_size <= pool.objectSize) - return &pool; - } - return nullptr; -} - -static void -memStringStats(std::ostream &stream) +void +Mem::Stats(StoreEntry * sentry) { - int i; - int pooled_count = 0; - size_t pooled_volume = 0; - /* heading */ - stream << "String Pool\t Impact\t\t\n \t (%strings)\t (%volume)\n"; - /* table body */ - - for (i = 0; i < mem_str_pool_count; ++i) { - const auto &pool = GetStrPool(i); - const auto plevel = pool.meter.inuse.currentLevel(); - stream << std::setw(20) << std::left << pool.label; - stream << std::right << "\t " << xpercentInt(plevel, StrCountMeter.currentLevel()); - stream << "\t " << xpercentInt(plevel * pool.objectSize, StrVolumeMeter.currentLevel()) << "\n"; - pooled_count += plevel; - pooled_volume += plevel * pool.objectSize; - } - - /* malloc strings */ - stream << std::setw(20) << std::left << "Other Strings"; - stream << std::right << "\t "; - stream << xpercentInt(StrCountMeter.currentLevel() - pooled_count, StrCountMeter.currentLevel()) << "\t "; - stream << xpercentInt(StrVolumeMeter.currentLevel() - pooled_volume, StrVolumeMeter.currentLevel()) << "\n\n"; -} + PackableStream stream(*sentry); + Report(stream); -static void -memBufStats(std::ostream & stream) -{ stream << "Large buffers: " << HugeBufCountMeter.currentLevel() << " (" << HugeBufVolumeMeter.currentLevel() / 1024 << " KB)\n"; -} -void -Mem::Stats(StoreEntry * sentry) -{ - PackableStream stream(*sentry); - Report(stream); - memStringStats(stream); - memBufStats(stream); #if WITH_VALGRIND if (RUNNING_ON_VALGRIND) { long int leaked = 0, dubious = 0, reachable = 0, suppressed = 0; @@ -222,48 +133,6 @@ memFree(void *p, int type) GetPool(type)->freeOne(p); } -void * -memAllocRigid(size_t net_size) -{ - // TODO: Use memAllocBuf() instead (after it stops zeroing memory). - - if (const auto pool = memFindStringPool(net_size)) { - ++StrCountMeter; - StrVolumeMeter += pool->objectSize; - return pool->alloc(); - } - - ++StrCountMeter; - StrVolumeMeter += net_size; - return xmalloc(net_size); -} - -size_t -memStringCount() -{ - size_t result = 0; - - for (int counter = 0; counter < mem_str_pool_count; ++counter) - result += GetStrPool(counter).getInUseCount(); - - return result; -} - -void -memFreeRigid(void *buf, size_t net_size) -{ - if (const auto pool = memFindStringPool(net_size)) { - pool->freeOne(buf); - StrVolumeMeter -= pool->objectSize; - --StrCountMeter; - return; - } - - xfree(buf); - StrVolumeMeter -= net_size; - --StrCountMeter; -} - /* Find the best fit MEM_X_BUF type */ static mem_type memFindBufSizeType(size_t net_size, size_t * gross_size) @@ -328,8 +197,8 @@ memAllocBuf(size_t net_size, size_t * gross_size) return memAllocate(type); else { ++HugeBufCountMeter; - HugeBufVolumeMeter += *gross_size; - return xcalloc(1, net_size); + HugeBufVolumeMeter += net_size; + return xmalloc(net_size); } } diff --git a/src/tests/stub_libmem.cc b/src/tests/stub_libmem.cc index 14fd8e95c3..d539bd375b 100644 --- a/src/tests/stub_libmem.cc +++ b/src/tests/stub_libmem.cc @@ -39,16 +39,12 @@ void *memAllocate(mem_type) return xmalloc(64*1024); } -void *memAllocRigid(size_t net_size) -{ - return xmalloc(net_size); -} - void * memAllocBuf(size_t net_size, size_t * gross_size) { - *gross_size=net_size; - return xcalloc(1, net_size); + if (gross_size) + *gross_size = net_size; + return xmalloc(net_size); } /* net_size is the new size, *gross size is the old gross size, to be changed to @@ -65,7 +61,6 @@ memReallocBuf(void *oldbuf, size_t net_size, size_t * gross_size) } void memFree(void *p, int) {xfree(p);} -void memFreeRigid(void *buf, size_t) {xfree(buf);} void memFreeBuf(size_t, void *buf) {xfree(buf);} static void cxx_xfree(void * ptr) {xfree(ptr);} FREE *memFreeBufFunc(size_t) {return cxx_xfree;}