tracing v2: building blocks for reading-back trace protobufs

components/tracing/core/proto_utils.h

Index: components/tracing/core/proto_utils.h
diff --git a/components/tracing/core/proto_utils.h b/components/tracing/core/proto_utils.h
index fdadf1a18df0b98f0f269b23c02c978c659a78c0..59ab4e0ad9bf8e567000f0df964d6cba8377b3a9 100644
--- a/components/tracing/core/proto_utils.h
+++ b/components/tracing/core/proto_utils.h
@@ -10,6 +10,7 @@
 #include <type_traits>
 
 #include "base/logging.h"
+#include "components/tracing/tracing_export.h"
 
 namespace tracing {
 namespace v2 {
@@ -17,7 +18,7 @@ namespace proto {
 
 // See https://developers.google.com/protocol-buffers/docs/encoding wire types.
 
-enum : uint32_t {
+enum FieldType : uint32_t {

[oystein (OOO til 10th of July), 2016/09/08 22:16:07]

Why is this an uint32_t (and hence everywhere ParseField is called), if it's
read as an & 7?

[Primiano Tucci (use gerrit), 2016/09/09 14:15:49]

So, I was about to switch this to uint8_t after your comment but.... I then
talked with kraynov@
he convinced me that, in the cases where this is used to form back a uint32_t ,
if this is a variable it will end up requiring an extra register expansion (From
8 to 32 bi).
concretely shouldn't make any big difference with the code as it is today,
because we always use constant, so kSometing << 3 ends up with the same
instructions because the compiler knows the value of kSomehing so that becomes
an operation between register and immediate.
However, if you have
FieldType field_type = getFieldTypeFromSomewhere()
and then do
uint32_t = uint32_var << 3 || field_type

having FieldType being a uint32 is slightly more efficient (I tested the dasm)

In practice shouldn't be a big deal anyways.

   kFieldTypeVarInt = 0,
   kFieldTypeFixed64 = 1,
   kFieldTypeLengthDelimited = 2,
@@ -94,6 +95,26 @@ void StaticAssertSingleBytePreamble() {
                 "Proto field id too big to fit in a single byte preamble");
 };
 
+// Parses a VarInt from the encoded buffer [start, end). |end| is STL-style and
+// points one byte past the end of buffer.
+// The parsed int value is stored in the output arg |value|. Returns a pointer
+// to the next unconsumed byte (so start < retval <= end).
+TRACING_EXPORT const uint8_t* ParseVarInt(const uint8_t* start,
+                                          const uint8_t* end,
+                                          uint64_t* value);
+
+// Parses a protobuf field, returning its id, type and value.

[petrcermak, 2016/09/08 16:13:25]

"returning" is misleading here. The value is not returned, but assigned to the
output parameters.

[Primiano Tucci (use gerrit), 2016/09/09 14:15:49]

Done.

+// Returns a pointer to the next unconsumed byte (|start| < retval <= end), that

[petrcermak, 2016/09/08 16:13:25]

grammar nit: either there shouldn't be a comma or s/that/which/ (see
http://www.oxforddictionaries.com/us/words/clauses-american#non-restrictive_r...).

[Primiano Tucci (use gerrit), 2016/09/09 14:15:49]

Done.

+// is either the beginning of the next field or the end of the parent message.
+// In the case of a kFieldTypeLengthDelimited field |field_intvalue| will store

[petrcermak, 2016/09/08 16:13:25]

Here, on the other hand, should be a comma after "field". Otherwise, the
sentence is confusing.

[Primiano Tucci (use gerrit), 2016/09/09 14:15:49]

Done.

+// the length of the payload (either a string or a nested message). In this case

[petrcermak, 2016/09/08 16:13:25]

nit: there should be a comma after "In this case"

[Primiano Tucci (use gerrit), 2016/09/09 14:15:49]

Done.

+// the start of the payload will be at (return value) - |field_intvalue|.
+TRACING_EXPORT const uint8_t* ParseField(const uint8_t* start,
+                                         const uint8_t* end,
+                                         uint32_t* field_id,
+                                         FieldType* field_type,
+                                         uint64_t* field_intvalue);
+
 }  // namespace proto
 }  // namespace v2
 }  // namespace tracing