Optimization of loading sync entries from the DB to avoid deep copy o…

…f EntitySpecifics

BUG=517275

Review URL: https://codereview.chromium.org/1220393002

Cr-Commit-Position: refs/heads/master@{#342428}

sync/syncable/directory_backing_store.cc

@@ -105,9 +105,7 @@ void UnpackProtoFields(sql::Statement* statement,
                    static_cast<TField>(*index));
     } else {
       // Regular case - deserialize and copy the value to the field.
-      TValue value;
-      value.ParseFromArray(blob, length);
-      kernel->put(static_cast<TField>(*index), value);
+      kernel->load(static_cast<TField>(*index), blob, length);
       prev_blob = blob;
       prev_length = length;
       prev_index = *index;

sync/syncable/entry_kernel.h

@@ -336,6 +336,18 @@ struct SYNC_EXPORT_PRIVATE EntryKernel {
     return unique_position_fields[field - UNIQUE_POSITION_FIELDS_BEGIN];
   }
 
+  // Deserialization methods for ::google::protobuf::MessageLite derived types.
+  inline void load(ProtoField field, const void* blob, int length) {
+    specifics_fields[field - PROTO_FIELDS_BEGIN].load(blob, length);
+  }
+
+  inline void load(AttachmentMetadataField field,
+                   const void* blob,
+                   int length) {
+    attachment_metadata_fields[field - ATTACHMENT_METADATA_FIELDS_BEGIN].load(
+        blob, length);
+  }
+
   // Sharing data methods for ::google::protobuf::MessageLite derived types.
   inline void copy(ProtoField src, ProtoField dest) {
     DCHECK_NE(src, dest);

sync/syncable/proto_value_ptr.h

@@ -19,6 +19,10 @@ template <typename T>
 struct DefaultProtoValuePtrTraits {
   // Deep copy the value from |src| to |dest|.
   static void CopyValue(T* dest, const T& src) { dest->CopyFrom(src); }
+  // Parse the value from BLOB.
+  static void ParseFromBlob(T* dest, const void* blob, int length) {
+    dest->ParseFromArray(blob, length);
+  }
   // True if the |value| is a non-default value.
   static bool HasValue(const T& value) { return value.ByteSize() > 0; }
   // Default value for the type.
@@ -47,9 +51,16 @@ class ProtoValuePtr {
    public:
     Wrapper(const T& value) { Traits::CopyValue(&value_, value); }
     const T& value() const { return value_; }
+    // Create wrapper by deserializing a BLOB.
+    static Wrapper* ParseFromBlob(const void* blob, int length) {
+      Wrapper* wrapper = new Wrapper;
+      Traits::ParseFromBlob(&wrapper->value_, blob, length);
+      return wrapper;
+    }
 
    private:
     friend class base::RefCountedThreadSafe<Wrapper>;
+    Wrapper() {}
     ~Wrapper() {}
 
     T value_;
@@ -72,6 +83,7 @@ class ProtoValuePtr {
   friend struct EntryKernel;
   FRIEND_TEST_ALL_PREFIXES(ProtoValuePtrTest, BasicTest);
   FRIEND_TEST_ALL_PREFIXES(ProtoValuePtrTest, SharingTest);
+  FRIEND_TEST_ALL_PREFIXES(ProtoValuePtrTest, ParsingTest);
 
   void set_value(const T& new_value) {
     if (Traits::HasValue(new_value)) {
@@ -82,6 +94,10 @@ class ProtoValuePtr {
     }
   }
 
+  void load(const void* blob, int length) {
+    wrapper_ = Wrapper::ParseFromBlob(blob, length);
+  }
+
   scoped_refptr<Wrapper> wrapper_;
 };
 

sync/syncable/proto_value_ptr_unittest.cc

@@ -22,18 +22,20 @@ class TestValue {
 
   static void ResetCounters() {
     g_copy_count = 0;
+    g_parse_count = 0;
     g_delete_count = 0;
   }
 
   static int copy_count() { return g_copy_count; }
+  static int parse_count() { return g_parse_count; }
   static int delete_count() { return g_delete_count; }
 
   int value() const { return value_; }
   bool is_initialized() const { return is_initialized_; }
   bool is_default() const { return is_default_; }
 
   // TestValue uses the default traits struct with ProtoValuePtr<TestValue>.
-  // The following 3 functions are expected by the traits struct to exist
+  // The following 4 functions are expected by the traits struct to exist
   // in this class.
   void CopyFrom(const TestValue& from) {
     // Expected to always copy from an initialized instance
@@ -44,10 +46,22 @@ class TestValue {
     ASSERT_TRUE(from.is_initialized());
     ASSERT_FALSE(from.is_default());
     value_ = from.value();
-    is_initialized_ = false;
+    is_initialized_ = true;
     g_copy_count++;
   }
 
+  void ParseFromArray(const void* blob, int length) {
+    // Similarly to CopyFrom this is expected to be called on
+    // an uninitialized instance.
+    ASSERT_FALSE(is_initialized());
+    ASSERT_FALSE(is_default());
+    // The blob is an address of an integer
+    ASSERT_EQ(static_cast<int>(sizeof(int)), length);
+    value_ = *static_cast<const int*>(blob);
+    is_initialized_ = true;
+    g_parse_count++;
+  }
+
   int ByteSize() const { return is_initialized() ? sizeof(int) : 0; }
 
   static const TestValue& default_instance() {
@@ -58,6 +72,7 @@ class TestValue {
 
  private:
   static int g_copy_count;
+  static int g_parse_count;
   static int g_delete_count;
 
   int value_;
@@ -69,6 +84,7 @@ class TestValue {
 
 // Static initializers.
 int TestValue::g_copy_count = 0;
+int TestValue::g_parse_count = 0;
 int TestValue::g_delete_count = 0;
 
 }  // namespace
@@ -171,5 +187,22 @@ TEST_F(ProtoValuePtrTest, SharingTest) {
   EXPECT_EQ(2, TestValue::delete_count());
 }
 
+TEST_F(ProtoValuePtrTest, ParsingTest) {
+  int v1 = 21;
+
+  {
+    TestPtr ptr1;
+
+    ptr1.load(&v1, sizeof(int));
+
+    EXPECT_EQ(1, TestValue::parse_count());
+    EXPECT_EQ(0, TestValue::copy_count());
+
+    EXPECT_EQ(v1, ptr1->value());
+  }
+
+  EXPECT_EQ(1, TestValue::delete_count());
+}
+
 }  // namespace syncable
 }  // namespace syncer