Sync: Implement sharing of sync data in the directory using ProtoValu…

…ePtr

1) In EntryKernel sync_pb::EntitySpecifics and sync_pb::AttachmentMetadata
fields are wrapped with ProtoValuePtr smart pointers that implement COW.
This allows implicit sharing of the data when entire instances of
EntryKernel are copied.
2) Mutable access to those fields (mutable_ref methods) is removed from
EntryKernel and copy methods are added to allow sharing of
data between the server and the client fields of EntryKernel.
3) The code that loads EntryKernel fields from Sync DB is modified to
perform binary comparison of the serialized data and share the values
between the server and the client fields of EntryKernel.
4) In MutableEntry and ModelNeutralMutableEntry, the following Put*
methods are modified to perform an additional check for the sharing opportunity
between the assigned field and another field that is likely to be source
for the value being assigned and therefore have an identical value:
- PutServerSpecifics checks against SPECIFICS
- PutSpecifics checks agains against SERVER_SPECIFICS
- PutBaseServerSpecifics checks against SERVER_SPECIFICS
- PutServerAttachmentMetadata checks against ATTACHMENT_METADATA
- PutAttachmentMetadata checks against SERVER_ATTACHMENT_METADATA

BUG=499443

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

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

sync/syncable/directory_backing_store.cc

@@ -76,6 +76,46 @@ void BindFields(const EntryKernel& entry,
   }
 }
 
+// Helper function that loads a number of shareable fields of the
+// same type. The sharing criteria is based on comparison of
+// the serialized data. Only consecutive DB columns need to compared
+// to cover all possible sharing combinations.
+template <typename TValue, typename TField>
+void UnpackProtoFields(sql::Statement* statement,
+                       EntryKernel* kernel,
+                       int* index,
+                       int end_index) {
+  const void* prev_blob = nullptr;
+  int prev_length = -1;
+  int prev_index = -1;
+
+  for (; *index < end_index; ++(*index)) {
+    int length = statement->ColumnByteLength(*index);
+    if (length == 0) {
+      // Skip this column and keep the default value in the kernel field.
+      continue;
+    }
+
+    const void* blob = statement->ColumnBlob(*index);
+    // According to sqlite3 documentation, the prev_blob pointer should remain
+    // valid until moving to the next row.
+    if (length == prev_length && memcmp(blob, prev_blob, length) == 0) {
+      // Serialized values are the same - share the value from |prev_index|
+      // field with the current field.
+      kernel->copy(static_cast<TField>(prev_index),
+                   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);
+      prev_blob = blob;
+      prev_length = length;
+      prev_index = *index;
+    }
+  }
+}
+
 // The caller owns the returned EntryKernel*.  Assumes the statement currently
 // points to a valid row in the metas table. Returns NULL to indicate that
 // it detected a corruption in the data on unpacking.
@@ -101,10 +141,8 @@ scoped_ptr<EntryKernel> UnpackEntry(sql::Statement* statement) {
     kernel->put(static_cast<StringField>(i),
                 statement->ColumnString(i));
   }
