diff --git a/src/MemObject.cc b/src/MemObject.cc index bb1292108f2..b07dc3672e0 100644 --- a/src/MemObject.cc +++ b/src/MemObject.cc @@ -100,6 +100,7 @@ MemObject::MemObject() ping_reply_callback = nullptr; memset(&start_ping, 0, sizeof(start_ping)); reply_ = new HttpReply; + assert(!repl.data); } MemObject::~MemObject() diff --git a/src/stmem.cc b/src/stmem.cc index 33ef855548f..193902b080c 100644 --- a/src/stmem.cc +++ b/src/stmem.cc @@ -13,8 +13,11 @@ #include "HttpReply.h" #include "mem_node.h" #include "MemObject.h" +#include "SquidMath.h" #include "stmem.h" +size_t mem_hdr::ReplPolicyIdleNodesCount = 0; + /* * NodeGet() is called to get the data buffer to pass to storeIOWrite(). * By setting the write_pending flag here we are assuming that there @@ -58,7 +61,9 @@ mem_hdr::endOffset () const void mem_hdr::freeContent() { + const auto initialNodes = size(); nodes.destroy(); + updateIdleNodes(initialNodes); inmem_hi = 0; debugs(19, 9, this << " hi: " << inmem_hi); } @@ -72,8 +77,10 @@ mem_hdr::unlink(mem_node *aNode) } debugs(19, 8, this << " removing " << aNode); + const auto initialNodes = size(); nodes.remove (aNode, NodeCompare); delete aNode; + updateIdleNodes(initialNodes); return true; } @@ -320,6 +327,7 @@ mem_hdr::write (StoreIOBuffer const &writeBuffer) char *currentSource = writeBuffer.data; size_t len = writeBuffer.length; + const auto initialNodes = size(); while (len && (target = nodeToRecieve(currentOffset))) { size_t wrote = writeAvailable(target, currentOffset, len, currentSource); assert (wrote); @@ -327,11 +335,12 @@ mem_hdr::write (StoreIOBuffer const &writeBuffer) currentOffset += wrote; currentSource += wrote; } + updateIdleNodes(initialNodes); return true; } -mem_hdr::mem_hdr() : inmem_hi(0) +mem_hdr::mem_hdr() : inmem_hi(0), removableByReplPolicy(false) { debugs(19, 9, this << " hi: " << inmem_hi); } @@ -376,3 +385,40 @@ mem_hdr::getNodes() const return nodes; } +static void +UpdateIdleNodesCounter(const size_t oldSize, const size_t newSize) +{ + if (newSize == oldSize) + return; + const auto delta = newSize > oldSize ? newSize - oldSize : oldSize - newSize; + const auto oldCount = mem_hdr::ReplPolicyIdleNodesCount; + if (newSize > oldSize) { + mem_hdr::ReplPolicyIdleNodesCount = IncreaseSum(mem_hdr::ReplPolicyIdleNodesCount, delta).value(); + } else { + assert(mem_hdr::ReplPolicyIdleNodesCount >= delta); + mem_hdr::ReplPolicyIdleNodesCount -= delta; + } + debugs(19, 5, "Updated ReplPolicyIdleNodesCount from " << oldCount << " to " << mem_hdr::ReplPolicyIdleNodesCount); +} + +void +mem_hdr::allowedToFreeWithReplPolicy(const bool allowed) +{ + if (removableByReplPolicy == allowed) + return; + removableByReplPolicy = allowed; + const auto oldSize = removableByReplPolicy ? 0 : size(); + const auto newSize = removableByReplPolicy ? size() : 0; + UpdateIdleNodesCounter(oldSize, newSize); +} + +/// Adjusts the ReplPolicyIdleNodesCount counter by the difference +/// between the current size() and oldSize. +void +mem_hdr::updateIdleNodes(const size_t oldSize) +{ + if (!removableByReplPolicy) + return; + UpdateIdleNodesCounter(oldSize, size()); +} + diff --git a/src/stmem.h b/src/stmem.h index 1ca5495c8aa..58d8bab15b8 100644 --- a/src/stmem.h +++ b/src/stmem.h @@ -33,6 +33,7 @@ class mem_hdr void dump() const; size_t size() const; mem_node *getBlockContainingLocation (int64_t location) const; + void allowedToFreeWithReplPolicy(const bool allowed); /* access the contained nodes - easier than punning * as a container ourselves */ @@ -41,6 +42,10 @@ class mem_hdr static Splay::SPLAYCMP NodeCompare; + /// the total number of pages that allowed to be purged by + /// the associated replacement policy + static size_t ReplPolicyIdleNodesCount; + private: void debugDump() const; bool unlink(mem_node *aNode); @@ -49,8 +54,10 @@ class mem_hdr bool unionNotEmpty (StoreIOBuffer const &); mem_node *nodeToRecieve(int64_t offset); size_t writeAvailable(mem_node *aNode, int64_t location, size_t amount, char const *source); + void updateIdleNodes(const size_t oldSize); int64_t inmem_hi; Splay nodes; + bool removableByReplPolicy; ///< whether nodes allowed to be purged by the associated replacement policy }; #endif /* SQUID_STMEM_H */ diff --git a/src/store.cc b/src/store.cc index 44b866582ea..e3886ab3ad5 100644 --- a/src/store.cc +++ b/src/store.cc @@ -419,6 +419,8 @@ void StoreEntry::lock(const char *context) { ++lock_count; + if (mem_obj) + mem_obj->data_hdr.allowedToFreeWithReplPolicy(false); debugs(20, 3, context << " locked key " << getMD5Text() << ' ' << *this); } @@ -450,6 +452,9 @@ StoreEntry::unlock(const char *context) if (lock_count) return (int) lock_count; + if (mem_obj) + mem_obj->data_hdr.allowedToFreeWithReplPolicy(mem_obj->repl.data); + abandon(context); return 0; } @@ -756,6 +761,7 @@ StoreEntry::write (StoreIOBuffer writeBuffer) assert(mem_obj != nullptr); /* This assert will change when we teach the store to update */ assert(store_status == STORE_PENDING); + assert(locked()); // XXX: caller uses content offset, but we also store headers writeBuffer.offset += mem_obj->baseReply().hdr_sz; @@ -1513,6 +1519,8 @@ StoreEntry::setMemStatus(mem_status_t new_status) } else { mem_policy->Add(mem_policy, this, &mem_obj->repl); debugs(20, 4, "inserted " << *this << " key: " << getMD5Text()); + // only idle entries can be freed by the replacement policy + mem_obj->data_hdr.allowedToFreeWithReplPolicy(!locked()); } ++hot_obj_count; // TODO: maintain for the shared hot cache as well @@ -1520,6 +1528,7 @@ StoreEntry::setMemStatus(mem_status_t new_status) if (EBIT_TEST(flags, ENTRY_SPECIAL)) { debugs(20, 4, "not removing special " << *this << " from policy"); } else { + mem_obj->data_hdr.allowedToFreeWithReplPolicy(false); mem_policy->Remove(mem_policy, this, &mem_obj->repl); debugs(20, 4, "removed " << *this); } diff --git a/src/store/Controller.cc b/src/store/Controller.cc index 03eaaa15b20..865c06fd943 100644 --- a/src/store/Controller.cc +++ b/src/store/Controller.cc @@ -534,6 +534,10 @@ Store::Controller::freeMemorySpace(const int bytesRequired) if (memoryCacheHasSpaceFor(pagesRequired)) return; + // do not free anything if freeing everything freeable would not be enough + if (Less(mem_hdr::ReplPolicyIdleNodesCount, pagesRequired)) + return; + // XXX: When store_pages_max is smaller than pagesRequired, we should not // look for more space (but we do because we want to abandon idle entries?). diff --git a/src/tests/testStore.cc b/src/tests/testStore.cc index 7538f0d8d28..a83696b2ae0 100644 --- a/src/tests/testStore.cc +++ b/src/tests/testStore.cc @@ -112,6 +112,7 @@ TestStore::testStats() Store::Init(aStore); CPPUNIT_ASSERT_EQUAL(false, aStore->statsCalled); StoreEntry entry; + entry.lock("TestStore::testStats"); Store::Stats(&entry); CPPUNIT_ASSERT_EQUAL(true, aStore->statsCalled); Store::FreeMemory(); diff --git a/src/tests/testStoreController.cc b/src/tests/testStoreController.cc index ff528f135f4..d2cd2294dd2 100644 --- a/src/tests/testStoreController.cc +++ b/src/tests/testStoreController.cc @@ -30,6 +30,7 @@ TestStoreController::testStats() { Store::Init(); StoreEntry *logEntry = new StoreEntry; + logEntry->lock("TestStoreController::testStats"); logEntry->createMemObject("dummy_storeId", nullptr, HttpRequestMethod()); logEntry->store_status = STORE_PENDING; TestSwapDirPointer aStore (new TestSwapDir); diff --git a/src/tests/testStoreHashIndex.cc b/src/tests/testStoreHashIndex.cc index 7ddf48f6676..90d08814d62 100644 --- a/src/tests/testStoreHashIndex.cc +++ b/src/tests/testStoreHashIndex.cc @@ -29,6 +29,7 @@ void TestStoreHashIndex::testStats() { StoreEntry *logEntry = new StoreEntry; + logEntry->lock("TestStoreHashIndex::testStats"); logEntry->createMemObject("dummy_storeId", nullptr, HttpRequestMethod()); logEntry->store_status = STORE_PENDING; Store::Init();