[Sync] Store the past 10 traffic records in memory.

sync/engine/traffic_recorder.cc

+// Copyright (c) 2012 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.
+
+#include "sync/engine/traffic_recorder.h"
+
+#include "base/json/json_writer.h"
+#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/values.h"
+#include "sync/protocol/proto_value_conversions.h"
+#include "sync/protocol/sync.pb.h"
+#include "sync/sessions/sync_session.h"
+
+namespace browser_sync {
+
+TrafficRecorder::TrafficRecord::TrafficRecord(const std::string& message,
+                                              TrafficMessageType message_type,
+                                              bool truncated) :
+    message(message),
+    message_type(message_type),
+    truncated(truncated) {
+}
+
+TrafficRecorder::TrafficRecord::TrafficRecord()
+    : message_type(UNKNOWN_MESSAGE_TYPE),
+      truncated(false) {
+}
+
+TrafficRecorder::TrafficRecord::~TrafficRecord() {
+}
+
+TrafficRecorder::TrafficRecorder(unsigned int max_messages,
+    unsigned int max_message_size)
+    : max_messages_(max_messages),
+      max_message_size_(max_message_size) {
+}
+
+TrafficRecorder::~TrafficRecorder() {
+}
+
+
+void TrafficRecorder::AddTrafficToQueue(const TrafficRecord& record) {
+  records_.push_back(record);
+
+  // We might have more records than our limit.
+  // Maintain the size invariant by deleting items.
+  while (records_.size() > max_messages_) {
+    records_.pop_front();
+  }
+}
+
+void TrafficRecorder::StoreProtoInQueue(
+    const ::google::protobuf::MessageLite& msg,
+    TrafficMessageType type) {
+  bool truncated = false;
+  std::string message;
+  if ((unsigned int)msg.ByteSize() >= max_message_size_) {

[akalin, 2012/03/29 18:54:06]

static_cast<unsigned int>

+    // TODO(lipalani): Trim the specifics to fit in size.
+    truncated = true;
+  } else {
+    msg.SerializeToString(&message);
+  }
+
+  TrafficRecord record(message, type, truncated);

[rlarocque, 2012/03/29 17:56:21]

Unless the compiler is really smart, this will likely be another copy.  You
could avoid this if you cad a TrafficRecord constructor that took a const
MessageLite& parameter and serialized to its member directly.

[lipalani1, 2012/03/29 18:39:29]

this is only one extra copy. We dont copy it during accessing. It will become
clear in the next CR. Also the only way to implement this would be smart
pointers. I am deferring it until a need arises.
On 2012/03/29 17:56:21, rlarocque wrote:

[akalin, 2012/03/29 18:54:06]

No, smart pointers isn't the only way.  Here's an easy way that should make
everyone happy:

Make AddTrafficToQueue look like:

void ...AddTrafficToQueue(TrafficRecord* record) {
  records_.resize(records_.size() + 1);
  std::swap(records_.back(), *record);
  ...
}

The string swap should be efficient.

On 2012/03/29 18:39:29, lipalani1 wrote:

[rlarocque, 2012/03/29 21:38:56]

That's a clever solution, but I think it requires either a template
specialization of swap<TrafficRecord> or compiler support for std::move for this
to be effective.  Otherwise the swap will be implemented as { t = a; a = b; b =
t; } with the default assignment operators.

My suggestion for this particular case was to pass the proto directly into the
constructor so it could be serialized into the TrafficRecord's member, rather
than having it serialized into the stack-allocated temporary "message" then
having the result copied into the TrafficRecord's member as that object is
constructed.  No smart pointers required.  

I suppose you could swap() the message argument into the member during
construction, but that seems like bad style.  It's not obvious to the caller
that the constructor would modify one of its arguments.  

I didn't intend to spend a lot of time on this.  My point was that there exists
an alternate way to write this code that is probably more efficient but
otherwise very similar.  It might be worth making the change if doing so doesn't
require a lot of effort and doesn't obfuscate the code too much.  

+  AddTrafficToQueue(record);
+}
+
+void TrafficRecorder::RecordClientToServerMessage(
+    const sync_pb::ClientToServerMessage& msg) {
+  StoreProtoInQueue(msg, CLIENT_TO_SERVER_MESSAGE);
+}
+
+void TrafficRecorder::RecordClientToServerResponse(
+    const sync_pb::ClientToServerResponse& response) {
+  StoreProtoInQueue(response, CLIENT_TO_SERVER_RESPONSE);
+}
+
+}  // namespace browser_sync
+