From b0025e414e7b133b2ce5fb638113297b946d2b83 Mon Sep 17 00:00:00 2001
From: Rahul Kumar <rahulkmr@meta.com>
Date: Tue, 15 Nov 2022 08:11:16 -0800
Subject: [PATCH] Prevent copy of protocol::object on protocol::SerializeObject

Summary:
Inside protocol::SerializeObject following statement:
  val.objectValue_ref() = obj;
was making a copy of the object. This can be very costly when it is an object corresponding to a large struct. After this change, object serialization time reduced dramatically from 10X slower to 3X slower when compared to schema based serialization.

After perf numbers - https://docs.google.com/spreadsheets/d/1hlxjBt7PCi4tLUZO7NaZt5wBq22h3zi4-GjfbUP2xQU/edit?usp=sharing

Before perf numbers - https://www.internalfb.com/phabricator/paste/view/P549047640?lines=208

Reviewed By: Mizuchi

Differential Revision: D41216856

fbshipit-source-id: b5a79aad882c0318bf2556de27448f030bdd2e68
---
 .../thrift/src/thrift/lib/cpp2/protocol/Object.h   | 10 +++++----
 .../src/thrift/lib/cpp2/protocol/detail/Object.h   | 24 +++++++++++++---------
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/third-party/thrift/src/thrift/lib/cpp2/protocol/Object.h b/third-party/thrift/src/thrift/lib/cpp2/protocol/Object.h
index da78f31aef8..1962eeda246 100644
--- a/third-party/thrift/src/thrift/lib/cpp2/protocol/Object.h
+++ b/third-party/thrift/src/thrift/lib/cpp2/protocol/Object.h
@@ -107,10 +107,12 @@ std::unique_ptr<folly::IOBuf> serializeValue(const Value& val) {
 // obj: object to be serialized Serialized output is same as schema based
 // serialization except when struct contains an empty list, set or map
 template <class Protocol>
-std::unique_ptr<folly::IOBuf> serializeObject(const Object& obj) {
-  Value val;
-  val.objectValue_ref() = obj;
-  return serializeValue<Protocol>(val);
+std::unique_ptr<folly::IOBuf> serializeObject(const Object& val) {
+  Protocol prot;
+  folly::IOBufQueue queue(folly::IOBufQueue::cacheChainLength());
+  prot.setOutput(&queue);
+  detail::serializeObject(prot, val);
+  return queue.move();
 }
 
 // Serialization of protocol::Object with MaskedProtocolData.
diff --git a/third-party/thrift/src/thrift/lib/cpp2/protocol/detail/Object.h b/third-party/thrift/src/thrift/lib/cpp2/protocol/detail/Object.h
index 92f159aca7f..c034ee06981 100644
--- a/third-party/thrift/src/thrift/lib/cpp2/protocol/detail/Object.h
+++ b/third-party/thrift/src/thrift/lib/cpp2/protocol/detail/Object.h
@@ -701,16 +701,7 @@ void serializeValue(Protocol& prot, const Value& value) {
       return;
     }
     case Value::Type::objectValue: {
-      prot.writeStructBegin("");
-      for (const auto& [fieldID, fieldVal] :
-           *value.objectValue_ref()->members()) {
-        auto fieldType = getTType(fieldVal);
-        prot.writeFieldBegin("", fieldType, fieldID);
-        serializeValue(prot, fieldVal);
-        prot.writeFieldEnd();
-      }
-      prot.writeFieldStop();
-      prot.writeStructEnd();
+      serializeObject(prot, *value.objectValue_ref());
       return;
     }
     default: {
@@ -719,6 +710,19 @@ void serializeValue(Protocol& prot, const Value& value) {
   }
 }
 
+template <class Protocol>
+void serializeObject(Protocol& prot, const Object& obj) {
+  prot.writeStructBegin("");
+  for (const auto& [fieldID, fieldVal] : *obj.members()) {
+    auto fieldType = getTType(fieldVal);
+    prot.writeFieldBegin("", fieldType, fieldID);
+    serializeValue(prot, fieldVal);
+    prot.writeFieldEnd();
+  }
+  prot.writeFieldStop();
+  prot.writeStructEnd();
+}
+
 // Writes the field from raw data in MaskedData.
 template <class Protocol>
 void writeRawField(
-- 
2.11.4.GIT