Move the error description to a separate allocation as it is rarely used.

This reduces the size of frequently used objects.

5 files changed, 67 insertions, 30 deletions

diff --git a/storageapi/src/tests/mbusprot/storageprotocoltest.cpp b/storageapi/src/tests/mbusprot/storageprotocoltest.cpp
index c2ebffb23e8..01719fbd7ec 100644
--- a/storageapi/src/tests/mbusprot/storageprotocoltest.cpp
+++ b/storageapi/src/tests/mbusprot/storageprotocoltest.cpp
@@ -816,13 +816,13 @@ TEST_P(StorageProtocolTest, serialized_size_is_used_to_set_approx_size_of_storag
 }
 
 TEST_P(StorageProtocolTest, track_memory_footprint_for_some_messages) {
-    EXPECT_EQ(136u, sizeof(StorageReply));
-    EXPECT_EQ(160u, sizeof(BucketReply));
-    EXPECT_EQ(192u, sizeof(BucketInfoReply));
-    EXPECT_EQ(336u, sizeof(PutReply));
-    EXPECT_EQ(320u, sizeof(UpdateReply));
-    EXPECT_EQ(312u, sizeof(RemoveReply));
-    EXPECT_EQ(400u, sizeof(GetReply));
+    EXPECT_EQ(80u, sizeof(StorageReply));
+    EXPECT_EQ(104u, sizeof(BucketReply));
+    EXPECT_EQ(136u, sizeof(BucketInfoReply));
+    EXPECT_EQ(280u, sizeof(PutReply));
+    EXPECT_EQ(264u, sizeof(UpdateReply));
+    EXPECT_EQ(256u, sizeof(RemoveReply));
+    EXPECT_EQ(344u, sizeof(GetReply));
     EXPECT_EQ(80u, sizeof(StorageCommand));
     EXPECT_EQ(104u, sizeof(BucketCommand));
     EXPECT_EQ(104u, sizeof(BucketInfoCommand));
diff --git a/storageapi/src/vespa/storageapi/message/visitor.h b/storageapi/src/vespa/storageapi/message/visitor.h
index 67c41a0cc4d..8440591ecde 100644
--- a/storageapi/src/vespa/storageapi/message/visitor.h
+++ b/storageapi/src/vespa/storageapi/message/visitor.h
@@ -205,18 +205,19 @@ public:
     VisitorInfoCommand();
     ~VisitorInfoCommand() override;
 
-    void setErrorCode(const ReturnCode& code) { _error = code; }
+    void setErrorCode(ReturnCode && code) { _error = std::move(code); }
     void setCompleted() { _completed = true; }
-    void setBucketCompleted(const document::BucketId& id, Timestamp lastVisited)
-    {
+    void setBucketCompleted(const document::BucketId& id, Timestamp lastVisited) {
         _bucketsCompleted.push_back(BucketTimestampPair(id, lastVisited));
     }
-    void setBucketsCompleted(const std::vector<BucketTimestampPair>& bc)
-        { _bucketsCompleted = bc; }
+    void setBucketsCompleted(const std::vector<BucketTimestampPair>& bc) {
+        _bucketsCompleted = bc;
+    }
 
     const ReturnCode& getErrorCode() const { return _error; }
-    const std::vector<BucketTimestampPair>& getCompletedBucketsList() const
-        { return _bucketsCompleted; }
+    const std::vector<BucketTimestampPair>& getCompletedBucketsList() const {
+        return _bucketsCompleted;
+    }
     bool visitorCompleted() const { return _completed; }
 
     void print(std::ostream& out, bool verbose, const std::string& indent) const override;
diff --git a/storageapi/src/vespa/storageapi/messageapi/returncode.cpp b/storageapi/src/vespa/storageapi/messageapi/returncode.cpp
index d5c8cb7da68..f2bcb54ca41 100644
--- a/storageapi/src/vespa/storageapi/messageapi/returncode.cpp
+++ b/storageapi/src/vespa/storageapi/messageapi/returncode.cpp
@@ -10,17 +10,39 @@ ReturnCode::ReturnCode()
       _message()
 {}
 
-ReturnCode::ReturnCode(const ReturnCode &) = default;
-ReturnCode & ReturnCode::operator = (const ReturnCode &) = default;
 ReturnCode & ReturnCode::operator = (ReturnCode &&) noexcept = default;
 ReturnCode::~ReturnCode() = default;
 
-ReturnCode::ReturnCode(Result result, vespalib::stringref msg)
+ReturnCode::ReturnCode(Result result)
     : _result(result),
-      _message(msg)
+      _message()
 {}
 
-vespalib::string ReturnCode::getResultString(Result result) {
+ReturnCode::ReturnCode(Result result, vespalib::stringref msg)
+    : _result(result),
+      _message()
+{
+    if ( ! msg.empty()) {
+        _message = std::make_unique<vespalib::string>(msg);
+    }
+}
+
+ReturnCode::ReturnCode(const ReturnCode & rhs)
+    : _result(rhs._result),
+      _message()
+{
+    if (rhs._message) {
+        _message = std::make_unique<vespalib::string>(*rhs._message);
+    }
+}
+
+ReturnCode &
+ReturnCode::operator = (const ReturnCode & rhs) {
+    return operator=(ReturnCode(rhs));
+}
+
+vespalib::string
+ReturnCode::getResultString(Result result) {
     return documentapi::DocumentProtocol::getErrorName(result);
 }
 
@@ -28,9 +50,9 @@ vespalib::string
 ReturnCode::toString() const {
     vespalib::string ret = "ReturnCode(";
     ret += getResultString(_result);
-    if ( ! _message.empty()) {
+    if ( _message && ! _message->empty()) {
         ret += ", ";
-        ret += _message;
+        ret += *_message;
     }
     ret += ")";
     return ret;
@@ -145,5 +167,21 @@ ReturnCode::isBucketDisappearance() const
             return false;
     }
 }
+namespace {
+    vespalib::stringref Empty("");
+}
+vespalib::stringref
+ReturnCode::getMessage() const {
+    return _message ? _message->operator vespalib::stringref() : Empty;
+}
+
+bool
+ReturnCode::operator==(const ReturnCode& code) const {
+    return (_result == code._result) && (getMessage() == code.getMessage());
+}
 
+bool
+ReturnCode::operator!=(const ReturnCode& code) const {
+    return (_result != code._result) || (getMessage() != code.getMessage());
+}
 }
diff --git a/storageapi/src/vespa/storageapi/messageapi/returncode.h b/storageapi/src/vespa/storageapi/messageapi/returncode.h
index 0149ae29a05..bef59a334d9 100644
--- a/storageapi/src/vespa/storageapi/messageapi/returncode.h
+++ b/storageapi/src/vespa/storageapi/messageapi/returncode.h
@@ -59,18 +59,18 @@ public:
 
 private:
     Result _result;
-    vespalib::string _message;
+    std::unique_ptr<vespalib::string> _message;
 public:
     ReturnCode();
-    explicit ReturnCode(Result result, vespalib::stringref msg = "");
+    explicit ReturnCode(Result result);
+    explicit ReturnCode(Result result, vespalib::stringref msg);
     ReturnCode(const ReturnCode &);
     ReturnCode & operator = (const ReturnCode &);
     ReturnCode(ReturnCode &&) noexcept = default;
     ReturnCode & operator = (ReturnCode &&) noexcept;
     ~ReturnCode();
 
-    const vespalib::string& getMessage() const { return _message; }
-    void setMessage(vespalib::stringref message) { _message = message; }
+    vespalib::stringref getMessage() const;
 
     Result getResult() const { return _result; }
 
@@ -85,10 +85,8 @@ public:
 
     bool operator==(Result res) const { return _result == res; }
     bool operator!=(Result res) const { return _result != res; }
-    bool operator==(const ReturnCode& code) const
-        { return _result == code._result && _message == code._message; }
-    bool operator!=(const ReturnCode& code) const
-        { return _result != code._result || _message != code._message; }
+    bool operator==(const ReturnCode& code) const;
+    bool operator!=(const ReturnCode& code) const;
 
     // To avoid lots of code matching various return codes in storage, we define
     // some functions they can use to match those codes that corresponds to what
diff --git a/storageapi/src/vespa/storageapi/messageapi/storagereply.h b/storageapi/src/vespa/storageapi/messageapi/storagereply.h
index 1a3bbe35eb4..53516949110 100644
--- a/storageapi/src/vespa/storageapi/messageapi/storagereply.h
+++ b/storageapi/src/vespa/storageapi/messageapi/storagereply.h
@@ -30,7 +30,7 @@ public:
     ~StorageReply() override;
     DECLARE_POINTER_TYPEDEFS(StorageReply);
 
-    void setResult(const ReturnCode& r) { _result = r; }
+    void setResult(ReturnCode r) { _result = std::move(r); }
     void setResult(ReturnCode::Result r) { _result = ReturnCode(r); }
     const ReturnCode& getResult() const { return _result; }
     void print(std::ostream& out, bool verbose, const std::string& indent) const override;