TraceProcessor: introduce RefCounted<T>, remove std::shared_ptr
1. There is already a case of manually-handled refcounting
in TraceBlob / TraceBlobView.
2. PacketSequenceStateGeneration was using std::shared_ptr.
That introduce unnecessary atomics because it's thread-safe
(which we don't need and it's a fast-path).
Bug: 205302474
Change-Id: Ia5fd6e9b0389a570e55474c4d81f588c7dc5cba1
diff --git a/Android.bp b/Android.bp
index 780930a..845d0c4 100644
--- a/Android.bp
+++ b/Android.bp
@@ -8304,6 +8304,7 @@
"src/trace_processor/importers/proto/proto_trace_parser_unittest.cc",
"src/trace_processor/importers/syscalls/syscall_tracker_unittest.cc",
"src/trace_processor/importers/systrace/systrace_parser_unittest.cc",
+ "src/trace_processor/ref_counted_unittest.cc",
"src/trace_processor/trace_sorter_unittest.cc",
],
}
diff --git a/BUILD b/BUILD
index 6d764f6..a5ef32c 100644
--- a/BUILD
+++ b/BUILD
@@ -527,6 +527,7 @@
srcs = [
"include/perfetto/trace_processor/iterator.h",
"include/perfetto/trace_processor/read_trace.h",
+ "include/perfetto/trace_processor/ref_counted.h",
"include/perfetto/trace_processor/trace_processor.h",
],
)
diff --git a/include/perfetto/trace_processor/BUILD.gn b/include/perfetto/trace_processor/BUILD.gn
index cdd8b6c..e6e0131 100644
--- a/include/perfetto/trace_processor/BUILD.gn
+++ b/include/perfetto/trace_processor/BUILD.gn
@@ -16,6 +16,7 @@
sources = [
"iterator.h",
"read_trace.h",
+ "ref_counted.h",
"trace_processor.h",
]
public_deps = [
diff --git a/include/perfetto/trace_processor/ref_counted.h b/include/perfetto/trace_processor/ref_counted.h
new file mode 100644
index 0000000..4eb27c9
--- /dev/null
+++ b/include/perfetto/trace_processor/ref_counted.h
@@ -0,0 +1,135 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef INCLUDE_PERFETTO_TRACE_PROCESSOR_REF_COUNTED_H_
+#define INCLUDE_PERFETTO_TRACE_PROCESSOR_REF_COUNTED_H_
+
+#include <stddef.h>
+#include <stdint.h>
+
+#include <memory>
+
+#include "perfetto/base/logging.h"
+
+// A non-thread-safe refcounted implementation.
+// Unlike std::shared_ptr The target class needs to explicitly derive
+// RefCounted.
+
+// Usage:
+// class MyRefcountedThing : public RefCounted {};
+// ...
+// RefPtr<MyRefcountedThing> shareable_ptr(new MyRefcountedThing());
+// auto copy = shareable_ptr;
+
+namespace perfetto {
+namespace trace_processor {
+
+// The base class that refcounted classes should inherit.
+class RefCounted {
+ public:
+ RefCounted() = default;
+ RefCounted(RefCounted&&) noexcept = default;
+ RefCounted& operator=(RefCounted&&) noexcept = default;
+ RefCounted(const RefCounted&) = delete;
+ RefCounted& operator=(const RefCounted&) = delete;
+
+ private:
+ template <typename T>
+ friend class RefPtr;
+
+ void AddRef() const {
+ PERFETTO_DCHECK(refcount_ >= 0);
+ ++refcount_;
+ }
+ bool Release() const {
+ PERFETTO_DCHECK(refcount_ > 0);
+ return --refcount_ == 0;
+ }
+
+ mutable intptr_t refcount_ = 0;
+};
+
+// The RAII smart-pointer.
+template <typename T>
+class RefPtr {
+ public:
+ static_assert(std::is_base_of<RefCounted, T>::value,
+ "T must be a descendant of RefCounted");
+
+ // Adopt a newly created object.
+ RefPtr() : ptr_(nullptr) {}
+ explicit RefPtr(T* ptr) : ptr_(ptr) {
+ if (ptr_)
+ ptr_->AddRef();
+ }
+
+ ~RefPtr() { reset(); }
+
+ void reset() {
+ auto* old_ptr = ptr_;
+ ptr_ = nullptr;
+ if (old_ptr && old_ptr->Release())
+ delete old_ptr;
+ }
+
+ // This case is really the move-assignment operator=(&&).
+ void reset(T* new_obj) { *this = RefPtr<T>(new_obj); }
+
+ // Move operators.
+ RefPtr(RefPtr&& move_from) noexcept {
+ ptr_ = move_from.ptr_;
+ move_from.ptr_ = nullptr;
+ }
+
+ RefPtr& operator=(RefPtr&& move_from) noexcept {
+ this->~RefPtr();
+ new (this) RefPtr(std::move(move_from));
+ return *this;
+ }
+
+ // Copy operators.
+ RefPtr(const RefPtr& copy_from) {
+ ptr_ = copy_from.ptr_;
+ if (ptr_)
+ ptr_->AddRef();
+ }
+
+ RefPtr& operator=(const RefPtr& copy_from) {
+ if (this != ©_from) {
+ this->~RefPtr();
+ new (this) RefPtr(copy_from);
+ }
+ return *this;
+ }
+
+ template <typename U>
+ bool operator==(const RefPtr<U>& rhs) const {
+ return ptr_ == rhs.ptr_;
+ }
+
+ T* get() const { return ptr_; }
+ T* operator->() const { return ptr_; }
+ T& operator*() const { return *ptr_; }
+ explicit operator bool() const { return ptr_ != nullptr; }
+
+ private:
+ T* ptr_ = nullptr;
+};
+
+} // namespace trace_processor
+} // namespace perfetto
+
+#endif // INCLUDE_PERFETTO_TRACE_PROCESSOR_REF_COUNTED_H_
diff --git a/include/perfetto/trace_processor/trace_blob.h b/include/perfetto/trace_processor/trace_blob.h
index 7c2626f..108bb7a 100644
--- a/include/perfetto/trace_processor/trace_blob.h
+++ b/include/perfetto/trace_processor/trace_blob.h
@@ -26,6 +26,7 @@
#include "perfetto/base/build_config.h"
#include "perfetto/base/export.h"
#include "perfetto/base/logging.h"
+#include "perfetto/trace_processor/ref_counted.h"
// TODO(primiano): implement file mmap on Windows.
#if PERFETTO_BUILDFLAG(PERFETTO_OS_LINUX) || \
@@ -52,7 +53,7 @@
// sub-offsets of) the same TraceBlob.
// The neat thing about TraceBlob is that it deals transparently with owned
// memory (in the case of Allocate and TakeOwnership) and memory-mapped memory.
-class PERFETTO_EXPORT TraceBlob {
+class PERFETTO_EXPORT TraceBlob : public RefCounted {
public:
static TraceBlob Allocate(size_t size);
static TraceBlob CopyFrom(const void*, size_t size);
@@ -74,30 +75,12 @@
uint8_t* data() const { return data_; }
size_t size() const { return size_; }
- protected:
- // "protected" here is not for inheritance. It's just to document the scope of
- // the methods exposed to TraceBlobView.
- friend class TraceBlobView;
-
- // Refcount inc/dec is used only by TraceBlobView.
- void IncRefcount() {
- PERFETTO_DCHECK(refcount_ >= 0);
- ++refcount_;
- }
-
- void DecRefcountAndDeleteIfZero() {
- PERFETTO_DCHECK(refcount_ > 0);
- if (--refcount_ == 0)
- delete this;
- }
-
private:
enum class Ownership { kNull = 0, kHeapBuf, kMmaped };
TraceBlob(Ownership ownership, uint8_t* data, size_t size)
: ownership_(ownership), data_(data), size_(size) {}
- int refcount_ = 0;
Ownership ownership_ = Ownership::kNull;
uint8_t* data_ = nullptr;
size_t size_ = 0;
diff --git a/include/perfetto/trace_processor/trace_blob_view.h b/include/perfetto/trace_processor/trace_blob_view.h
index de09dee..884d3a8 100644
--- a/include/perfetto/trace_processor/trace_blob_view.h
+++ b/include/perfetto/trace_processor/trace_blob_view.h
@@ -24,6 +24,7 @@
#include <memory>
#include "perfetto/base/logging.h"
+#include "perfetto/trace_processor/ref_counted.h"
#include "perfetto/trace_processor/trace_blob.h"
namespace perfetto {
@@ -60,28 +61,21 @@
PERFETTO_DCHECK(offset + length_ <= blob.size());
length_ = static_cast<uint32_t>(length);
}
- blob_refcounted_ = new TraceBlob(std::move(blob));
- blob_refcounted_->IncRefcount();
+ blob_.reset(new TraceBlob(std::move(blob)));
}
// Trivial empty ctor.
- TraceBlobView() : data_(nullptr), length_(0), blob_refcounted_(nullptr) {}
+ TraceBlobView() : data_(nullptr), length_(0) {}
- ~TraceBlobView() {
- if (blob_refcounted_)
- blob_refcounted_->DecRefcountAndDeleteIfZero();
- }
+ ~TraceBlobView() = default;
// Allow std::move().
TraceBlobView(TraceBlobView&& other) noexcept { *this = std::move(other); }
TraceBlobView& operator=(TraceBlobView&& other) noexcept {
- // TraceBlobView moving is a hotpath. Assume we never x = std::move(x).
- PERFETTO_DCHECK(this != &other);
data_ = other.data_;
length_ = other.length_;
- blob_refcounted_ = other.blob_refcounted_;
- other.blob_refcounted_ = nullptr;
+ blob_ = std::move(other.blob_);
return *this;
}
@@ -93,21 +87,20 @@
TraceBlobView slice(const uint8_t* data, size_t length) const {
PERFETTO_DCHECK(data >= data_);
PERFETTO_DCHECK(data + length <= data_ + length_);
- return TraceBlobView(data, static_cast<uint32_t>(length), blob_refcounted_);
+ return TraceBlobView(data, static_cast<uint32_t>(length), blob_);
}
// Like slice() but takes an offset rather than a pointer as 1st argument.
TraceBlobView slice_off(size_t off, size_t length) const {
PERFETTO_DCHECK(off + length <= length_);
- return TraceBlobView(data_ + off, static_cast<uint32_t>(length),
- blob_refcounted_);
+ return TraceBlobView(data_ + off, static_cast<uint32_t>(length), blob_);
}
TraceBlobView copy() const { return slice(data_, length_); }
bool operator==(const TraceBlobView& rhs) const {
return (data_ == rhs.data_) && (length_ == rhs.length_) &&
- (blob_refcounted_ == rhs.blob_refcounted_);
+ (blob_ == rhs.blob_);
}
bool operator!=(const TraceBlobView& rhs) const { return !(*this == rhs); }
@@ -117,14 +110,12 @@
size_t size() const { return length_; }
private:
- TraceBlobView(const uint8_t* data, uint32_t length, TraceBlob* blob)
- : data_(data), length_(length), blob_refcounted_(blob) {
- blob_refcounted_->IncRefcount();
- }
+ TraceBlobView(const uint8_t* data, uint32_t length, RefPtr<TraceBlob> blob)
+ : data_(data), length_(length), blob_(std::move(blob)) {}
const uint8_t* data_ = nullptr;
uint32_t length_ = 0;
- TraceBlob* blob_refcounted_ = nullptr;
+ RefPtr<TraceBlob> blob_;
};
} // namespace trace_processor
diff --git a/src/trace_processor/BUILD.gn b/src/trace_processor/BUILD.gn
index d47e65a..02231ce 100644
--- a/src/trace_processor/BUILD.gn
+++ b/src/trace_processor/BUILD.gn
@@ -406,6 +406,7 @@
"importers/proto/proto_trace_parser_unittest.cc",
"importers/syscalls/syscall_tracker_unittest.cc",
"importers/systrace/systrace_parser_unittest.cc",
+ "ref_counted_unittest.cc",
"trace_sorter_unittest.cc",
]
deps = [
diff --git a/src/trace_processor/importers/proto/packet_sequence_state.h b/src/trace_processor/importers/proto/packet_sequence_state.h
index ce88f72..d985572 100644
--- a/src/trace_processor/importers/proto/packet_sequence_state.h
+++ b/src/trace_processor/importers/proto/packet_sequence_state.h
@@ -24,6 +24,7 @@
#include "perfetto/base/compiler.h"
#include "perfetto/protozero/proto_decoder.h"
+#include "perfetto/trace_processor/ref_counted.h"
#include "perfetto/trace_processor/trace_blob_view.h"
#include "src/trace_processor/importers/proto/stack_profile_tracker.h"
#include "src/trace_processor/storage/trace_storage.h"
@@ -43,7 +44,7 @@
class PacketSequenceState;
-class PacketSequenceStateGeneration {
+class PacketSequenceStateGeneration : public RefCounted {
public:
// Returns |nullptr| if the message with the given |iid| was not found (also
// records a stat in this case).
@@ -118,7 +119,7 @@
class PacketSequenceState {
public:
- PacketSequenceState(TraceProcessorContext* context)
+ explicit PacketSequenceState(TraceProcessorContext* context)
: context_(context), sequence_stack_profile_tracker_(context) {
current_generation_.reset(
new PacketSequenceStateGeneration(this, generation_index_++));
@@ -197,7 +198,7 @@
}
// Returns a ref-counted ptr to the current generation.
- std::shared_ptr<PacketSequenceStateGeneration> current_generation() const {
+ RefPtr<PacketSequenceStateGeneration> current_generation() const {
return current_generation_;
}
@@ -242,7 +243,7 @@
int64_t track_event_thread_timestamp_ns_ = 0;
int64_t track_event_thread_instruction_count_ = 0;
- std::shared_ptr<PacketSequenceStateGeneration> current_generation_;
+ RefPtr<PacketSequenceStateGeneration> current_generation_;
SequenceStackProfileTracker sequence_stack_profile_tracker_;
};
diff --git a/src/trace_processor/ref_counted_unittest.cc b/src/trace_processor/ref_counted_unittest.cc
new file mode 100644
index 0000000..541b49a
--- /dev/null
+++ b/src/trace_processor/ref_counted_unittest.cc
@@ -0,0 +1,161 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "perfetto/trace_processor/ref_counted.h"
+
+#include "test/gtest_and_gmock.h"
+
+namespace perfetto {
+namespace trace_processor {
+namespace {
+
+int g_instances = 0;
+
+class RObj : public RefCounted {
+ public:
+ RObj() { ++g_instances; }
+ ~RObj() { --g_instances; }
+};
+
+TEST(RefCountedTest, CreateAndReset) {
+ RefPtr<RObj> ptr;
+ EXPECT_FALSE(ptr);
+ EXPECT_EQ(ptr.get(), nullptr);
+
+ g_instances = 0;
+
+ for (int i = 0; i < 3; i++) {
+ ptr.reset(new RObj());
+ EXPECT_TRUE(ptr);
+ EXPECT_NE(ptr.get(), nullptr);
+ EXPECT_EQ(g_instances, 1);
+ }
+
+ ptr.reset();
+ EXPECT_EQ(g_instances, 0);
+ EXPECT_FALSE(ptr);
+
+ ptr.reset(new RObj());
+ ptr.reset(nullptr);
+ EXPECT_EQ(g_instances, 0);
+ EXPECT_FALSE(ptr);
+
+ // Test RAII.
+ {
+ RefPtr<RObj> ptr1(new RObj());
+ EXPECT_EQ(g_instances, 1);
+ {
+ RefPtr<RObj> ptr2(new RObj());
+ EXPECT_EQ(g_instances, 2);
+ }
+ EXPECT_EQ(g_instances, 1);
+ }
+ EXPECT_EQ(g_instances, 0);
+}
+
+TEST(RefCountedTest, CopyOperators) {
+ g_instances = 0;
+
+ RefPtr<RObj> x1(new RObj());
+ RefPtr<RObj> y1(new RObj());
+ EXPECT_EQ(g_instances, 2);
+
+ auto x2 = x1;
+ EXPECT_EQ(g_instances, 2);
+
+ auto y2 = y1;
+ EXPECT_EQ(g_instances, 2);
+
+ EXPECT_EQ(x1.get(), x2.get());
+ EXPECT_EQ(&*y1, &*y2);
+
+ x1.reset();
+ y2.reset();
+ EXPECT_EQ(g_instances, 2);
+
+ x2.reset();
+ EXPECT_EQ(g_instances, 1);
+
+ y1 = x2;
+ EXPECT_EQ(g_instances, 0);
+
+ {
+ RefPtr<RObj> nested1(new RObj());
+ EXPECT_EQ(g_instances, 1);
+ {
+ RefPtr<RObj> nested2(new RObj());
+ EXPECT_EQ(g_instances, 2);
+ nested1 = nested2;
+ EXPECT_EQ(g_instances, 1);
+ }
+ EXPECT_EQ(g_instances, 1);
+ }
+ EXPECT_EQ(g_instances, 0);
+}
+
+TEST(RefCountedTest, MoveOperators) {
+ g_instances = 0;
+
+ RefPtr<RObj> x1(new RObj());
+ RefPtr<RObj> y1(new RObj());
+ EXPECT_EQ(g_instances, 2);
+
+ auto x2 = std::move(x1);
+ EXPECT_EQ(g_instances, 2);
+ EXPECT_FALSE(x1);
+
+ auto y2 = std::move(y1);
+ EXPECT_EQ(g_instances, 2);
+ EXPECT_FALSE(y1);
+
+ // Test recycling.
+ x1 = RefPtr<RObj>(new RObj());
+ EXPECT_EQ(g_instances, 3);
+
+ // y1 is still null;
+ y2 = std::move(y1);
+ EXPECT_FALSE(y1);
+ EXPECT_FALSE(y2);
+ EXPECT_EQ(g_instances, 2); // y2 goes away.
+
+ // We are left with x1 and x2.
+ EXPECT_TRUE(x1);
+ EXPECT_TRUE(x2);
+ EXPECT_NE(&*x1, &*x2);
+
+ x1 = std::move(x2); // Now only x1 is left.
+ EXPECT_EQ(g_instances, 1);
+ EXPECT_FALSE(x2);
+
+ x1 = std::move(x2);
+ EXPECT_EQ(g_instances, 0);
+
+ {
+ RefPtr<RObj> nested1(new RObj());
+ EXPECT_EQ(g_instances, 1);
+ {
+ RefPtr<RObj> nested2(new RObj());
+ EXPECT_EQ(g_instances, 2);
+ nested1 = std::move(nested2);
+ EXPECT_EQ(g_instances, 1);
+ }
+ EXPECT_EQ(g_instances, 1);
+ }
+ EXPECT_EQ(g_instances, 0);
+}
+
+} // namespace
+} // namespace trace_processor
+} // namespace perfetto
diff --git a/src/trace_processor/timestamped_trace_piece.h b/src/trace_processor/timestamped_trace_piece.h
index a1c7112..ca61546 100644
--- a/src/trace_processor/timestamped_trace_piece.h
+++ b/src/trace_processor/timestamped_trace_piece.h
@@ -19,6 +19,7 @@
#include "perfetto/base/build_config.h"
#include "perfetto/trace_processor/basic_types.h"
+#include "perfetto/trace_processor/ref_counted.h"
#include "perfetto/trace_processor/trace_blob_view.h"
#include "src/trace_processor/importers/fuchsia/fuchsia_record.h"
#include "src/trace_processor/importers/json/json_utils.h"
@@ -55,17 +56,17 @@
struct TracePacketData {
TraceBlobView packet;
- std::shared_ptr<PacketSequenceStateGeneration> sequence_state;
+ RefPtr<PacketSequenceStateGeneration> sequence_state;
};
struct FtraceEventData {
TraceBlobView event;
- std::shared_ptr<PacketSequenceStateGeneration> sequence_state;
+ RefPtr<PacketSequenceStateGeneration> sequence_state;
};
struct TrackEventData : public TracePacketData {
TrackEventData(TraceBlobView pv,
- std::shared_ptr<PacketSequenceStateGeneration> generation)
+ RefPtr<PacketSequenceStateGeneration> generation)
: TracePacketData{std::move(pv), std::move(generation)} {}
static constexpr size_t kMaxNumExtraCounters = 8;
@@ -91,11 +92,10 @@
kSystraceLine,
};
- TimestampedTracePiece(
- int64_t ts,
- uint64_t idx,
- TraceBlobView tbv,
- std::shared_ptr<PacketSequenceStateGeneration> sequence_state)
+ TimestampedTracePiece(int64_t ts,
+ uint64_t idx,
+ TraceBlobView tbv,
+ RefPtr<PacketSequenceStateGeneration> sequence_state)
: packet_data{std::move(tbv), std::move(sequence_state)},
timestamp(ts),
packet_idx(idx),
diff --git a/src/trace_processor/trace_blob.cc b/src/trace_processor/trace_blob.cc
index db11d64..dfa9ece 100644
--- a/src/trace_processor/trace_blob.cc
+++ b/src/trace_processor/trace_blob.cc
@@ -65,7 +65,6 @@
}
TraceBlob::~TraceBlob() {
- PERFETTO_CHECK(refcount_ == 0);
switch (ownership_) {
case Ownership::kHeapBuf:
delete[] data_;
@@ -92,16 +91,15 @@
return *this;
static_assert(sizeof(*this) == base::AlignUp<sizeof(void*)>(
sizeof(data_) + sizeof(size_) +
- sizeof(ownership_) + sizeof(refcount_)),
+ sizeof(ownership_) + sizeof(RefCounted)),
"TraceBlob move operator needs updating");
data_ = other.data_;
size_ = other.size_;
ownership_ = other.ownership_;
- refcount_ = other.refcount_;
- other.refcount_ = 0;
other.data_ = nullptr;
other.size_ = 0;
other.ownership_ = Ownership::kNull;
+ RefCounted::operator=(std::move(other));
return *this;
}
