Restore memory caching ability lost since r11969.
Honor maximum_object_size_in_memory for non-shared memory cache.
Since r11969, Squid calls trimMemory() for all entries to release unused
MemObjects memory. Unfortunately, that revision also started trimming objects
that could have been stored in memory cache later. Trimmed objects are no
longer eligible for memory caching.
It is possible that IN_MEMORY check in StoreEntry::trimMemory() was preventing
excessive trimming in the past, but after SMP changes, IN_MEMORY flag is set
only after the object is committed to a memory cache in
StoreController::handleIdleEntry(), so we cannot rely on it. For example:
clientReplyContext::removeStoreReference()
storeUnregister()
StoreEntry::swapOut()
StoreEntry::trimMemory()
StoreEntry::releaseRequest()
StoreEntry::unlock()
StoreController::handleIdleEntry() // never get here because entry is
set IN_MEMORY status // already marked for release
This change adds StoreController::keepForLocalMemoryCache() and
MemStore::keepInLocalMemory() methods to check if an entry could be stored in
a memory cache (and, hence, should not be trimmed). The store-related part of
the trimming logic has been moved from StoreEntry::trimMemory() to
StoreController::maybeTrimMemory(). StoreEntry code now focuses on checks that
are not specific to any memory cache settings or admission policies.
I believe this code polishing also resulted in Squid honoring
maximum_object_size_in_memory for non-shared memory cache. We may have been
ignoring that option for non-shared memory caches since SMP changes because
the check was specific to a shared memory cache.
=== modified file 'src/MemStore.cc'
--- src/MemStore.cc 2012-06-22 03:49:38 +0000
+++ src/MemStore.cc 2012-07-12 22:04:41 +0000
@@ -234,78 +234,104 @@
// local memory stores both headers and body
e.mem_obj->object_sz = sourceBuf.length; // from StoreEntry::complete()
storeGetMemSpace(sourceBuf.length); // from StoreEntry::write()
assert(e.mem_obj->data_hdr.write(sourceBuf)); // from MemObject::write()
const int64_t written = e.mem_obj->endOffset();
// we should write all because StoreEntry::write() never fails
assert(written >= 0 &&
static_cast<size_t>(written) == sourceBuf.length);
// would be nice to call validLength() here, but it needs e.key
debugs(20, 7, HERE << "mem-loaded all " << written << " bytes of " << e <<
" from " << page);
e.hideMemObject();
return true;
}
-void
-MemStore::considerKeeping(StoreEntry &e)
+bool
+MemStore::keepInLocalMemory(const StoreEntry &e) const
{
if (!e.memoryCachable()) {
debugs(20, 7, HERE << "Not memory cachable: " << e);
- return; // cannot keep due to entry state or properties
+ return false; // will not cache due to entry state or properties
+ }
+
+ assert(e.mem_obj);
+ const int64_t loadedSize = e.mem_obj->endOffset();
+ const int64_t expectedSize = e.mem_obj->expectedReplySize(); // may be < 0
+ const int64_t ramSize = max(loadedSize, expectedSize);
+
+ if (ramSize > static_cast<int64_t>(Config.Store.maxInMemObjSize)) {
+ debugs(20, 5, HERE << "Too big max(" <<
+ loadedSize << ", " << expectedSize << "): " << e);
+ return false; // will not cache due to cachable entry size limits
+ }
+
+ if (!willFit(ramSize)) {
+ debugs(20, 5, HERE << "Wont fit max(" <<
+ loadedSize << ", " << expectedSize << "): " << e);
+ return false; // will not cache due to memory cache slot limit
}
+ return true;
+}
+
+void
+MemStore::considerKeeping(StoreEntry &e)
+{
+ if (!keepInLocalMemory(e))
+ return;
+
// since we copy everything at once, we can only keep complete entries
if (e.store_status != STORE_OK) {
debugs(20, 7, HERE << "Incomplete: " << e);
return;
}
assert(e.mem_obj);
const int64_t loadedSize = e.mem_obj->endOffset();
const int64_t expectedSize = e.mem_obj->expectedReplySize();
+ // objects of unknown size are not allowed into memory cache, for now
+ if (expectedSize < 0) {
+ debugs(20, 5, HERE << "Unknown expected size: " << e);
+ return;
+ }
+
// since we copy everything at once, we can only keep fully loaded entries
if (loadedSize != expectedSize) {
debugs(20, 7, HERE << "partially loaded: " << loadedSize << " != " <<
expectedSize);
return;
}
- if (!willFit(expectedSize)) {
- debugs(20, 5, HERE << "No mem-cache space for " << e);
- return; // failed to free enough space
- }
-
keep(e); // may still fail
}
bool
-MemStore::willFit(int64_t need)
+MemStore::willFit(int64_t need) const
{
- // TODO: obey configured maximum entry size (with page-based rounding)
return need <= static_cast<int64_t>(Ipc::Mem::PageSize());
}
/// allocates map slot and calls copyToShm to store the entry in shared memory
void
MemStore::keep(StoreEntry &e)
{
if (!map) {
debugs(20, 5, HERE << "No map to mem-cache " << e);
return;
}
sfileno index = 0;
Ipc::StoreMapSlot *slot = map->openForWriting(reinterpret_cast<const cache_key *>(e.key), index);
if (!slot) {
debugs(20, 5, HERE << "No room in mem-cache map to index " << e);
return;
}
MemStoreMap::Extras &extras = map->extras(index);
=== modified file 'src/MemStore.h'
--- src/MemStore.h 2011-10-14 16:21:48 +0000
+++ src/MemStore.h 2012-07-12 19:38:33 +0000
@@ -10,61 +10,64 @@
#include "Store.h"
// StoreEntry restoration info not already stored by Ipc::StoreMap
struct MemStoreMapExtras {
Ipc::Mem::PageId page; ///< shared memory page with the entry content
int64_t storedSize; ///< total size of the stored entry content
};
typedef Ipc::StoreMapWithExtras<MemStoreMapExtras> MemStoreMap;
/// Stores HTTP entities in RAM. Current implementation uses shared memory.
/// Unlike a disk store (SwapDir), operations are synchronous (and fast).
class MemStore: public Store, public Ipc::StoreMapCleaner
{
public:
MemStore();
virtual ~MemStore();
/// cache the entry or forget about it until the next considerKeeping call
void considerKeeping(StoreEntry &e);
+ /// whether e should be kept in local RAM for possible future caching
+ bool keepInLocalMemory(const StoreEntry &e) const;
+
/* Store API */
virtual int callback();
virtual StoreEntry * get(const cache_key *);
virtual void get(String const key , STOREGETCLIENT callback, void *cbdata);
virtual void init();
virtual uint64_t maxSize() const;
virtual uint64_t minSize() const;
virtual uint64_t currentSize() const;
virtual uint64_t currentCount() const;
virtual int64_t maxObjectSize() const;
virtual void getStats(StoreInfoStats &stats) const;
virtual void stat(StoreEntry &) const;
virtual StoreSearch *search(String const url, HttpRequest *);
virtual void reference(StoreEntry &);
virtual bool dereference(StoreEntry &);
virtual void maintain();
static int64_t EntryLimit();
protected:
- bool willFit(int64_t needed);
+ bool willFit(int64_t needed) const;
void keep(StoreEntry &e);
bool copyToShm(StoreEntry &e, MemStoreMap::Extras &extras);
bool copyFromShm(StoreEntry &e, const MemStoreMap::Extras &extras);
// Ipc::StoreMapCleaner API
virtual void cleanReadable(const sfileno fileno);
private:
MemStoreMap *map; ///< index of mem-cached entries
uint64_t theCurrentSize; ///< currently used space in the storage area
};
// Why use Store as a base? MemStore and SwapDir are both "caches".
// Why not just use a SwapDir API? That would not help much because Store has
// to check/update memory cache separately from the disk cache. And same API
// would hurt because we can support synchronous get/put, unlike the disks.
#endif /* SQUID_MEMSTORE_H */
=== modified file 'src/Store.h'
--- src/Store.h 2012-06-18 23:08:56 +0000
+++ src/Store.h 2012-07-12 18:43:39 +0000
@@ -334,40 +334,45 @@
/** remove a Store entry from the store */
virtual void unlink (StoreEntry &);
/* search in the store */
virtual StoreSearch *search(String const url, HttpRequest *) = 0;
/* pulled up from SwapDir for migration.... probably do not belong here */
virtual void reference(StoreEntry &) = 0; /* Reference this object */
/// Undo reference(), returning false iff idle e should be destroyed
virtual bool dereference(StoreEntry &e) = 0;
virtual void maintain() = 0; /* perform regular maintenance should be private and self registered ... */
// XXX: This method belongs to Store::Root/StoreController, but it is here
// because test cases use non-StoreController derivatives as Root
/// called when the entry is no longer needed by any transaction
virtual void handleIdleEntry(StoreEntry &e) {}
+ // XXX: This method belongs to Store::Root/StoreController, but it is here
+ // because test cases use non-StoreController derivatives as Root
+ /// called to get rid of no longer needed entry data in RAM, if any
+ virtual void maybeTrimMemory(StoreEntry &e, const bool preserveSwappable) {}
+
private:
static RefCount<Store> CurrentRoot;
};
/// \ingroup StoreAPI
typedef RefCount<Store> StorePointer;
/// \ingroup StoreAPI
SQUIDCEXTERN size_t storeEntryInUse();
/// \ingroup StoreAPI
SQUIDCEXTERN const char *storeEntryFlags(const StoreEntry *);
/// \ingroup StoreAPI
extern void storeEntryReplaceObject(StoreEntry *, HttpReply *);
/// \ingroup StoreAPI
SQUIDCEXTERN StoreEntry *storeGetPublic(const char *uri, const HttpRequestMethod& method);
/// \ingroup StoreAPI
=== modified file 'src/SwapDir.h'
--- src/SwapDir.h 2011-10-27 23:14:28 +0000
+++ src/SwapDir.h 2012-07-12 21:18:17 +0000
@@ -43,71 +43,73 @@
/* SwapDir *sd, char *path ( + char *opt later when the strtok mess is gone) */
class ConfigOption;
/// hides memory/disk cache distinction from callers
class StoreController : public Store
{
public:
StoreController();
virtual ~StoreController();
virtual int callback();
virtual void create();
virtual StoreEntry * get(const cache_key *);
virtual void get(String const, STOREGETCLIENT, void * cbdata);
/* Store parent API */
virtual void handleIdleEntry(StoreEntry &e);
+ virtual void maybeTrimMemory(StoreEntry &e, const bool preserveSwappable);
virtual void init();
virtual void maintain(); /* perform regular maintenance should be private and self registered ... */
virtual uint64_t maxSize() const;
virtual uint64_t minSize() const;
virtual uint64_t currentSize() const;
virtual uint64_t currentCount() const;
virtual int64_t maxObjectSize() const;
virtual void getStats(StoreInfoStats &stats) const;
virtual void stat(StoreEntry &) const;
virtual void sync(); /* Sync the store prior to shutdown */
virtual StoreSearch *search(String const url, HttpRequest *);
virtual void reference(StoreEntry &); /* Reference this object */
virtual bool dereference(StoreEntry &); /* Unreference this object */
/* the number of store dirs being rebuilt. */
static int store_dirs_rebuilding;
private:
void createOneStore(Store &aStore);
+ bool keepForLocalMemoryCache(const StoreEntry &e) const;
StorePointer swapDir; ///< summary view of all disk caches
MemStore *memStore; ///< memory cache
};
/* migrating from the Config based list of swapdirs */
extern void allocate_new_swapdir(SquidConfig::_cacheSwap *);
extern void free_cachedir(SquidConfig::_cacheSwap * swap);
SQUIDCEXTERN OBJH storeDirStats;
SQUIDCEXTERN char *storeDirSwapLogFile(int, const char *);
SQUIDCEXTERN char *storeSwapFullPath(int, char *);
SQUIDCEXTERN char *storeSwapSubSubDir(int, char *);
SQUIDCEXTERN const char *storeSwapPath(int);
SQUIDCEXTERN int storeDirWriteCleanLogs(int reopen);
SQUIDCEXTERN STDIRSELECT *storeDirSelectSwapDir;
SQUIDCEXTERN int storeVerifySwapDirs(void);
SQUIDCEXTERN void storeDirCloseSwapLogs(void);
SQUIDCEXTERN void storeDirCloseTmpSwapLog(int dirn);
SQUIDCEXTERN void storeDirDiskFull(sdirno);
SQUIDCEXTERN void storeDirOpenSwapLogs(void);
=== modified file 'src/store.cc'
--- src/store.cc 2012-06-18 23:08:56 +0000
+++ src/store.cc 2012-07-12 20:50:20 +0000
@@ -1429,48 +1429,40 @@
store_swap_low = (long) (((float) Store::Root().maxSize() *
(float) Config.Swap.lowWaterMark) / (float) 100);
store_pages_max = Config.memMaxSize / sizeof(mem_node);
}
bool
StoreEntry::memoryCachable() const
{
if (mem_obj == NULL)
return 0;
if (mem_obj->data_hdr.size() == 0)
return 0;
if (mem_obj->inmem_lo != 0)
return 0;
if (!Config.onoff.memory_cache_first && swap_status == SWAPOUT_DONE && refcount == 1)
return 0;
- if (Config.memShared && IamWorkerProcess()) {
- const int64_t expectedSize = mem_obj->expectedReplySize();
- // objects of unknown size are not allowed into memory cache, for now
- if (expectedSize < 0 ||
- expectedSize > static_cast<int64_t>(Config.Store.maxInMemObjSize))
- return 0;
- }
-
return 1;
}
int
StoreEntry::checkNegativeHit() const
{
if (!EBIT_TEST(flags, ENTRY_NEGCACHED))
return 0;
if (expires <= squid_curtime)
return 0;
if (store_status != STORE_OK)
return 0;
return 1;
}
/**
* Set object for negative caching.
=== modified file 'src/store_dir.cc'
--- src/store_dir.cc 2012-06-22 03:49:38 +0000
+++ src/store_dir.cc 2012-07-12 21:29:57 +0000
@@ -764,55 +764,89 @@
if (StoreEntry *e = sd->get(key)) {
debugs(20, 3, HERE << "cache_dir " << idx <<
" got cached entry: " << *e);
return e;
}
}
}
debugs(20, 4, HERE << "none of " << Config.cacheSwap.n_configured <<
" cache_dirs have " << storeKeyText(key));
return NULL;
}
void
StoreController::get(String const key, STOREGETCLIENT aCallback, void *aCallbackData)
{
fatal("not implemented");
}
+// move this into [non-shared] memory cache class when we have one
+/// whether e should be kept in local RAM for possible future caching
+bool
+StoreController::keepForLocalMemoryCache(const StoreEntry &e) const
+{
+ if (!e.memoryCachable())
+ return false;
+
+ // does the current and expected size obey memory caching limits?
+ assert(e.mem_obj);
+ const int64_t loadedSize = e.mem_obj->endOffset();
+ const int64_t expectedSize = e.mem_obj->expectedReplySize(); // may be < 0
+ const int64_t ramSize = max(loadedSize, expectedSize);
+ const int64_t ramLimit = min(
+ static_cast<int64_t>(Config.memMaxSize),
+ static_cast<int64_t>(Config.Store.maxInMemObjSize));
+ return ramSize <= ramLimit;
+}
+
+void
+StoreController::maybeTrimMemory(StoreEntry &e, const bool preserveSwappable)
+{
+ bool keepInLocalMemory = false;
+ if (memStore)
+ keepInLocalMemory = memStore->keepInLocalMemory(e);
+ else
+ keepInLocalMemory = keepForLocalMemoryCache(e);
+
+ debugs(20, 7, HERE << "keepInLocalMemory: " << keepInLocalMemory);
+
+ if (!keepInLocalMemory)
+ e.trimMemory(preserveSwappable);
+}
+
void
StoreController::handleIdleEntry(StoreEntry &e)
{
bool keepInLocalMemory = false;
if (EBIT_TEST(e.flags, ENTRY_SPECIAL)) {
// Icons (and cache digests?) should stay in store_table until we
// have a dedicated storage for them (that would not purge them).
// They are not managed [well] by any specific Store handled below.
keepInLocalMemory = true;
} else if (memStore) {
memStore->considerKeeping(e);
// leave keepInLocalMemory false; memStore maintains its own cache
} else {
- keepInLocalMemory = e.memoryCachable() && // entry is in good shape and
+ keepInLocalMemory = keepForLocalMemoryCache(e) && // in good shape and
// the local memory cache is not overflowing
(mem_node::InUseCount() <= store_pages_max);
}
// An idle, unlocked entry that belongs to a SwapDir which controls
// its own index, should not stay in the global store_table.
if (!dereference(e)) {
debugs(20, 5, HERE << "destroying unlocked entry: " << &e << ' ' << e);
destroyStoreEntry(static_cast<hash_link*>(&e));
return;
}
debugs(20, 5, HERE << "keepInLocalMemory: " << keepInLocalMemory);
// TODO: move this into [non-shared] memory cache class when we have one
if (keepInLocalMemory) {
e.setMemStatus(IN_MEMORY);
e.mem_obj->unlinkRequest();
} else {
e.purgeMem(); // may free e
=== modified file 'src/store_swapout.cc'
--- src/store_swapout.cc 2012-02-10 00:01:17 +0000
+++ src/store_swapout.cc 2012-07-12 17:43:10 +0000
@@ -181,41 +181,41 @@
/* This routine is called every time data is sent to the client side.
* It's overhead is therefor, significant.
*/
void
StoreEntry::swapOut()
{
if (!mem_obj)
return;
// this flag may change so we must check even if we are swappingOut
if (EBIT_TEST(flags, ENTRY_ABORTED)) {
assert(EBIT_TEST(flags, RELEASE_REQUEST));
// StoreEntry::abort() already closed the swap out file, if any
// no trimming: data producer must stop production if ENTRY_ABORTED
return;
}
const bool weAreOrMayBeSwappingOut = swappingOut() || mayStartSwapOut();
- trimMemory(weAreOrMayBeSwappingOut);
+ Store::Root().maybeTrimMemory(*this, weAreOrMayBeSwappingOut);
if (!weAreOrMayBeSwappingOut)
return; // nothing else to do
// Aborted entries have STORE_OK, but swapoutPossible rejects them. Thus,
// store_status == STORE_OK below means we got everything we wanted.
debugs(20, 7, HERE << "storeSwapOut: mem->inmem_lo = " << mem_obj->inmem_lo);
debugs(20, 7, HERE << "storeSwapOut: mem->endOffset() = " << mem_obj->endOffset());
debugs(20, 7, HERE << "storeSwapOut: swapout.queue_offset = " << mem_obj->swapout.queue_offset);
if (mem_obj->swapout.sio != NULL)
debugs(20, 7, "storeSwapOut: storeOffset() = " << mem_obj->swapout.sio->offset() );
int64_t const lowest_offset = mem_obj->lowestMemReaderOffset();
debugs(20, 7, HERE << "storeSwapOut: lowest_offset = " << lowest_offset);
#if SIZEOF_OFF_T <= 4