tracing v2: Add ScatteredStreamWriter

components/tracing/core/scattered_stream_writer.cc

Index: components/tracing/core/scattered_stream_writer.cc
diff --git a/components/tracing/core/scattered_stream_writer.cc b/components/tracing/core/scattered_stream_writer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..acd0e512701aad3d889edafc9a009c3e6ac31e73
--- /dev/null
+++ b/components/tracing/core/scattered_stream_writer.cc
@@ -0,0 +1,88 @@
+// 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/scattered_stream_writer.h"
+
+#include <string.h>
+
+#include "base/logging.h"
+
+namespace tracing {
+namespace v2 {
+
+ScatteredStreamWriter::ScatteredStreamWriter(Delegate* delegate)
+    : delegate_(delegate),
+      cur_range_({nullptr, nullptr}),
+      write_ptr_(nullptr) {}
+
+ScatteredStreamWriter::~ScatteredStreamWriter() {}
+
+void ScatteredStreamWriter::Reset(
+    ScatteredStreamWriter::ContiguousMemoryRange range) {
+  cur_range_ = range;
+  write_ptr_ = range.begin;
+  DCHECK_LT(write_ptr_, cur_range_.end);
+}
+
+void ScatteredStreamWriter::Expand() {

[oystein (OOO til 10th of July), 2016/06/15 08:35:09]

Maybe Extend() instead? Or Advance() to signal that it's not actually getting
bigger.

[Primiano Tucci (use gerrit), 2016/06/29 09:32:02]

Right. Expand is the worst word given that nothing is shrinking/expanding.

+  Reset(delegate_->GetNewContiguousMemoryBuffer());
+}
+
+void ScatteredStreamWriter::WriteByte(uint8_t value) {
+  if (write_ptr_ >= cur_range_.end)
+    Expand();
+  *write_ptr_++ = value;
+}
+
+void ScatteredStreamWriter::WriteBytes(const uint8_t* src, size_t size) {
+  uint8_t* const end = const_cast<uint8_t*>(write_ptr_) + size;
+  if (end <= cur_range_.end) {
+    // Fast-path, the buffer fits into the current contiguous range.
+    // TODO(primiano): there is some potential perf optimization here, this is
+    // a tracing hot path. Looks like this is faster than memcpy for small

[oystein (OOO til 10th of July), 2016/06/15 08:35:09]

Huh interesting (and surprising).  Any ideas what the threshold is? It'd be
interesting to see overloads for constant sizes, if we'll mainly be dealing with
a few specific ones, so the loop could be unrolled.

[alph, 2016/06/15 10:08:22]

Compiler should be able to handle this optimization.
Back from my compiler dev days, I remember there were dozens of memcpy
expansions paths for various sizes and CPUs.

Having size to be a template argument might help if there are cases when the
size is known at compile time.

[Primiano Tucci (use gerrit), 2016/06/29 09:32:02]

Ok let's do something, I want to look with a fresh mind to this. Let's use
memcpy for the moment and leave the todo for the perf optimization.
I agree with alph, I think that the template will help here. I think we just
need the template for size 2 and 4 that will be the more common, and for the
rest we can use this version.

EDIT: looks like templating the size is the key. If I pass a constexpr arg to
memcpy it becomes 2x faster on Mac (even w.r.t the home brewed loop).
I think what's happening is that the generic version of memcpy has too many
branches to figure out the smart path which cost outweight a simple loop.

+    // sizes. Should probably split this into WriteFewBytes() (for varints)
+    // and WriteManyBytes() (for strings).
+    for (size_t i = 0; i < size; ++i)
+      write_ptr_[i] = src[i];
+    write_ptr_ = end;
+    return;
+  } else {
+    // Slow path, scatter the writes.
+    size_t bytes_left = size;
+    while (bytes_left > 0) {
+      if (write_ptr_ >= cur_range_.end)
+        Expand();
+      const size_t burst_size = std::min(bytes_available(), bytes_left);
+      WriteBytes(src, burst_size);
+      bytes_left -= burst_size;
+      src += burst_size;
+    }
+  }
+}
+
+// TODO(primiano): perf optimization: I suspect that at the end this will always
+// be called with size == 4, in which case we might jsut hardcode it. Ditto for
+// WriteReservedBytes() below.
+ScatteredStreamWriter::ContiguousMemoryRange
+ScatteredStreamWriter::ReserveBytes(size_t size) {
+  if (write_ptr_ + size > cur_range_.end)
+    Expand();
+  uint8_t* begin = write_ptr_;
+  write_ptr_ += size;

[alph, 2016/06/15 10:08:22]

Some bytes may be left at the end of the previous chunk. are we fine with that?

[Primiano Tucci (use gerrit), 2016/06/29 09:32:02]

Yes, so, are you talking about the fact that the last bytes of a chunk might be
left uninitialized, right? It is intentional, reason why the chunk has the
set_used_size() which writes the payload size in the header.
We can ever guarantee that a chunk is fully packed. For instance we might be in
the state where there are 2 bytes left and we want to add a new event. But a new
event requires at least the 1 byte proto preamble (to tell the type) plus the
size, plus 2 bytes to tell that the event is not complete.

+#ifndef NDEBUG
+  memset(begin, '\x42', size);
+#endif
+  return {begin, begin + size};
+}
+
+void ScatteredStreamWriter::WriteReservedBytes(
+    const ScatteredStreamWriter::ContiguousMemoryRange& reserved_range,
+    const uint8_t* src) {
+  DCHECK_EQ(4, reserved_range.end - reserved_range.begin);
+  for(uint8_t* dst = reserved_range.begin; dst < reserved_range.end;)
+    *dst++ = *src++;
+}
+
+}  // namespace v2
+}  // namespace tracing