Use compact object as our MainValue type

core/CMakeLists.txt

@@ -1,5 +1,5 @@
 add_library(dfly_core  compact_object.cc dragonfly_core.cc tx_queue.cc)
 cxx_link(dfly_core base absl::flat_hash_map redis_lib)
 
-cxx_test(dfly_core_test dfly_core)
-cxx_test(compact_object_test dfly_core)
+cxx_test(dfly_core_test dfly_core LABELS DFLY)
+cxx_test(compact_object_test dfly_core LABELS DFLY)

core/compact_object.h

@@ -54,7 +54,7 @@ class CompactBlob {
 
 static_assert(sizeof(CompactBlob) == 12, "");
 
-// Objects/blobs of upto 4GB size.
+// redis objects or blobs of upto 4GB size.
 struct RobjWrapper {
   size_t MallocUsed() const;
 
@@ -128,6 +128,10 @@ class CompactObj {
     return res;
   }
 
+  bool IsRef() const {
+    return mask_ & REF_BIT;
+  }
+
   std::string_view GetSlice(std::string* scratch) const;
 
   std::string ToString() const {
@@ -201,10 +205,6 @@ class CompactObj {
 
   bool HasAllocated() const;
 
-  bool IsRef() const {
-    return mask_ & REF_BIT;
-  }
-
   void SetMeta(uint8_t taglen, uint8_t mask = 0) {
     if (HasAllocated()) {
       Free();
@@ -216,6 +216,9 @@ class CompactObj {
   }
 
   // My main data structure. Union of representations.
+  // RobjWrapper is kInlineLen=16 bytes, so we have inline_str for SSO of that size.
+  // In case of int values, we waste 8 bytes. I am assuming it's not the data type
+  // with biggest memory usage.
   union U {
     char inline_str[kInlineLen];
 
@@ -226,6 +229,7 @@ class CompactObj {
     }
   } u_;
 
+  //
   static_assert(sizeof(u_) == 16, "");
 
   mutable uint8_t mask_ = 0;

server/db_slice.cc

@@ -78,7 +78,7 @@ pair<MainIterator, ExpireIterator> DbSlice::FindExt(DbIndex db_ind, std::string_
     if (expire_it->second <= now_ms_) {
       db->expire_table.erase(expire_it);
 
-      db->stats.obj_memory_usage -= (it->first.capacity() + it->second.str.capacity());
+      db->stats.obj_memory_usage -= (it->first.capacity() + it->second.MallocUsed());
       db->main_table.erase(it);
       return make_pair(MainIterator{}, ExpireIterator{});
     }
@@ -127,12 +127,8 @@ auto DbSlice::AddOrFind(DbIndex db_index, std::string_view key) -> pair<MainIter
       db->expire_table.erase(expire_it);
 
       // Keep the entry but free the object.
-      db->stats.obj_memory_usage -= existing->second.str.capacity();
-      existing->second.obj_type = OBJ_STRING;
-      if (existing->second.robj) {
-        decrRefCountVoid(existing->second.robj);
-        existing->second.robj = nullptr;
-      }
+      db->stats.obj_memory_usage -= existing->second.MallocUsed();
+      existing->second.Reset();
 
       return make_pair(existing, true);
     }
@@ -164,7 +160,7 @@ bool DbSlice::Del(DbIndex db_ind, const MainIterator& it) {
     CHECK_EQ(1u, db->expire_table.erase(it->first));
   }
 
-  db->stats.obj_memory_usage -= (it->first.capacity() + it->second.str.capacity());
+  db->stats.obj_memory_usage -= (it->first.capacity() + it->second.MallocUsed());
   db->main_table.erase(it);
 
   return true;
@@ -225,13 +221,15 @@ void DbSlice::AddNew(DbIndex db_ind, string_view key, MainValue obj, uint64_t ex
 }
 
 bool DbSlice::AddIfNotExist(DbIndex db_ind, string_view key, MainValue obj, uint64_t expire_at_ms) {
+  DCHECK(!obj.IsRef());
+
   auto& db = db_arr_[db_ind];
 
-  auto [new_entry, success] = db->main_table.emplace(key, obj);
+  auto [new_entry, success] = db->main_table.emplace(key, std::move(obj));
   if (!success)
     return false;  // in this case obj won't be moved and will be destroyed during unwinding.
 
-  db->stats.obj_memory_usage += (new_entry->first.capacity() + new_entry->second.str.capacity());
+  db->stats.obj_memory_usage += (new_entry->first.capacity() + new_entry->second.MallocUsed());
 
   if (expire_at_ms) {
     new_entry->second.SetExpire(true);

server/generic_family.cc

@@ -61,7 +61,7 @@ OpResult<void> Renamer::Find(ShardId shard_id, const ArgSlice& args) {
 
   res->found = IsValid(it);
   if (IsValid(it)) {
-    res->val = it->second;  // TODO: won't work for robj because we copy pointers.
+    res->val = it->second.AsRef();
     res->expire_ts = IsValid(exp_it) ? exp_it->second : 0;
   }
 
@@ -89,7 +89,8 @@ void Renamer::MoveValues(EngineShard* shard, const ArgSlice& args) {
     shard->db_slice().Expire(db_indx_, dest_it, src_res_.expire_ts);
   } else {
     // we just add the key to destination with the source object.
-    shard->db_slice().AddNew(db_indx_, dest_key, src_res_.val, src_res_.expire_ts);
+
+    shard->db_slice().AddNew(db_indx_, dest_key, std::move(src_res_.val), src_res_.expire_ts);
   }
 }
 
@@ -108,6 +109,15 @@ Transaction::RunnableType Renamer::Finalize(bool skip_exist_dest) {
     return cleanup;
   }
 
+  DCHECK(src_res_.val.IsRef());
+
+  // We can not copy from the existing value and delete it at the same time.
+  // TODO: if we want to allocate in shard, we must implement CompactObject::Clone.
+  // For now we hack it for strings only.
+  string val;
+  src_res_.val.GetString(&val);
+  src_res_.val.SetString(val);
+
   // Src key exist and we need to override the destination.
   return [this](Transaction* t, EngineShard* shard) {
     this->MoveValues(shard, t->ShardArgsInShard(shard->shard_id()));

server/list_family.cc

@@ -60,9 +60,7 @@ using namespace std;
 namespace {
 
 quicklist* GetQL(const MainValue& mv) {
-  DCHECK(mv.robj);
-  robj* o = (robj*)mv.robj;
-  return (quicklist*)o->ptr;
+  return mv.GetQL();
 }
 
 void* listPopSaver(unsigned char* data, size_t sz) {
@@ -334,8 +332,7 @@ OpResult<uint32_t> ListFamily::OpPush(const OpArgs& op_args, std::string_view ke
     robj* o = createQuicklistObject();
     ql = (quicklist*)o->ptr;
     quicklistSetOptions(ql, FLAGS_list_max_listpack_size, FLAGS_list_compress_depth);
-    it->second.robj = o;
-    it->second.obj_type = OBJ_LIST;
+    it->second.ImportRObj(o);
   } else {
     if (it->second.ObjType() != OBJ_LIST)
       return OpStatus::WRONG_TYPE;

server/string_family.cc

@@ -52,7 +52,10 @@ OpResult<void> SetCmd::Set(const SetParams& params, std::string_view key, std::s
     if (params.prev_val) {
       if (it->second.ObjType() != OBJ_STRING)
         return OpStatus::WRONG_TYPE;
-      params.prev_val->emplace(it->second.str);
+
+      string val;
+      it->second.GetString(&val);
+      params.prev_val->emplace(move(val));
     }
 
     return SetExisting(params.db_index, value, at_ms, it, expire_it);
@@ -61,7 +64,7 @@ OpResult<void> SetCmd::Set(const SetParams& params, std::string_view key, std::s
   if (params.how == SET_IF_EXISTS)
     return OpStatus::SKIPPED;
 
-  db_slice_->AddNew(params.db_index, key, value, at_ms);
+  db_slice_->AddNew(params.db_index, key, MainValue{value}, at_ms);
 
   return OpStatus::OK;
 }
@@ -74,12 +77,7 @@ OpResult<void> SetCmd::SetExisting(DbIndex db_ind, std::string_view value, uint6
     db_slice_->Expire(db_ind, dest, expire_at_ms);
   }
 
-  if (dest->second.robj) {
-    decrRefCountVoid(dest->second.robj);
-    dest->second.robj = nullptr;
-  }
-  dest->second = value;
-  dest->second.obj_type = OBJ_STRING;
+  dest->second.SetString(value);
 
   return OpStatus::OK;
 }
@@ -154,7 +152,8 @@ void StringFamily::Get(CmdArgList args, ConnectionContext* cntx) {
     if (!it_res.ok())
       return it_res.status();
 
-    string val = it_res.value()->second.str;
+    string val;
+    it_res.value()->second.GetString(&val);
 
     return val;
   };
@@ -272,7 +271,7 @@ auto StringFamily::OpMGet(const Transaction* t, EngineShard* shard) -> MGetRespo
   for (size_t i = 0; i < args.size(); ++i) {
     OpResult<MainIterator> it_res = db_slice.Find(t->db_index(), args[i], OBJ_STRING);
     if (it_res.ok()) {
-      response[i] = it_res.value()->second.str;
+      it_res.value()->second.GetString(&response[i].emplace());
     }
   }
 

server/table.h

@@ -1,4 +1,4 @@
-// Copyright 2021, Roman Gershman.  All rights reserved.
+// Copyright 2022, Roman Gershman.  All rights reserved.
 // See LICENSE for licensing terms.
 //
 
@@ -6,33 +6,11 @@
 
 #include <absl/container/flat_hash_map.h>
 
+#include "core/compact_object.h"
+
 namespace dfly {
 
-struct MainValue {
-  std::string str;
-  void* robj = nullptr;
-  uint8_t obj_type = 0;
-
-  MainValue() = default;
-  MainValue(std::string_view s) : str(s) {
-  }
-
-  bool HasExpire() const {
-    return has_expire_;
-  }
-
-  void SetExpire(bool b) {
-    has_expire_ = b;
-  }
-
-  unsigned ObjType() const {
-    return obj_type;
-  }
-
- private:
-  bool has_expire_ = false;
-};
-
+using MainValue = CompactObj;
 using MainTable = absl::flat_hash_map<std::string, MainValue>;
 using ExpireTable = absl::flat_hash_map<std::string, uint64_t>;