Use string_view for pref interface to reduce copy
If you pass in a static string literal like "Hello World!", then with
parameter type of const string& you need to construct a new string
object, requiring a copy. It will also require a copy if your data is in
another container, for example std::vector<char> . In update_engine, we
store manifest bytes in std::vector, and sometimes we want to save that
manifest to disk. This CL can help us reduce copy of the manifest(up to
2MB).
Test: treehugger
Change-Id: I70feb4c0673c174fd47f02c4bd41994f74cda743
diff --git a/common/fake_prefs.cc b/common/fake_prefs.cc
index 275667e..ea6ea60 100644
--- a/common/fake_prefs.cc
+++ b/common/fake_prefs.cc
@@ -17,6 +17,7 @@
#include "update_engine/common/fake_prefs.h"
#include <algorithm>
+#include <utility>
#include <gtest/gtest.h>
@@ -66,8 +67,8 @@
return GetValue(key, value);
}
-bool FakePrefs::SetString(const string& key, const string& value) {
- SetValue(key, value);
+bool FakePrefs::SetString(const string& key, std::string_view value) {
+ SetValue(key, std::string(value));
return true;
}
@@ -149,10 +150,10 @@
}
template <typename T>
-void FakePrefs::SetValue(const string& key, const T& value) {
+void FakePrefs::SetValue(const string& key, T value) {
CheckKeyType(key, PrefConsts<T>::type);
values_[key].type = PrefConsts<T>::type;
- values_[key].value.*(PrefConsts<T>::member) = value;
+ values_[key].value.*(PrefConsts<T>::member) = std::move(value);
const auto observers_for_key = observers_.find(key);
if (observers_for_key != observers_.end()) {
std::vector<ObserverInterface*> copy_observers(observers_for_key->second);
diff --git a/common/fake_prefs.h b/common/fake_prefs.h
index 9af2550..430c291 100644
--- a/common/fake_prefs.h
+++ b/common/fake_prefs.h
@@ -19,6 +19,7 @@
#include <map>
#include <string>
+#include <string_view>
#include <vector>
#include <base/macros.h>
@@ -40,7 +41,7 @@
// PrefsInterface methods.
bool GetString(const std::string& key, std::string* value) const override;
- bool SetString(const std::string& key, const std::string& value) override;
+ bool SetString(const std::string& key, std::string_view value) override;
bool GetInt64(const std::string& key, int64_t* value) const override;
bool SetInt64(const std::string& key, const int64_t value) override;
bool GetBoolean(const std::string& key, bool* value) const override;
@@ -96,7 +97,7 @@
// Helper function to set a value of the passed |key|. It sets the type based
// on the template parameter T.
template <typename T>
- void SetValue(const std::string& key, const T& value);
+ void SetValue(const std::string& key, T value);
// Helper function to get a value from the map checking for invalid calls.
// The function fails the test if you attempt to read a value defined as a
diff --git a/common/mock_prefs.h b/common/mock_prefs.h
index c91664e..49431fb 100644
--- a/common/mock_prefs.h
+++ b/common/mock_prefs.h
@@ -31,8 +31,7 @@
public:
MOCK_CONST_METHOD2(GetString,
bool(const std::string& key, std::string* value));
- MOCK_METHOD2(SetString,
- bool(const std::string& key, const std::string& value));
+ MOCK_METHOD2(SetString, bool(const std::string& key, std::string_view value));
MOCK_CONST_METHOD2(GetInt64, bool(const std::string& key, int64_t* value));
MOCK_METHOD2(SetInt64, bool(const std::string& key, const int64_t value));
diff --git a/common/prefs.cc b/common/prefs.cc
index 84fe536..1e06be4 100644
--- a/common/prefs.cc
+++ b/common/prefs.cc
@@ -55,7 +55,7 @@
return storage_->GetKey(key, value);
}
-bool PrefsBase::SetString(const string& key, const string& value) {
+bool PrefsBase::SetString(const string& key, std::string_view value) {
TEST_AND_RETURN_FALSE(storage_->SetKey(key, value));
const auto observers_for_key = observers_.find(key);
if (observers_for_key != observers_.end()) {
@@ -192,7 +192,7 @@
return true;
}
-bool Prefs::FileStorage::SetKey(const string& key, const string& value) {
+bool Prefs::FileStorage::SetKey(const string& key, std::string_view value) {
base::FilePath filename;
TEST_AND_RETURN_FALSE(GetFileNameForKey(key, &filename));
if (!base::DirectoryExists(filename.DirName())) {
@@ -263,7 +263,7 @@
}
bool MemoryPrefs::MemoryStorage::SetKey(const string& key,
- const string& value) {
+ std::string_view value) {
values_[key] = value;
return true;
}
diff --git a/common/prefs.h b/common/prefs.h
index d6ef668..93477dd 100644
--- a/common/prefs.h
+++ b/common/prefs.h
@@ -19,6 +19,7 @@
#include <map>
#include <string>
+#include <string_view>
#include <vector>
#include <base/files/file_path.h>
@@ -49,7 +50,7 @@
// Set the value of the key named |key| to |value| regardless of the
// previous value. Returns whether the operation succeeded.
- virtual bool SetKey(const std::string& key, const std::string& value) = 0;
+ virtual bool SetKey(const std::string& key, std::string_view value) = 0;
// Returns whether the key named |key| exists.
virtual bool KeyExists(const std::string& key) const = 0;
@@ -66,7 +67,7 @@
// PrefsInterface methods.
bool GetString(const std::string& key, std::string* value) const override;
- bool SetString(const std::string& key, const std::string& value) override;
+ bool SetString(const std::string& key, std::string_view value) override;
bool GetInt64(const std::string& key, int64_t* value) const override;
bool SetInt64(const std::string& key, const int64_t value) override;
bool GetBoolean(const std::string& key, bool* value) const override;
@@ -123,7 +124,7 @@
bool GetKey(const std::string& key, std::string* value) const override;
bool GetSubKeys(const std::string& ns,
std::vector<std::string>* keys) const override;
- bool SetKey(const std::string& key, const std::string& value) override;
+ bool SetKey(const std::string& key, std::string_view value) override;
bool KeyExists(const std::string& key) const override;
bool DeleteKey(const std::string& key) override;
@@ -163,7 +164,7 @@
bool GetKey(const std::string& key, std::string* value) const override;
bool GetSubKeys(const std::string& ns,
std::vector<std::string>* keys) const override;
- bool SetKey(const std::string& key, const std::string& value) override;
+ bool SetKey(const std::string& key, std::string_view value) override;
bool KeyExists(const std::string& key) const override;
bool DeleteKey(const std::string& key) override;
diff --git a/common/prefs_interface.h b/common/prefs_interface.h
index 866d0ca..e773a35 100644
--- a/common/prefs_interface.h
+++ b/common/prefs_interface.h
@@ -52,7 +52,7 @@
// Associates |key| with a string |value|. Returns true on success,
// false otherwise.
- virtual bool SetString(const std::string& key, const std::string& value) = 0;
+ virtual bool SetString(const std::string& key, std::string_view value) = 0;
// Gets an int64_t |value| associated with |key|. Returns true on
// success, false on failure (including when the |key| is not
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index 5b41485..08118d8 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -436,7 +436,8 @@
return false;
manifest_valid_ = true;
if (!install_plan_->is_resume) {
- prefs_->SetString(kPrefsManifestBytes, {buffer_.begin(), buffer_.end()});
+ auto begin = reinterpret_cast<const char*>(buffer_.data());
+ prefs_->SetString(kPrefsManifestBytes, {begin, buffer_.size()});
}
// Clear the download buffer.