Add LwwRegister CRDT

blimp/net/helium/lww_register.h

Index: blimp/net/helium/lww_register.h
diff --git a/blimp/net/helium/lww_register.h b/blimp/net/helium/lww_register.h
new file mode 100644
index 0000000000000000000000000000000000000000..b73a093e6b08f36d847e7322db82d2ede5e4b4a9
--- /dev/null
+++ b/blimp/net/helium/lww_register.h
@@ -0,0 +1,109 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef BLIMP_NET_HELIUM_LWW_REGISTER_H_
+#define BLIMP_NET_HELIUM_LWW_REGISTER_H_
+
+#include "base/memory/ptr_util.h"
+#include "blimp/net/blimp_net_export.h"
+#include "blimp/net/helium/syncable.h"
+#include "blimp/net/helium/syncable_common.h"
+#include "blimp/net/helium/syncable_primitive_serializer.h"
+#include "blimp/net/helium/version_vector.h"
+#include "blimp/net/helium/version_vector_generator.h"
+#include "third_party/protobuf/src/google/protobuf/io/coded_stream.h"
+
+namespace blimp {
+
+// Provides a simple syncable and atomically-writable "register" holding
+// contents of type |RegisterType|. When there is a write conflict, it is
+// resolved by assuming the writer indicated by |bias| has the correct value.
+template <class RegisterType>
+class BLIMP_NET_EXPORT LwwRegister : public Syncable {
+ public:
+  LwwRegister(VersionVectorGenerator* version_gen,
+              Bias bias,
+              RunningAs running_as);
+  ~LwwRegister() = default;
+
+  void Set(const RegisterType& value);
+
+  const RegisterType& value() const;

[Kevin M, 2016/10/17 22:19:45]

Hmm. This should probably use a similar naming convention as Set. Since Set
can't be made hacker_case because its impl is >1 line, should we just make the
getter into "Get()"? Get() and Set() seem like good companion names.

Apologies for the churn.

[steimel, 2016/10/18 23:53:20]

Done.

+
+  // Syncable implementation.
+  bool ModifiedSince(const VersionVector& from) const override;
+  void CreateChangesetToCurrent(
+      const VersionVector& from,
+      google::protobuf::io::CodedOutputStream* output_stream) override;
+  void ApplyChangeset(
+      const VersionVector& from,
+      const VersionVector& to,
+      google::protobuf::io::CodedInputStream* input_stream) override;
+  void ReleaseCheckpointsBefore(const VersionVector& checkpoint) override;
+
+ private:
+  VersionVectorGenerator* version_gen_;
+  VersionVector last_modified_;
+  bool locally_owned_;
+  RegisterType value_;
+
+  DISALLOW_COPY_AND_ASSIGN(LwwRegister);
+};
+
+template <class RegisterType>
+LwwRegister<RegisterType>::LwwRegister(VersionVectorGenerator* version_gen,
+                                       Bias bias,
+                                       RunningAs running_as)
+    : version_gen_(version_gen), last_modified_(version_gen->current()) {

[Kevin M, 2016/10/17 22:19:45]

DCHECK(version_gen_)

[Kevin M, 2016/10/17 22:19:45]

value_() so that it gets properly initialized to the type-specific default? Or
should we add a check to ensure that the value can't be accessed without first
being set via Set() or ApplyChangeset()?

[steimel, 2016/10/18 23:53:20]

Done.

+  locally_owned_ =

[Kevin M, 2016/10/17 22:19:45]

nit: this could be turned into an initialization list member too, but I'm fine
with this too

[steimel, 2016/10/18 23:53:20]

I thought it might be a little complex for an initialization list member. Is
there some style-guide rule regarding this?

+      ((running_as == RunningAs::Client && bias == Bias::ClientWins) ||
+       (running_as == RunningAs::Engine && bias == Bias::EngineWins));
+}
+
+template <class RegisterType>
+void LwwRegister<RegisterType>::Set(const RegisterType& value) {
+  value_ = value;
+  version_gen_->Increment();
+  last_modified_ = last_modified_.MergeWith(version_gen_->current());
+}
+
+template <class RegisterType>
+const RegisterType& LwwRegister<RegisterType>::value() const {
+  return value_;
+}
+
+template <class RegisterType>
+bool LwwRegister<RegisterType>::ModifiedSince(const VersionVector& from) const {
+  return from.local_revision() < last_modified_.local_revision();
+}
+
+template <class RegisterType>
+void LwwRegister<RegisterType>::CreateChangesetToCurrent(
+    const VersionVector& from,
+    google::protobuf::io::CodedOutputStream* output_stream) {
+  SyncablePrimitiveSerializer::Serialize(value_, output_stream);
+}
+
+template <class RegisterType>
+void LwwRegister<RegisterType>::ApplyChangeset(
+    const VersionVector& from,
+    const VersionVector& to,
+    google::protobuf::io::CodedInputStream* input_stream) {
+  VersionVector::Comparison cmp = last_modified_.CompareTo(to);
+  if (cmp == VersionVector::Comparison::LessThan ||
+      (cmp == VersionVector::Comparison::Conflict && !locally_owned_)) {
+    SyncablePrimitiveSerializer::Deserialize(input_stream, &value_);
+  }
+  last_modified_ = last_modified_.MergeWith(to);

[scf, 2016/10/17 22:09:51]

do you need to merge in the case where you are not updating the data?

[steimel, 2016/10/18 23:53:20]

Well, if there's a conflict but we win, in my mind we should still update our
|last_modified_| to show that we're at x revision for remote. In reality it
won't affect functionality, but I see no harm in keeping it this way.

+}
+
+template <class RegisterType>
+void LwwRegister<RegisterType>::ReleaseCheckpointsBefore(
+    const VersionVector& checkpoint) {
+  // no-op
+}
+
+}  // namespace blimp
+
+#endif  // BLIMP_NET_HELIUM_LWW_REGISTER_H_