-  for ( ; i < PROTO_FIELDS_END; ++i) {
-    kernel->mutable_ref(static_cast<ProtoField>(i)).ParseFromArray(
-        statement->ColumnBlob(i), statement->ColumnByteLength(i));
-  }
+  UnpackProtoFields<sync_pb::EntitySpecifics, ProtoField>(
+      statement, kernel.get(), &i, PROTO_FIELDS_END);
   for ( ; i < UNIQUE_POSITION_FIELDS_END; ++i) {
     std::string temp;
     statement->ColumnBlobAsString(i, &temp);
@@ -118,10 +156,8 @@ scoped_ptr<EntryKernel> UnpackEntry(sql::Statement* statement) {
     kernel->mutable_ref(static_cast<UniquePositionField>(i)) =
         UniquePosition::FromProto(proto);
   }
-  for (; i < ATTACHMENT_METADATA_FIELDS_END; ++i) {
-    kernel->mutable_ref(static_cast<AttachmentMetadataField>(i)).ParseFromArray(
-        statement->ColumnBlob(i), statement->ColumnByteLength(i));
-  }
+  UnpackProtoFields<sync_pb::AttachmentMetadata, AttachmentMetadataField>(
+      statement, kernel.get(), &i, ATTACHMENT_METADATA_FIELDS_END);
 
   // Sanity check on positions.  We risk strange and rare crashes if our
   // assumptions about unique position values are broken.

sync/syncable/directory_unittest.cc

@@ -1768,18 +1768,27 @@ TEST_F(SyncableDirectoryTest, MutableEntry_UpdateAttachmentId) {
   entry.PutId(TestIdFactory::FromNumber(-1));
   entry.PutAttachmentMetadata(attachment_metadata);
 
-  const sync_pb::AttachmentMetadata& entry_metadata =
-      entry.GetAttachmentMetadata();
-  ASSERT_EQ(2, entry_metadata.record_size());
-  ASSERT_FALSE(entry_metadata.record(0).is_on_server());
-  ASSERT_FALSE(entry_metadata.record(1).is_on_server());
-  ASSERT_FALSE(entry.GetIsUnsynced());
+  {
+    const sync_pb::AttachmentMetadata& entry_metadata =
+        entry.GetAttachmentMetadata();
+    ASSERT_EQ(2, entry_metadata.record_size());
+    ASSERT_FALSE(entry_metadata.record(0).is_on_server());
+    ASSERT_FALSE(entry_metadata.record(1).is_on_server());
+    ASSERT_FALSE(entry.GetIsUnsynced());
+  }
 
   entry.MarkAttachmentAsOnServer(attachment_id_proto);
 
-  ASSERT_TRUE(entry_metadata.record(0).is_on_server());
-  ASSERT_FALSE(entry_metadata.record(1).is_on_server());
-  ASSERT_TRUE(entry.GetIsUnsynced());
+  {
+    // Re-get entry_metadata because it is immutable in the directory and
+    // entry_metadata reference has been made invalid by
+    // MarkAttachmentAsOnServer call above.
+    const sync_pb::AttachmentMetadata& entry_metadata =
+        entry.GetAttachmentMetadata();
+    ASSERT_TRUE(entry_metadata.record(0).is_on_server());
+    ASSERT_FALSE(entry_metadata.record(1).is_on_server());
+    ASSERT_TRUE(entry.GetIsUnsynced());
+  }
 }
 
 // Verify that deleted entries with attachments will retain the attachments.
@@ -2043,6 +2052,60 @@ TEST_F(SyncableDirectoryTest, CatastrophicError) {
   ASSERT_EQ(2, unrecoverable_error_handler.invocation_count());
 }
 
+bool EntitySpecificsValuesAreSame(const sync_pb::EntitySpecifics& v1,
+                                  const sync_pb::EntitySpecifics& v2) {
+  return &v1 == &v2;
+}
+
+// Verifies that server and client specifics are shared when their values
+// are equal.
+TEST_F(SyncableDirectoryTest, SharingOfClientAndServerSpecifics) {
+  sync_pb::EntitySpecifics specifics1;
+  sync_pb::EntitySpecifics specifics2;
+  sync_pb::EntitySpecifics specifics3;
+  AddDefaultFieldValue(BOOKMARKS, &specifics1);
+  AddDefaultFieldValue(BOOKMARKS, &specifics2);
+  AddDefaultFieldValue(BOOKMARKS, &specifics3);
+  specifics1.mutable_bookmark()->set_url("foo");
+  specifics2.mutable_bookmark()->set_url("bar");
+  // specifics3 has the same URL as specifics1
+  specifics3.mutable_bookmark()->set_url("foo");
+
+  WriteTransaction trans(FROM_HERE, UNITTEST, dir().get());
+  MutableEntry item(&trans, CREATE, BOOKMARKS, trans.root_id(), "item");
+  item.PutId(TestIdFactory::FromNumber(1));
+  item.PutBaseVersion(10);
+
+  // Verify sharing.
+  item.PutSpecifics(specifics1);
+  item.PutServerSpecifics(specifics1);
+  EXPECT_TRUE(EntitySpecificsValuesAreSame(item.GetSpecifics(),
+                                           item.GetServerSpecifics()));
+
+  // Verify that specifics are no longer shared.
+  item.PutServerSpecifics(specifics2);
+  EXPECT_FALSE(EntitySpecificsValuesAreSame(item.GetSpecifics(),
+                                            item.GetServerSpecifics()));
+
+  // Verify that specifics are shared again because specifics3 matches
+  // specifics1.
+  item.PutServerSpecifics(specifics3);
+  EXPECT_TRUE(EntitySpecificsValuesAreSame(item.GetSpecifics(),
+                                           item.GetServerSpecifics()));
+
+  // Verify that copying the same value back to SPECIFICS is still OK.
+  item.PutSpecifics(specifics3);
+  EXPECT_TRUE(EntitySpecificsValuesAreSame(item.GetSpecifics(),
+                                           item.GetServerSpecifics()));
+
+  // Verify sharing with BASE_SERVER_SPECIFICS.
+  EXPECT_FALSE(EntitySpecificsValuesAreSame(item.GetServerSpecifics(),
+                                            item.GetBaseServerSpecifics()));
+  item.PutBaseServerSpecifics(specifics3);
+  EXPECT_TRUE(EntitySpecificsValuesAreSame(item.GetServerSpecifics(),
+                                           item.GetBaseServerSpecifics()));
+}
+
 }  // namespace syncable
 
 }  // namespace syncer

