Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(978)

Unified Diff: base/debug/activity_tracker.h

Issue 1980743002: Track thread activities in order to diagnose hangs. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@readwrite-mmf
Patch Set: rebased Created 4 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: base/debug/activity_tracker.h
diff --git a/base/debug/activity_tracker.h b/base/debug/activity_tracker.h
new file mode 100644
index 0000000000000000000000000000000000000000..742cbd5edd4940b742c8bb5efb75c4d7fd8abc3f
--- /dev/null
+++ b/base/debug/activity_tracker.h
@@ -0,0 +1,228 @@
+// 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 BASE_METRICS_ACTIVITY_TRACKER_H_
+#define BASE_METRICS_ACTIVITY_TRACKER_H_
+
+#include <atomic>
+#include <memory>
+
+#include "base/base_export.h"
+#include "base/feature_list.h"
+#include "base/files/file_path.h"
+#include "base/metrics/persistent_memory_allocator.h"
+#include "base/synchronization/lock.h"
+#include "base/threading/thread_checker.h"
+#include "base/threading/thread_local_storage.h"
+
+namespace base {
+
+struct PendingTask;
+
+class Lock;
+class MemoryMappedFile;
+
+namespace debug {
Sigurður Ásgeirsson 2016/05/20 13:12:05 A general comment on thread identity here. First i
manzagop (departed) 2016/05/20 13:55:42 +1 There's also some system wide info that would
bcwhite 2016/05/20 19:19:19 My understanding is that the thread-local-storage,
bcwhite 2016/05/20 19:19:19 That's outside of the scope of this module but I'v
manzagop (departed) 2016/05/20 20:24:21 Great!
+
+// Enables the global activity tracker according to a field trial setting.
+BASE_EXPORT void SetupGlobalActivityTrackerFieldTrial();
+
+
+// This class manages tracking a stack of activities for a single thread in
+// a persistent manner. However, in order to support an operational mode where
+// another thread is analyzing this data in real-time, atomic operations are
+// used where necessary to guarantee a consistent view from the outside.
+class BASE_EXPORT ThreadActivityTracker {
Sigurður Ásgeirsson 2016/05/20 13:12:05 I assume each instance of this class has thread af
bcwhite 2016/05/20 19:19:19 Correct. A ThreadChecker ensures this.
+ public:
+ enum ActivityType : uint8_t {
+ ACT_TASK,
+ ACT_LOCK,
+ ACT_EVENT,
+ };
+
+ struct StackEntry {
Sigurður Ásgeirsson 2016/05/20 13:12:05 nit: the fields warrant a little bit of documentat
bcwhite 2016/05/20 19:19:19 Yup. Documentation is definitely unfinished. It'
+ int64_t time_ticks;
+ uint8_t activity_type;
Sigurður Ásgeirsson 2016/05/20 13:12:05 I think what we need is an easily extensible tagge
bcwhite 2016/05/20 19:19:19 I agree. That'll simplify some other places, too,
+ intptr_t source_address;
+ intptr_t method_address;
+ uint64_t sequence_id;
+ };
+
+ class BASE_EXPORT ScopedActivity {
+ public:
+ ScopedActivity(ThreadActivityTracker* tracker,
+ const void* source,
+ ActivityType activity,
+ intptr_t method,
+ uint64_t sequence)
+ : tracker_(tracker), source_(source) {
+ if (tracker_)
+ tracker_->RecordStart(source, activity, method, sequence);
+ }
+ ~ScopedActivity() {
+ if (tracker_)
+ tracker_->RecordFinish(source_);
+ }
+
+ private:
+ ThreadActivityTracker* const tracker_;
+ const void* const source_;
+ };
+
+ // A ThreadActivityTracker runs on top of memory that is managed externally.
manzagop (departed) 2016/05/20 18:19:30 Mention the precondition on the size (large enough
bcwhite 2016/05/20 19:19:19 Done.
+ ThreadActivityTracker(void* base, size_t size);
+ virtual ~ThreadActivityTracker();
+
+ // Indicate that a method of the given (arbitrary) identifier has started.
+ void RecordStart(const void* source,
Sigurður Ásgeirsson 2016/05/20 13:12:04 RecordXXX implies tracing, but here we're maintain
bcwhite 2016/05/20 19:19:19 Done.
+ ActivityType activity,
+ intptr_t method,
+ uint64_t sequence);
+
+ // Indicate that a method of the given (arbitrary) identifier has finished.
+ void RecordFinish(const void* source);
+
+ // Gets a copy of the current stack contents. The return value is the current
+ // depth of the stack which may be greater than the number of StackEntry
+ // records returned. If so, the returned stack has the "base" of the stack
+ // with later entries omitted.
+ uint32_t CopyStack(std::vector<StackEntry>* stack);
Sigurður Ásgeirsson 2016/05/20 13:12:05 is this ForTesting, or do you foresee this being a
bcwhite 2016/05/20 19:19:19 It's not for testing. It's the primary interface
+
+ // Returns whether the current data is valid or not. Fetching a copy of the
manzagop (departed) 2016/05/20 18:19:30 When can the data be not valid? Is it only for a
bcwhite 2016/05/20 19:19:19 Done.
+ // stack will return nothing if the data is not valid.
+ bool is_valid() { return valid_; }
+
+ // Calculates the memory size required for a given stack depth.
manzagop (departed) 2016/05/20 18:19:30 nit: clarify this includes the header, eg "require
bcwhite 2016/05/20 19:19:19 Done.
+ static size_t SizeForStackDepth(int stack_depth);
+
+ private:
+ friend class Iterator;
manzagop (departed) 2016/05/20 18:19:30 Is this needed?
bcwhite 2016/05/20 19:19:19 No. The iterator went away in favor of CopyStack(
+
+ struct Header;
+
+ Header* const header_;
+ StackEntry* const stack_;
+ const uint32_t slots_;
manzagop (departed) 2016/05/20 18:19:30 nit: max_stack_depth_ or stack_slots_ clearer?
bcwhite 2016/05/20 19:19:19 Done. The "depth" can actually be greater; it's j
+
+ bool valid_ = false;
+
+ base::ThreadChecker thread_checker_;
+
+ DISALLOW_COPY_AND_ASSIGN(ThreadActivityTracker);
+};
+
+class BASE_EXPORT GlobalActivityTracker {
Sigurður Ásgeirsson 2016/05/20 13:12:05 so one of these guys manages one per thread of the
bcwhite 2016/05/20 19:19:19 Correct.
+ public:
+ class BASE_EXPORT ScopedThreadActivity
+ : public ThreadActivityTracker::ScopedActivity {
+ public:
+ ScopedThreadActivity(const void* source,
+ ThreadActivityTracker::ActivityType activity,
+ intptr_t method,
+ uint64_t sequence)
+ : ThreadActivityTracker::ScopedActivity(GetOrCreateTracker(),
+ source,
+ activity,
+ method,
+ sequence) {}
+
+ private:
+ static ThreadActivityTracker* GetOrCreateTracker() {
+ GlobalActivityTracker* global_tracker = Get();
+ if (!global_tracker)
+ return nullptr;
+ return global_tracker->GetOrCreateTrackerForCurrentThread();
+ }
+ };
+
+ ~GlobalActivityTracker();
+
+ static void CreateWithAllocator(
+ std::unique_ptr<PersistentMemoryAllocator> allocator,
+ int stack_depth);
+
+ static void CreateWithLocalMemory(size_t size,
+ uint64_t id,
+ StringPiece name,
+ int stack_depth);
+
+ static void CreateWithFile(const FilePath& file_path,
+ size_t size,
+ uint64_t id,
+ StringPiece name,
+ int stack_depth);
+
+ // Gets the global activity-tracker or null if none exists.
+ static GlobalActivityTracker* Get() { return g_tracker_; }
+
+ // Gets the thread's activity-tracker, assuming it already exists. This
+ // is inline for performance reasons. Ownership remains with the global
+ // tracker.
+ ThreadActivityTracker* GetTrackerForCurrentThread() {
+ void* tracker = this_thread_tracker_.Get();
+ DCHECK(tracker);
+ return reinterpret_cast<ThreadActivityTracker*>(tracker);
+ }
+
+ // Gets the thread's activity-tracker or creates one if none exists. This
+ // is inline for performance reasons. Ownership remains with the global
+ // tracker.
+ ThreadActivityTracker* GetOrCreateTrackerForCurrentThread() {
Sigurður Ásgeirsson 2016/05/20 13:12:04 I dunno about Chromium coding guidelines, but I li
bcwhite 2016/05/20 19:19:19 I've seen it done both ways. It was defined inlin
Sigurður Ásgeirsson 2016/05/24 14:11:50 Right - I'm saying you can have your cake and eat
bcwhite 2016/05/26 15:35:39 Understood. I'm saying that I've seen Chromium co
+ void* tracker = this_thread_tracker_.Get();
+ if (tracker)
+ return reinterpret_cast<ThreadActivityTracker*>(tracker);
+ return CreateTrackerForCurrentThread();
+ }
+
+ // Creates an activity-tracker for the current thread.
+ ThreadActivityTracker* CreateTrackerForCurrentThread();
+
+ // Releases the activity-tracker for the current thread (for testing only).
+ void ReleaseTrackerForCurrentThreadForTesting();
+
+ private:
+ class ManagedActivityTracker : public ThreadActivityTracker {
+ public:
+ ManagedActivityTracker(PersistentMemoryAllocator::Reference mem_reference,
+ void* base,
+ size_t size);
+ ~ManagedActivityTracker() override;
+
+ private:
+ const PersistentMemoryAllocator::Reference mem_reference_;
+ void* const mem_base_;
+ };
+
+ GlobalActivityTracker(std::unique_ptr<PersistentMemoryAllocator> allocator,
+ int stack_depth);
+
+ // Returns the memory used by an activity-tracker managed by this class.
+ void ReturnTrackerMemory(ManagedActivityTracker* tracker,
+ PersistentMemoryAllocator::Reference mem_reference,
+ void* mem_base);
+
+ static void OnTLSDestroy(void* value);
+
+ std::unique_ptr<PersistentMemoryAllocator> allocator_;
+ const size_t stack_memory_;
manzagop (departed) 2016/05/20 20:24:22 nit: stack_memory_size_?
bcwhite 2016/05/26 15:35:39 Done.
+
+ base::ThreadLocalStorage::Slot this_thread_tracker_;
+
+ Lock lock_;
+ std::set<ManagedActivityTracker*> thread_trackers_;
+ std::vector<PersistentMemoryAllocator::Reference> available_memories_;
+
+ static GlobalActivityTracker* g_tracker_;
+};
+
+class BASE_EXPORT ScopedTaskActivity
Sigurður Ásgeirsson 2016/05/20 13:12:04 it looks like this class is the interface to the m
bcwhite 2016/05/20 19:19:19 Acknowledged.
+ : public GlobalActivityTracker::ScopedThreadActivity {
+ public:
+ ScopedTaskActivity(const base::PendingTask& task);
+};
+
+} // namespace debug
+} // namespace base
+
+#endif // BASE_METRICS_ACTIVITY_TRACKER_H_
« no previous file with comments | « base/base.gypi ('k') | base/debug/activity_tracker.cc » ('j') | base/debug/activity_tracker.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698