tracing v2: Add ScatteredStreamWriter

components/tracing/core/scattered_stream_writer.h

Index: components/tracing/core/scattered_stream_writer.h
diff --git a/components/tracing/core/scattered_stream_writer.h b/components/tracing/core/scattered_stream_writer.h
new file mode 100644
index 0000000000000000000000000000000000000000..a2a00352dfa9fa839da110562ea184079f787f4e
--- /dev/null
+++ b/components/tracing/core/scattered_stream_writer.h
@@ -0,0 +1,74 @@
+// 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 COMPONENTS_TRACING_CORE_SCATTERED_STREAM_WRITER_H_
+#define COMPONENTS_TRACING_CORE_SCATTERED_STREAM_WRITER_H_
+
+#include <stdint.h>
+
+#include "base/macros.h"
+#include "components/tracing/tracing_export.h"
+
+namespace tracing {
+namespace v2 {
+
+struct ContiguousMemoryRange {
+  uint8_t* begin;
+  uint8_t* end;  // STL style: one byte past the end of the buffer.
+};
+
+// This class deals with the following problem: append-only proto messages want
+// to write a stream of bytes, without caring about the implementation of the
+// underlying buffer (which concretely will be either the trace ring buffer
+// or a heap-allocated buffer). The main deal is: proto messages don't know in
+// advance what the size of the message will be.

[petrcermak, 2016/06/29 10:27:48]

nit: The subject of this sentence is "proto messages", so this should probably
be "what THEIR size will be".

[Primiano Tucci (use gerrit), 2016/06/30 12:17:34]

Done.

+// Due to the tracing buffer being split into fixed-size chunks, in some

[petrcermak, 2016/06/29 10:27:48]

nit: I think that the correct wording is "ON ... occasions"

[Primiano Tucci (use gerrit), 2016/06/30 12:17:34]

but.. there are 6 ""in some occasions" vs 1 "on some occasions" in codesearch :)
(a search on google though shows a majority of "on"). Ok ok

+// occasions these writes need to be spread over two (or more) non contiguous

[petrcermak, 2016/06/29 10:27:48]

nit: s/non contiguous/non-contiguous/

[petrcermak, 2016/06/29 10:27:48]

nit: I think there should be a comma after "occasions".

[Primiano Tucci (use gerrit), 2016/06/30 12:17:34]

Done.

[Primiano Tucci (use gerrit), 2016/06/30 12:17:34]

Done.

+// chunks of memory. Similarly, when the buffer is backed by the heap, we want
+// to avoid realloc() calls, as they can cause an expensive memcpy to extend.

[petrcermak, 2016/06/29 10:27:48]

FYI, I have no idea what it means when "an expensive memcpy extends". Maybe be a
little more explanatory?

[Primiano Tucci (use gerrit), 2016/06/30 12:17:34]

Done.

+// The purpose of this class is to abtract away the non-contiguous write logic.
+// This class knows how to deal with writes as long as they fall in the same
+// ContiguousMemoryRange, and defers the chunk-chaining logic to the Delegate.

[petrcermak, 2016/06/29 10:27:48]

nit: there shouldn't be a comma here

[Primiano Tucci (use gerrit), 2016/06/30 12:17:34]

Done.

+class TRACING_EXPORT ScatteredStreamWriter {
+ public:
+  class Delegate {
+   public:
+    virtual ContiguousMemoryRange GetNewBuffer() = 0;
+  };
+
+  explicit ScatteredStreamWriter(Delegate* delegate);
+  ~ScatteredStreamWriter();
+
+  void WriteByte(uint8_t value);
+  void WriteBytes(const uint8_t* src, size_t size);
+
+  // Reserves a fixed number of bytes, to be backfilled later.

[petrcermak, 2016/06/29 10:27:48]

nit: the comma here is confusing since it incorrectly implies that "to be
backfilled later" does not rely to "bytes"

[Primiano Tucci (use gerrit), 2016/06/30 12:17:34]

Ups. Yeah I think it's a refuse from an older longer comment.

+  ContiguousMemoryRange ReserveBytes(size_t size);

[petrcermak, 2016/06/29 10:27:49]

Even though it's technically already in the type name, I think that the method
should be called "ReserveContiguousBytes" (and probably also mention it in the
comment).

[Primiano Tucci (use gerrit), 2016/06/30 12:17:34]

It's funny because the previous name was ReserveContiguous bytes but then I
thought that it was redundant as the return value makes it clear. Adding just a
comment.

+
+  // Resets the buffer boundaries and the write pointer to the given |range|.
+  // Subsequent WriteByte(s) will write into |range|.
+  void Reset(ContiguousMemoryRange range);
+
+  // Number of contiguous free bytes in |cur_range_| that can be written without
+  // requesting a new buffer.
+  size_t bytes_available() const {
+    return static_cast<size_t>(cur_range_.end - write_ptr_);
+  }
+
+  uint8_t* write_ptr() const { return write_ptr_; }
+
+ private:
+  void Extend();
+
+  Delegate* const delegate_;
+  ContiguousMemoryRange cur_range_;
+  uint8_t* write_ptr_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScatteredStreamWriter);
+};
+
+}  // namespace v2
+}  // namespace tracing
+
+#endif  // COMPONENTS_TRACING_CORE_SCATTERED_STREAM_WRITER_H_