sync/syncable/entry_kernel.cc

@@ -14,7 +14,7 @@ namespace syncable {
 
 EntryKernel::EntryKernel() : dirty_(false) {
   // Everything else should already be default-initialized.
-  for (int i = INT64_FIELDS_BEGIN; i < INT64_FIELDS_END; ++i) {
+  for (int i = 0; i < INT64_FIELDS_COUNT; ++i) {
     int64_fields[i] = 0;
   }
 }

sync/syncable/entry_kernel.h

@@ -13,9 +13,8 @@
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/internal_api/public/base/unique_position.h"
 #include "sync/internal_api/public/util/immutable.h"
-#include "sync/protocol/attachments.pb.h"
-#include "sync/protocol/sync.pb.h"
 #include "sync/syncable/metahandle_set.h"
+#include "sync/syncable/proto_value_ptr.h"
 #include "sync/syncable/syncable_id.h"
 #include "sync/util/time.h"
 
@@ -197,12 +196,12 @@ enum {
 struct SYNC_EXPORT_PRIVATE EntryKernel {
  private:
   std::string string_fields[STRING_FIELDS_COUNT];
-  sync_pb::EntitySpecifics specifics_fields[PROTO_FIELDS_COUNT];
+  EntitySpecificsPtr specifics_fields[PROTO_FIELDS_COUNT];
   int64 int64_fields[INT64_FIELDS_COUNT];
   base::Time time_fields[TIME_FIELDS_COUNT];
   Id id_fields[ID_FIELDS_COUNT];
   UniquePosition unique_position_fields[UNIQUE_POSITION_FIELDS_COUNT];
-  sync_pb::AttachmentMetadata
+  AttachmentMetadataPtr
       attachment_metadata_fields[ATTACHMENT_METADATA_FIELDS_COUNT];
   std::bitset<BIT_FIELDS_COUNT> bit_fields;
   std::bitset<BIT_TEMPS_COUNT> bit_temps;
@@ -269,15 +268,15 @@ struct SYNC_EXPORT_PRIVATE EntryKernel {
     string_fields[field - STRING_FIELDS_BEGIN] = value;
   }
   inline void put(ProtoField field, const sync_pb::EntitySpecifics& value) {
-    specifics_fields[field - PROTO_FIELDS_BEGIN].CopyFrom(value);
+    specifics_fields[field - PROTO_FIELDS_BEGIN].set_value(value);
   }
   inline void put(UniquePositionField field, const UniquePosition& value) {
     unique_position_fields[field - UNIQUE_POSITION_FIELDS_BEGIN] = value;
   }
   inline void put(AttachmentMetadataField field,
                   const sync_pb::AttachmentMetadata& value) {
-    attachment_metadata_fields[field - ATTACHMENT_METADATA_FIELDS_BEGIN] =
-        value;
+    attachment_metadata_fields[field - ATTACHMENT_METADATA_FIELDS_BEGIN]
+        .set_value(value);
   }
   inline void put(BitTemp field, bool value) {
     bit_temps[field - BIT_TEMPS_BEGIN] = value;
@@ -312,14 +311,15 @@ struct SYNC_EXPORT_PRIVATE EntryKernel {
     return string_fields[field - STRING_FIELDS_BEGIN];
   }
   inline const sync_pb::EntitySpecifics& ref(ProtoField field) const {
-    return specifics_fields[field - PROTO_FIELDS_BEGIN];
+    return specifics_fields[field - PROTO_FIELDS_BEGIN].value();
   }
   inline const UniquePosition& ref(UniquePositionField field) const {
     return unique_position_fields[field - UNIQUE_POSITION_FIELDS_BEGIN];
   }
   inline const sync_pb::AttachmentMetadata& ref(
       AttachmentMetadataField field) const {
-    return attachment_metadata_fields[field - ATTACHMENT_METADATA_FIELDS_BEGIN];
+    return attachment_metadata_fields[field - ATTACHMENT_METADATA_FIELDS_BEGIN]
+        .value();
   }
   inline bool ref(BitTemp field) const {
     return bit_temps[field - BIT_TEMPS_BEGIN];
@@ -329,18 +329,24 @@ struct SYNC_EXPORT_PRIVATE EntryKernel {
   inline std::string& mutable_ref(StringField field) {
     return string_fields[field - STRING_FIELDS_BEGIN];
   }
-  inline sync_pb::EntitySpecifics& mutable_ref(ProtoField field) {
-    return specifics_fields[field - PROTO_FIELDS_BEGIN];
-  }
   inline Id& mutable_ref(IdField field) {
     return id_fields[field - ID_FIELDS_BEGIN];
   }
   inline UniquePosition& mutable_ref(UniquePositionField field) {
     return unique_position_fields[field - UNIQUE_POSITION_FIELDS_BEGIN];
   }
-  inline sync_pb::AttachmentMetadata& mutable_ref(
-      AttachmentMetadataField field) {
-    return attachment_metadata_fields[field - ATTACHMENT_METADATA_FIELDS_BEGIN];
+
+  // Sharing data methods for ::google::protobuf::MessageLite derived types.
+  inline void copy(ProtoField src, ProtoField dest) {
+    DCHECK_NE(src, dest);
+    specifics_fields[dest - PROTO_FIELDS_BEGIN] =
+        specifics_fields[src - PROTO_FIELDS_BEGIN];
+  }
+
+  inline void copy(AttachmentMetadataField src, AttachmentMetadataField dest) {
+    DCHECK_NE(src, dest);
+    attachment_metadata_fields[dest - ATTACHMENT_METADATA_FIELDS_BEGIN] =
+        attachment_metadata_fields[src - ATTACHMENT_METADATA_FIELDS_BEGIN];
   }
 
   ModelType GetModelType() const;

sync/syncable/entry_kernel_unittest.cc

@@ -26,6 +26,61 @@ TEST_F(EntryKernelTest, ToValue) {
   }
 }
 
+bool ProtoFieldValuesEqual(const sync_pb::EntitySpecifics& v1,
+                           const sync_pb::EntitySpecifics& v2) {
+  return v1.SerializeAsString() == v2.SerializeAsString();
+}
+
+bool ProtoFieldValuesAreSame(const sync_pb::EntitySpecifics& v1,
+                             const sync_pb::EntitySpecifics& v2) {
+  return &v1 == &v2;
+}
+
+bool ProtoFieldValueIsDefault(const sync_pb::EntitySpecifics& v) {
+  return ProtoFieldValuesAreSame(v,
+                                 sync_pb::EntitySpecifics::default_instance());
+}
+
+// Tests default value, assignment, and sharing of proto fields.
+TEST_F(EntryKernelTest, ProtoFieldTest) {
+  EntryKernel kernel;
+
+  // Check default values.
+  EXPECT_TRUE(ProtoFieldValueIsDefault(kernel.ref(SPECIFICS)));
+  EXPECT_TRUE(ProtoFieldValueIsDefault(kernel.ref(SERVER_SPECIFICS)));
+  EXPECT_TRUE(ProtoFieldValueIsDefault(kernel.ref(BASE_SERVER_SPECIFICS)));
+
+  sync_pb::EntitySpecifics specifics;
+
+  // Assign empty value and verify that the field still returns the
+  // default value.
+  kernel.put(SPECIFICS, specifics);
+  EXPECT_TRUE(ProtoFieldValueIsDefault(kernel.ref(SPECIFICS)));
+  EXPECT_TRUE(ProtoFieldValuesEqual(kernel.ref(SPECIFICS), specifics));
+
+  // Verifies that the kernel holds the copy of the value assigned to it.
+  specifics.mutable_bookmark()->set_url("http://demo/");
+  EXPECT_FALSE(ProtoFieldValuesEqual(kernel.ref(SPECIFICS), specifics));
+
+  // Put the new value and verify the equality.
+  kernel.put(SPECIFICS, specifics);
+  EXPECT_TRUE(ProtoFieldValuesEqual(kernel.ref(SPECIFICS), specifics));
+  EXPECT_FALSE(ProtoFieldValueIsDefault(kernel.ref(SPECIFICS)));
+
+  // Copy the value between the fields and verify that exactly the same
+  // underlying value is shared.
+  kernel.copy(SPECIFICS, SERVER_SPECIFICS);
+  EXPECT_TRUE(ProtoFieldValuesEqual(kernel.ref(SERVER_SPECIFICS), specifics));
+  EXPECT_TRUE(ProtoFieldValuesAreSame(kernel.ref(SPECIFICS),
+                                      kernel.ref(SERVER_SPECIFICS)));
+
+  // Put the new value into SPECIFICS and verify that that stops sharing even
+  // though the values are still equal.
+  kernel.put(SPECIFICS, specifics);
+  EXPECT_FALSE(ProtoFieldValuesAreSame(kernel.ref(SPECIFICS),
+                                       kernel.ref(SERVER_SPECIFICS)));
+}
+
 }  // namespace syncable
 
 }  // namespace syncer