tracing v2: Add TraceRingBuffer class.

components/tracing/core/trace_ring_buffer.h

Index: components/tracing/core/trace_ring_buffer.h
diff --git a/components/tracing/core/trace_ring_buffer.h b/components/tracing/core/trace_ring_buffer.h
new file mode 100644
index 0000000000000000000000000000000000000000..165f4030de3420f929d87f8003ec3e17ae7b9c37
--- /dev/null
+++ b/components/tracing/core/trace_ring_buffer.h
@@ -0,0 +1,80 @@
+// 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_TRACE_RING_BUFFER_H_
+#define COMPONENTS_TRACING_CORE_TRACE_RING_BUFFER_H_
+
+#include "base/atomicops.h"
+#include "base/macros.h"
+#include "base/synchronization/lock.h"
+#include "base/threading/thread.h"
+#include "components/tracing/tracing_export.h"
+
+namespace tracing {
+namespace v2 {
+
+class TRACING_EXPORT TraceRingBuffer {
+ public:
+  class Chunk {
+   public:
+    using Header = base::subtle::Atomic32;
+    static const size_t kSize = 32768;

[petrcermak, 2016/06/28 10:22:30]

Would it make sense to define this with a bit shift (to guarantee that it's a
power of two)?

[Primiano Tucci (use gerrit), 2016/06/28 14:52:11]

Doesn't necessarily have to be a power of two IIRC. 32768 makes it easier to
tell that is 32k. Maybe better 32 * 1024, fixed. 

+
+    Chunk();
+    ~Chunk();
+
+    void Initialize(uint8_t* begin);
+    void Clear();
+
+    uint8_t* begin() const { return begin_; }
+    Header* header() const { return reinterpret_cast<Header*>(begin_); }
+    uint8_t* payload() const { return begin_ + sizeof(Header); }
+    uint8_t* end() const { return begin_ + kSize; }
+
+    void set_used_size(uint32_t size) {
+      base::subtle::NoBarrier_Store(header(), size);
+    }
+    uint32_t used_size() const {
+      return base::subtle::NoBarrier_Load(header());
+    }
+
+    bool is_owned() const { return owner_ != base::kInvalidThreadId; }
+    void clear_owner() { owner_ = base::kInvalidThreadId; }

[petrcermak, 2016/06/28 10:22:29]

perhaps call |set_owner(base::kInvalidThreadId)| ?

[Primiano Tucci (use gerrit), 2016/06/28 14:52:11]

not a huge difference really, and technically lower_methods should be only
trivial accessors.

+    void set_owner(base::PlatformThreadId tid) { owner_ = tid; }
+
+   private:
+    uint8_t* begin_;
+
+    // Accesses to owner_ must happen under the buffer |lock_|.

[petrcermak, 2016/06/28 10:22:29]

nit: "|owner_|"

[Primiano Tucci (use gerrit), 2016/06/28 14:52:11]

Done.

+    base::PlatformThreadId owner_;  // kInvalidThreadId -> Chunk is returned.

[petrcermak, 2016/06/28 10:22:29]

You mean has been returned? Is to be reused? Is *being* returned? Returned
*where*?

[Primiano Tucci (use gerrit), 2016/06/28 14:52:11]

reworded as "not owned".
It is the terminology we use today in TraceLog where threads "return" chunk to
the main buffer. But I agree that without that context it's not clear as a
cmment.

+
+    DISALLOW_COPY_AND_ASSIGN(Chunk);
+  };
+
+  TraceRingBuffer(uint8_t* begin, size_t size);
+  ~TraceRingBuffer();
+
+  Chunk* TakeChunk();
+  void ReturnChunk(Chunk* chunk, uint32_t used_size);
+
+ private:
+  base::Lock lock_;
+  std::unique_ptr<Chunk[]> chunks_;
+  const size_t num_chunks_;
+  size_t current_chunk_idx_;
+
+  // An emergency chunk used in the rare case in which all chunks are in flight.
+  // This chunk is not part of the ring buffer and its contents are always
+  // discarded. Its only purpose is to avoid a crash (due to TakeChunk returning
+  // nullptr) in the case of a thread storm.
+  Chunk bankrupcy_chunk_;
+  std::unique_ptr<uint8_t[]> bankrupcy_chunk_storage_;

[petrcermak, 2016/06/28 10:22:29]

would it make sense to rename this to |emergency_chunk_storage_| to keep the
comment and variable name in sync?

[petrcermak, 2016/06/28 10:22:30]

Just to clarify, it can be the case that multiple threads think that they own
the bankrupcy chunk storage and write over each other (so the data in it is most
likely going to be complete garbage)?

[Primiano Tucci (use gerrit), 2016/06/28 14:52:11]

Correct. If this is used this will contain complete garbage. But this is not
part of the begin-end range so will never be consumed. Is only a wait to make
writers not crash.

[Primiano Tucci (use gerrit), 2016/06/28 14:52:11]

Hmm bankrupcy gives me more the idea that you give up on something.
Emergency makes me feel as if we are still trying to make things work properly,
which is not the case here. We're just trying not to crash.

+
+  DISALLOW_COPY_AND_ASSIGN(TraceRingBuffer);
+};
+
+}  // namespace v2
+}  // namespace tracing
+
+#endif  // COMPONENTS_TRACING_CORE_TRACE_RING_BUFFER_H_