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 43786a40837db584c2323ec94240b869e9fbf1cf..2f482723c44ac9d94234d91ede5d064a5a5bab35 100644 (file)
--- 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 85f17abb271fc820e48c6cb076b8c794ca21361d..f15f1042b38d0b419e0798e4b2d9b31050cc4f2f 100644 (file)
--- a/src/mem/PoolingAllocator.h
+++ b/src/mem/PoolingAllocator.h
@@ -22,8 +22,8 @@ public:
     using value_type = Value;
     PoolingAllocator() noexcept {}
     template <class Other> PoolingAllocator(const PoolingAllocator<Other> &) noexcept {}
-    value_type *allocate(std::size_t n) { return static_cast<value_type*>(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<value_type*>(memAllocBuf(n*sizeof(value_type), nullptr)); }
+    void deallocate(value_type *vp, std::size_t n) noexcept { memFreeBuf(n*sizeof(value_type), vp); }
 
     template <class OtherValue>
     struct rebind {

diff --git a/src/mem/forward.h b/src/mem/forward.h
index 82edae4de5eea14062e0fa4f71fe221ac2efa464..f128804cf517a19f11d808139ced83d63af301d4 100644 (file)
--- 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 2e27804334ebe269940cc54e4534dc2d734c3ce9..7c7263ed55fff778f12b4d7790a9063b44ae2b2a 100644 (file)
--- 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 b551ad6397d1b0dfc5dfac7cf440d1494672507b..461be27d02d72de2a4cfcfe1e42277d2abdfd5ad 100644 (file)
--- 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 14fd8e95c39bbe5ffc464cac8189ac86c057fe00..d539bd375b5c73f0e5b1bd72216e4b4031fd0e4d 100644 (file)
--- 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;}