tracing v2: Introduce TraceBufferReader to read-back the trace buffer

components/tracing/core/trace_buffer_reader.cc

+// 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.
+
+#include "components/tracing/core/trace_buffer_reader.h"
+
+#include <ctype.h>
+
+#include <deque>
+#include <utility>
+
+#include "base/memory/ptr_util.h"
+#include "base/strings/string_piece.h"
+#include "components/tracing/core/proto_utils.h"
+#include "components/tracing/core/string_interning.h"
+#include "components/tracing/proto/event.pbzero.h"
+#include "components/tracing/proto/events_chunk.pbzero.h"
+
+namespace tracing {
+namespace v2 {
+
+TraceBufferReader::TraceBufferReader(uint32_t options) : options_(options) {
+  DCHECK((options & kBuildInternedStringsTable) ||
+         (options & kBuildLegacyJSONEvents));
+  if (options & kBuildLegacyJSONEvents)
+    legacy_events_ = base::MakeUnique<base::ListValue>();
+}
+
+TraceBufferReader::~TraceBufferReader() {}
+
+void TraceBufferReader::ParseChunk(const TraceRingBuffer::Chunk* chunk) {
+  // At the moment we support only chunks that have been returned.
+  DCHECK(!chunk->is_owned());
+  ParseChunk(chunk->payload(), chunk->used_size());
+}
+
+void TraceBufferReader::ParseChunk(const uint8_t* const start, size_t length) {
+  using ChunkProto = pbzero::tracing::proto::EventsChunk;
+  uint32_t writer_id = 0;
+  bool has_writer_id = false;
+  uint32_t seq_id = 0;
+  bool has_seq_id = false;
+  bool first_event_continues_from_prev_chunk = false;
+  bool last_event_continues_on_next_chunk = false;
+  using Range = std::pair<const uint8_t*, const uint8_t*>;  // (begin, end)
+  std::deque<Range> events;
+
+  const uint8_t* const end = start + length;
+  if (end == start)
+    return;  // Empty chunk.
+
+  const uint8_t* pos = start;
+  while (pos < end) {
+    uint32_t field_id;
+    proto::FieldType field_type;
+    uint64_t field_intvalue;
+    const uint8_t* field_payload = nullptr;
+    pos = proto::ParseField(pos, end, &field_id, &field_type, &field_intvalue);
+    if (field_type == proto::kFieldTypeLengthDelimited)
+      field_payload = pos - field_intvalue;

[oystein (OOO til 10th of July), 2016/09/09 00:27:35]

Can we do a DCHECK_LT(field_intvalue, limits::numerical_limits<uint32_t> in the
negative case here? Looks like all of the other cases static_cast it down or use
it as a flag.

[Primiano Tucci (use gerrit), 2016/09/13 14:40:25]

The reason I didn't put it is because ParseField has that sanity check in
ParseField. However I agree makes the code more legible, so I added a dcheck
here that ensures that the final payload is between the last field and the next
one.

+    DCHECK(pos);
+    switch (field_id) {
+      case ChunkProto::kWriterIdFieldNumber:
+        DCHECK_EQ(proto::kFieldTypeVarInt, field_type);
+        writer_id = static_cast<uint32_t>(field_intvalue);
+        has_writer_id = true;
+        break;
+      case ChunkProto::kSeqIdFieldNumber:
+        DCHECK_EQ(proto::kFieldTypeVarInt, field_type);
+        has_seq_id = true;
+        seq_id = static_cast<uint32_t>(field_intvalue);
+        break;
+      case ChunkProto::kFirstEventContinuesFromPrevChunkFieldNumber:
+        DCHECK_EQ(proto::kFieldTypeVarInt, field_type);
+        first_event_continues_from_prev_chunk = field_intvalue ? true : false;
+        break;
+      case ChunkProto::kLastEventContinuesOnNextChunkFieldNumber:
+        DCHECK_EQ(proto::kFieldTypeVarInt, field_type);
+        last_event_continues_on_next_chunk = field_intvalue ? true : false;
+        break;
+      case ChunkProto::kEventsFieldNumber:
+        DCHECK_EQ(proto::kFieldTypeLengthDelimited, field_type);
+        events.push_back(std::make_pair(field_payload, pos));
+        break;
+      default:
+        NOTREACHED();
+    }
+  }
+
+  DCHECK(has_seq_id);
+  DCHECK(has_writer_id);
+  WriterState* writer_state = &writer_state_[writer_id];

[oystein (OOO til 10th of July), 2016/09/09 00:27:35]

I'm finding it hard to reason about the usage of the writer_state_ entries, can
you explain it a bit? The writer state is sometimes carried over from one chunk
to the next? Or even more, in case of nested messages?

[Primiano Tucci (use gerrit), 2016/09/13 14:40:25]

Allright, I added a comment to the WriterState struct.
Essentially for each writer (read, for each thread) i have to keep track of:
- the sequencing of chunks: all I need here is to established if I saw all
chunks for a thread, or if some chunk got dropped because of the ring-buffer
wrapping.
- accumulating the bytes of fragmented events across chunks.

In essence, when I see fragments I have to merge them, but only if I saw all the
chunks for that fragment contiguously, otherwise I just have to drop the event. 

+
+  // When |first_event_continues_from_prev_chunk| is true, the first event is
+  // a fragment. The fragment will be parsed only if we saw all the previous
+  // fragments AND there are no more fragments upcoming.
+  if (first_event_continues_from_prev_chunk) {
+    DCHECK(!events.empty());
+    if (writer_state->last_fragment_is_valid &&
+        seq_id == writer_state->last_seq_id + 1) {
+      writer_state->partial_event_data.insert(
+          writer_state->partial_event_data.end(), events[0].first,
+          events[0].second);
+      writer_state->last_seq_id = seq_id;
+      if (events.size() > 1 || !last_event_continues_on_next_chunk) {
+        ParseEvent(&*writer_state->partial_event_data.begin(),
+                   &*writer_state->partial_event_data.end());
+        writer_state->Reset();
+      }
+    } else {
+      // We missed the previous chunk, dropping the first partial event. This
+      // can happen if the trace ring buffer wraps over and the previous chunk
+      // is recycled.
+      writer_state->Reset();
+    }
+    events.pop_front();
+  }
+
+  size_t event_count = 1;
+  for (const Range& event : events) {
+    if (event_count++ == events.size() && last_event_continues_on_next_chunk) {
+      writer_state->last_fragment_is_valid = true;
+      writer_state->last_seq_id = seq_id;
+      writer_state->partial_event_data.insert(

[kraynov, 2016/09/08 16:10:01]

move "insert" to new line. Hard to read.

[Primiano Tucci (use gerrit), 2016/09/13 14:40:25]

Done.

+          writer_state->partial_event_data.end(), event.first, event.second);
+      break;
+    }
+    ParseEvent(event.first, event.second);
+  }
+}
+
+void TraceBufferReader::ParseEvent(const uint8_t* const start,
+                                   const uint8_t* const end) {
+  using EventProto = pbzero::tracing::proto::Event;
+  std::unique_ptr<base::DictionaryValue> event_value;
+  if (options_ & kBuildLegacyJSONEvents)
+    event_value = base::MakeUnique<base::DictionaryValue>();
+
+  DCHECK_GT((void*)end, (void*)start);
+  const uint8_t* pos = start;
+  while (pos < end) {
+    uint32_t field_id;
+    proto::FieldType field_type;
+    uint64_t field_intvalue;
+    pos = proto::ParseField(pos, end, &field_id, &field_type, &field_intvalue);
+    base::StringPiece field_strvalue;
+    if (field_type == proto::kFieldTypeLengthDelimited) {
+      const size_t len = static_cast<size_t>(field_intvalue);
+      field_strvalue =
+          base::StringPiece(reinterpret_cast<const char*>(pos - len), len);

[oystein (OOO til 10th of July), 2016/09/09 00:27:35]

DCHECK_LT(pos, len)?

[Primiano Tucci (use gerrit), 2016/09/13 14:40:24]

Done (more strictly)

+    }
+    const char* interned_string = nullptr;
+    if (field_id == EventProto::kNameIdFieldNumber ||
+        field_id == EventProto::kCategoryIdFieldNumber) {
+      const int64_t str_id = static_cast<int64_t>(field_intvalue);
+      interned_string = GetInternedStringValue(str_id);
+      if (options_ & kBuildInternedStringsTable)
+        interned_strings_[str_id] = interned_string;
+    }
+
+    if (options_ & kBuildLegacyJSONEvents) {
+      switch (field_id) {

[oystein (OOO til 10th of July), 2016/09/09 00:27:35]

Should there be a default NOTREACHED() here?

[Primiano Tucci (use gerrit), 2016/09/13 14:40:25]

Hmm good question. So the case to cover here is some field that we want in the
proto but we ignore for the  readback -> JSON.
We might have fields lke that, but in that case I agree to stay conservative and
have them listed as an no-op case here, to make sure that if somebody adds a new
field during the interim phase we don't forget about it here.

+        case EventProto::kTypeFieldNumber: {
+          char event_phase[2] = {static_cast<char>(field_intvalue), '\0'};

[oystein (OOO til 10th of July), 2016/09/09 00:27:34]

nit: Can't you leave out the '2' here?

[Primiano Tucci (use gerrit), 2016/09/13 14:40:25]

Done.

+          DCHECK(std::isalnum(event_phase[0]));
+          event_value->SetString("ph", base::StringPiece(event_phase, 1));
+          break;
+        }
+        case EventProto::kNameStrFieldNumber:
+          // For the rare cases of TRACE_STR_COPY.
+          event_value->SetString("name", field_strvalue);
+          break;
+        case EventProto::kNameIdFieldNumber:
+          // For the most common case of event name being a const char* string.
+          event_value->SetString("name", interned_string);
+          break;
+        case EventProto::kCategoryIdFieldNumber:
+          event_value->SetString("cat", interned_string);
+          break;
+        case EventProto::kTimestampFieldNumber:
+          event_value->SetInteger("ts", static_cast<int>(field_intvalue));
+          break;
+        case EventProto::kThreadTimestampFieldNumber:
+          event_value->SetInteger("tts", static_cast<int>(field_intvalue));
+          break;
+      }
+    }
+  }
+  DCHECK_EQ(end, pos);
+  if (options_ & kBuildLegacyJSONEvents)
+    legacy_events_->Append(std::move(event_value));
+}
+
+TraceBufferReader::WriterState::WriterState() {
+  Reset();
+}
+
+TraceBufferReader::WriterState::~WriterState() {}
+
+void TraceBufferReader::WriterState::Reset() {
+  last_seq_id = 0;
+  last_fragment_is_valid = false;
+  partial_event_data.clear();
+}
+
+}  // namespace v2
+}  // namespace tracing