Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/653#discussion_r64924636
--- Diff: iocore/hostdb/P_RefCountCache.h ---
@@ -0,0 +1,572 @@
+#ifndef _P_RefCountCache_h_
+#define _P_RefCountCache_h_
+
+#include "I_EventSystem.h"
+
+#include <ts/Map.h>
+#include <ts/List.h>
+
+#include <cstdint>
+// TODO: no vector?
+#include <vector>
+#include <unistd.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <sys/mman.h>
+#include <cmath>
+#include <fstream>
+
+#include "ts/I_Version.h"
+
+#define REFCOUNT_CACHE_EVENT_SYNC REFCOUNT_CACHE_EVENT_EVENTS_START
+
+#define REFCOUNTCACHE_MAGIC_NUMBER 0x0BAD2D9
+#define REFCOUNTCACHE_MAJOR_VERSION 1
+#define REFCOUNTCACHE_MINOR_VERSION 0
+
+// TODO: elsewhere?
+#define PtrMutex Ptr<ProxyMutex>
+
+// In the cache we want to recount our items
+// This class will handle memory allocations (on the freelist) and
+// take care of implementing RefCountObj-- so that the items we want in
+// the cache don't have to inherit from it (which makes
serialization/deserialization much easier)
+class RefCountCacheItemBase : public RefCountObj
+{
+public:
+ RefCountCacheItemBase(uint64_t key = 0, unsigned int size = 0, char
*iobuf = NULL, int iobuffer_index = 0)
+ {
+ this->key = key;
+ this->size = size;
+ this->iobuf = iobuf;
+ this->iobuffer_index = iobuffer_index;
+ }
+ void
+ free()
+ {
+ if (this->size > 0) {
+ // free the block
+ ioBufAllocator[iobuffer_index].free_void((void *)(iobuf));
+ }
+ }
+ uint64_t key; // Key of this item
+ unsigned int size; // how much space does this guy get
+ int64_t iobuffer_index;
+ char *iobuf;
+};
+
+// Template to the particular class, so we can caste the item ptr correctly
+template <class C> class RefCountCacheItem : public RefCountCacheItemBase
+{
+public:
+ using RefCountCacheItemBase::RefCountCacheItemBase;
+ C *item();
+};
+
+template <class C>
+C *
+RefCountCacheItem<C>::item()
+{
+ return (C *)this->iobuf;
+}
+
+// Layer of inderection for the hashmap-- since it needs lots of things
inside of it
+class RefCountCacheHashingValue
+{
+public:
+ Ptr<RefCountCacheItemBase> item;
+ LINK(RefCountCacheHashingValue, item_link);
+ RefCountCacheHashingValue(Ptr<RefCountCacheItemBase> i) : item(i){};
+};
+
+// TODO: template?
+struct RefCountCacheHashing {
+ typedef uint64_t ID;
+ typedef uint64_t const Key;
+ typedef RefCountCacheHashingValue Value;
+ typedef DList(RefCountCacheHashingValue, item_link) ListHead;
+
+ static ID
+ hash(Key key)
+ {
+ return key;
+ }
+ static Key
+ key(Value const *value)
+ {
+ return value->item->key;
+ }
+ static bool
+ equal(Key lhs, Key rhs)
+ {
+ return lhs == rhs;
+ }
+};
+
+// The RefCountCachePartition is simply a map of key ->
Ptr<RefCountCacheItem<YourClass>>
+// TODO: add metrics
+// TODO: store partition number for debugging
+template <class C> class RefCountCachePartition
+{
+public:
+ RefCountCachePartition(int maxSize, int maxItems);
+ RefCountCacheItem<C> *alloc(uint64_t key, int size = 0);
+ Ptr<RefCountCacheItem<C>> get(uint64_t key);
+ void del(uint64_t key);
+ void clear();
+
+ int numItems();
+ void copy(std::vector<Ptr<RefCountCacheItemBase>> *items);
+
+ typedef typename TSHashTable<RefCountCacheHashing>::iterator
iteratortype;
+ typedef typename TSHashTable<RefCountCacheHashing>::self hashtype;
+ typedef typename TSHashTable<RefCountCacheHashing>::Location
locationtype;
+ TSHashTable<RefCountCacheHashing> *getMap();
+
+ PtrMutex lock; // Lock
+
+private:
+ int maxSize;
+ int maxItems;
+
+ int size;
+ int items;
+
+ hashtype itemMap;
+};
+
+template <class C> RefCountCachePartition<C>::RefCountCachePartition(int
maxSize, int maxItems)
+{
+ this->maxSize = maxSize;
+ this->maxItems = maxItems;
+ this->size = 0;
+ this->items = 0;
+
+ // Initialize lock
+ this->lock = new_ProxyMutex();
+}
+
+// TODO: error somehow if you asked for something too big!
+// Allocate space for an item `C` as well as `size` extra space.
+template <class C>
+RefCountCacheItem<C> *
+RefCountCachePartition<C>::alloc(uint64_t key, int size)
+{
+ size += sizeof(C);
+ // Remove any colliding entries
+ this->del(key);
+
+ // Now allocat a sufficiently large iobuf to store our data
+ // as well as the intermediate layers we have
+ int allocSize = size + sizeof(RefCountCacheItem<C>) +
sizeof(RefCountCacheHashingValue);
+ // allocate a sufficiently large block from the ioBufAllocator
+ int iobuffer_index = iobuffer_size_to_index(allocSize);
+ ink_release_assert(iobuffer_index >= 0);
+ char *iobuf = (char *)ioBufAllocator[iobuffer_index].alloc_void();
+
+ RefCountCacheItem<C> *item = new (iobuf + size)
RefCountCacheItem<C>(key, size, iobuf, iobuffer_index);
+
+ // TODO: either put the RefCountCacheItem on the same iobuf, or move this
+ // to a class allocator
+
+ // if we are past our limits, then lets not store a reference to the
return in our map
+ if ((this->maxItems > 0 && this->items > this->maxItems) ||
(this->maxSize > 0 && this->size > this->maxSize)) {
+ return item;
+ }
+
+ // Add our entry to the map
+ RefCountCacheHashingValue *val =
+ new ((char *)item + sizeof(RefCountCacheItem<C>))
RefCountCacheHashingValue(make_ptr((RefCountCacheItemBase *)item));
+ this->itemMap.insert(val);
+
+ return item;
+}
+
+template <class C>
+Ptr<RefCountCacheItem<C>>
+RefCountCachePartition<C>::get(uint64_t key)
+{
+ locationtype l = this->itemMap.find(key);
+ if (l.isValid()) {
+ // found
+ return make_ptr((RefCountCacheItem<C> *)l.m_value->item.get());
+ } else {
+ return Ptr<RefCountCacheItem<C>>(NULL);
+ }
+}
+
+template <class C>
+void
+RefCountCachePartition<C>::del(uint64_t key)
+{
+ this->itemMap.remove(key);
+}
+
+template <class C>
+void
+RefCountCachePartition<C>::clear()
+{
+ // Clear the in memory hashmap
+ // TODO: delete all items (not sure if clear calls delete on all items)
+ this->itemMap.clear();
+ this->items = 0;
+ this->size = 0;
+}
+
+template <class C>
+int
+RefCountCachePartition<C>::numItems()
+{
+ return this->itemMap.count();
+}
+
+template <class C>
+void
+RefCountCachePartition<C>::copy(std::vector<Ptr<RefCountCacheItemBase>>
*items)
+{
+ for (RefCountCachePartition<C>::iteratortype i = this->itemMap.begin();
i != this->itemMap.end(); ++i) {
+ items->push_back(make_ptr((RefCountCacheItemBase *)i->item.get()));
+ }
+}
+
+// TODO: pass an iterator or something? Seems not good to pass a pointer
to a member
+template <class C>
+TSHashTable<RefCountCacheHashing> *
+RefCountCachePartition<C>::getMap()
+{
+ return &this->itemMap;
+}
+
+// The implementation of this class must be in here, as this is used by
template
+// classes, and c++ rocks that way
+// The header for the cache, this is used to check if the serialized cache
is compatible
+class RefCountCacheHeader
+{
+public:
+ unsigned int magic;
+ VersionNumber version;
+
+ int numPartitions;
+ int partitionMaxSize;
+
+ RefCountCacheHeader(int numParts, int partSize)
+ : magic(REFCOUNTCACHE_MAGIC_NUMBER), numPartitions(numParts),
partitionMaxSize(partSize)
+ {
+ this->version.ink_major = REFCOUNTCACHE_MAJOR_VERSION;
+ this->version.ink_minor = REFCOUNTCACHE_MINOR_VERSION;
+ };
+
+ bool
+ operator==(const RefCountCacheHeader other) const
+ {
+ return (this->magic == other.magic && this->version.ink_major ==
other.version.ink_major &&
+ this->version.ink_minor == other.version.ink_minor &&
this->numPartitions == other.numPartitions &&
+ this->partitionMaxSize == other.partitionMaxSize);
+ }
+
+ bool
+ compatible(RefCountCacheHeader *other)
+ {
+ // printf("magic %d %d\n", this->magic, other->magic);
+ // printf("version.ink_major %d %d\n", this->version.ink_major,
other->version.ink_major);
+ // printf("numPartitions %d %d\n", this->numPartitions,
other->numPartitions);
+ // printf("partitionMaxSize %d %d\n", this->partitionMaxSize,
other->partitionMaxSize);
+
+ return (this->magic == other->magic && this->version.ink_major ==
other->version.ink_major &&
+ this->numPartitions == other->numPartitions &&
this->partitionMaxSize == other->partitionMaxSize);
+ };
+};
+
+// Base for syncing purposes
+// Since the actual RefCountCache is templated, we don't want to require
+// importing the template type for all uses (such as syncing)
+class RefCountCacheBase
--- End diff --
Using ``Base`` as a base class name is fairly ugly. This isn't really
related to the cache interface at all, it is a persistence interface.
All the virtual methods ought to be pure virtual. All methods that don't
mutate the object should be ``const``.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
