Remove redundant string contruction from Slice (#1745)

We remove lots of redundant string contruction from Slice to improve efficiency.
apache · Sep 7, 2023 · e096500 · e096500
1 parent 45d741a
commit e096500
Show file tree

Hide file tree

Showing 7 changed files with 13 additions and 18 deletions.
diff --git a/src/cluster/redis_slot.cc b/src/cluster/redis_slot.cc
@@ -65,11 +65,11 @@ uint16_t GetSlotIdFromKey(std::string_view key) {
 
 std::string_view GetTagFromKey(std::string_view key) {
   auto left_pos = key.find('{');
-  if (left_pos == std::string::npos) return {};
+  if (left_pos == std::string_view::npos) return {};
 
   auto right_pos = key.find('}', left_pos + 1);
   // Note that we hash the whole key if there is nothing between {}.
-  if (right_pos == std::string::npos || right_pos <= left_pos + 1) {
+  if (right_pos == std::string_view::npos || right_pos <= left_pos + 1) {
     return {};
   }
 

diff --git a/src/storage/batch_extractor.cc b/src/storage/batch_extractor.cc
@@ -58,7 +58,7 @@ rocksdb::Status WriteBatchExtractor::PutCF(uint32_t column_family_id, const Slic
     }
 
     Metadata metadata(kRedisNone);
-    metadata.Decode(value.ToString());
+    metadata.Decode(value);
 
     if (metadata.Type() == kRedisString) {
       command_args = {"SET", user_key, value.ToString().substr(Metadata::GetOffsetAfterExpire(value[0]))};
@@ -92,7 +92,7 @@ rocksdb::Status WriteBatchExtractor::PutCF(uint32_t column_family_id, const Slic
       }
 
       StreamMetadata stream_metadata;
-      auto s = stream_metadata.Decode(value.ToString());
+      auto s = stream_metadata.Decode(value);
       if (!s.ok()) return s;
 
       command_args = {"XSETID",

diff --git a/src/storage/compact_filter.cc b/src/storage/compact_filter.cc
@@ -35,9 +35,8 @@ using rocksdb::Slice;
 
 bool MetadataFilter::Filter(int level, const Slice &key, const Slice &value, std::string *new_value,
                             bool *modified) const {
-  std::string bytes = value.ToString();
   Metadata metadata(kRedisNone, false);
-  rocksdb::Status s = metadata.Decode(bytes);
+  rocksdb::Status s = metadata.Decode(value);
   auto [ns, user_key] = ExtractNamespaceKey(key, stor_->IsSlotIdEncoded());
   if (!s.ok()) {
     LOG(WARNING) << "[compact_filter/metadata] Failed to decode,"
@@ -106,8 +105,7 @@ rocksdb::CompactionFilter::Decision SubKeyFilter::FilterBlobByKey(int level, con
   }
   if (!s.IsOK()) {
     LOG(ERROR) << "[compact_filter/subkey] Failed to get metadata"
-               << ", namespace: " << ikey.GetNamespace().ToString() << ", key: " << ikey.GetKey().ToString()
-               << ", err: " << s.Msg();
+               << ", namespace: " << ikey.GetNamespace() << ", key: " << ikey.GetKey() << ", err: " << s.Msg();
     return rocksdb::CompactionFilter::Decision::kKeep;
   }
   // bitmap will be checked in Filter
@@ -129,8 +127,7 @@ bool SubKeyFilter::Filter(int level, const Slice &key, const Slice &value, std::
   }
   if (!s.IsOK()) {
     LOG(ERROR) << "[compact_filter/subkey] Failed to get metadata"
-               << ", namespace: " << ikey.GetNamespace().ToString() << ", key: " << ikey.GetKey().ToString()
-               << ", err: " << s.Msg();
+               << ", namespace: " << ikey.GetNamespace() << ", key: " << ikey.GetKey() << ", err: " << s.Msg();
     return false;
   }
 

diff --git a/src/storage/redis_db.cc b/src/storage/redis_db.cc
@@ -210,7 +210,7 @@ void Database::GetKeyNumStats(const std::string &prefix, KeyNumStats *stats) { K
 
 void Database::Keys(const std::string &prefix, std::vector<std::string> *keys, KeyNumStats *stats) {
   uint16_t slot_id = 0;
-  std::string ns_prefix, value;
+  std::string ns_prefix;
   if (namespace_ != kDefaultNamespace || keys != nullptr) {
     if (storage_->IsSlotIdEncoded()) {
       ns_prefix = ComposeNamespaceKey(namespace_, "", false);
@@ -236,8 +236,7 @@ void Database::Keys(const std::string &prefix, std::vector<std::string> *keys, K
         break;
       }
       Metadata metadata(kRedisNone, false);
-      value = iter->value().ToString();
-      metadata.Decode(value);
+      metadata.Decode(iter->value());
       if (metadata.Expired()) {
         if (stats) stats->n_expired++;
         continue;
@@ -313,8 +312,7 @@ rocksdb::Status Database::Scan(const std::string &cursor, uint64_t limit, const
         break;
       }
       Metadata metadata(kRedisNone, false);
-      std::string value = iter->value().ToString();
-      metadata.Decode(value);
+      metadata.Decode(iter->value());
       if (metadata.Expired()) continue;
       std::tie(std::ignore, user_key) = ExtractNamespaceKey<std::string>(iter->key(), storage_->IsSlotIdEncoded());
       keys->emplace_back(user_key);

diff --git a/src/storage/redis_metadata.cc b/src/storage/redis_metadata.cc
@@ -127,7 +127,7 @@ std::string ComposeNamespaceKey(const Slice &ns, const Slice &key, bool slot_id_
   ns_key.append(ns.data(), ns.size());
 
   if (slot_id_encoded) {
-    auto slot_id = GetSlotIdFromKey(key.ToString());
+    auto slot_id = GetSlotIdFromKey(key.ToStringView());
     PutFixed16(&ns_key, slot_id);
   }
 

diff --git a/src/storage/storage.cc b/src/storage/storage.cc
@@ -1107,7 +1107,7 @@ bool Storage::ReplDataManager::FileExists(Storage *storage, const std::string &d
 
     if (slice.size() == 0) return false;
 
-    tmp_crc = rocksdb::crc32c::Extend(0, slice.ToString().c_str(), slice.size());
+    tmp_crc = rocksdb::crc32c::Extend(0, slice.data(), slice.size());
     size -= slice.size();
   }
 

diff --git a/utils/kvrocks2redis/parser.cc b/utils/kvrocks2redis/parser.cc
@@ -44,7 +44,7 @@ Status Parser::ParseFullDB() {
 
   for (iter->SeekToFirst(); iter->Valid(); iter->Next()) {
     Metadata metadata(kRedisNone);
-    metadata.Decode(iter->value().ToString());
+    metadata.Decode(iter->value());
     if (metadata.Expired()) {  // ignore the expired key
       continue;
     }