[vm/profiler/gardening] Fix TSAN failures in profiler.
Use relaxed atomics for sample fields. Fix lock grabbing ordering. BUG=https://github.com/dart-lang/sdk/issues/62873 TEST=ci Change-Id: I9c4ae0c78d5b81a72dc8a9e4802f08fd57beb27a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/493161 Reviewed-by: Ryan Macnak <rmacnak@google.com> Commit-Queue: Alexander Aprelev <aam@google.com>
This commit is contained in:
committed by
Commit Queue
parent
527b05718c
commit
8649806076
+11
-8
@@ -1003,7 +1003,9 @@ class ReturnAddressLocator : public ValueObject {
|
||||
ASSERT(code_.ContainsInstructionAt(pc()));
|
||||
}
|
||||
|
||||
ReturnAddressLocator(uword pc, uword* stack_buffer, const Code& code)
|
||||
ReturnAddressLocator(uword pc,
|
||||
RelaxedAtomic<uword>* stack_buffer,
|
||||
const Code& code)
|
||||
: stack_buffer_(stack_buffer),
|
||||
pc_(pc),
|
||||
code_(Code::ZoneHandle(code.ptr())) {
|
||||
@@ -1037,7 +1039,7 @@ class ReturnAddressLocator : public ValueObject {
|
||||
}
|
||||
|
||||
private:
|
||||
uword* stack_buffer_;
|
||||
RelaxedAtomic<uword>* stack_buffer_;
|
||||
uword pc_;
|
||||
const Code& code_;
|
||||
};
|
||||
@@ -1282,7 +1284,7 @@ class ProfilerDartStackWalker : public ProfilerStackWalker {
|
||||
static void CopyStackBuffer(Sample* sample, uword sp_addr) {
|
||||
ASSERT(sample != nullptr);
|
||||
uword* sp = reinterpret_cast<uword*>(sp_addr);
|
||||
uword* buffer = sample->GetStackBuffer();
|
||||
RelaxedAtomic<uword>* buffer = sample->GetStackBuffer();
|
||||
if (sp != nullptr) {
|
||||
for (intptr_t i = 0; i < Sample::kStackBufferSizeInWords; i++) {
|
||||
buffer[i] = reinterpret_cast<uword>(LoadStackSlot(sp));
|
||||
@@ -2200,7 +2202,7 @@ ProcessedSample::ProcessedSample()
|
||||
|
||||
void ProcessedSample::FixupCaller(const CodeLookupTable& clt,
|
||||
uword pc_marker,
|
||||
uword* stack_buffer) {
|
||||
RelaxedAtomic<uword>* stack_buffer) {
|
||||
const CodeDescriptor* cd = clt.FindCode(At(0));
|
||||
if (cd == nullptr) {
|
||||
// No Dart code.
|
||||
@@ -2213,10 +2215,11 @@ void ProcessedSample::FixupCaller(const CodeLookupTable& clt,
|
||||
CheckForMissingDartFrame(clt, cd, pc_marker, stack_buffer);
|
||||
}
|
||||
|
||||
void ProcessedSample::CheckForMissingDartFrame(const CodeLookupTable& clt,
|
||||
const CodeDescriptor* cd,
|
||||
uword pc_marker,
|
||||
uword* stack_buffer) {
|
||||
void ProcessedSample::CheckForMissingDartFrame(
|
||||
const CodeLookupTable& clt,
|
||||
const CodeDescriptor* cd,
|
||||
uword pc_marker,
|
||||
RelaxedAtomic<uword>* stack_buffer) {
|
||||
ASSERT(cd != nullptr);
|
||||
if (cd->code().IsBytecode()) {
|
||||
// Bytecode frame build is atomic from the profiler's perspective,
|
||||
|
||||
+14
-14
@@ -428,22 +428,22 @@ class Sample {
|
||||
}
|
||||
|
||||
static constexpr int kPCArraySizeInWords = 32;
|
||||
uword* GetPCArray() { return &pc_array_[0]; }
|
||||
RelaxedAtomic<uword>* GetPCArray() { return &pc_array_[0]; }
|
||||
|
||||
static constexpr int kStackBufferSizeInWords = 2;
|
||||
uword* GetStackBuffer() { return &stack_buffer_[0]; }
|
||||
RelaxedAtomic<uword>* GetStackBuffer() { return &stack_buffer_[0]; }
|
||||
|
||||
private:
|
||||
int64_t timestamp_;
|
||||
Dart_Port port_;
|
||||
ThreadId tid_;
|
||||
uword stack_buffer_[kStackBufferSizeInWords];
|
||||
uword pc_array_[kPCArraySizeInWords];
|
||||
uword vm_tag_;
|
||||
uword user_tag_;
|
||||
Sample* next_;
|
||||
uint32_t state_;
|
||||
uint32_t allocation_identity_hash_;
|
||||
RelaxedAtomic<int64_t> timestamp_;
|
||||
RelaxedAtomic<Dart_Port> port_;
|
||||
RelaxedAtomic<ThreadId> tid_;
|
||||
RelaxedAtomic<uword> stack_buffer_[kStackBufferSizeInWords];
|
||||
RelaxedAtomic<uword> pc_array_[kPCArraySizeInWords];
|
||||
RelaxedAtomic<uword> vm_tag_;
|
||||
RelaxedAtomic<uword> user_tag_;
|
||||
RelaxedAtomic<Sample*> next_;
|
||||
RelaxedAtomic<uint32_t> state_;
|
||||
RelaxedAtomic<uint32_t> allocation_identity_hash_;
|
||||
|
||||
using HeadSampleBit = BitField<decltype(state_), bool, 0, 1>;
|
||||
using LeafFrameIsDart =
|
||||
@@ -944,12 +944,12 @@ class ProcessedSample : public ZoneObject {
|
||||
private:
|
||||
void FixupCaller(const CodeLookupTable& clt,
|
||||
uword pc_marker,
|
||||
uword* stack_buffer);
|
||||
RelaxedAtomic<uword>* stack_buffer);
|
||||
|
||||
void CheckForMissingDartFrame(const CodeLookupTable& clt,
|
||||
const CodeDescriptor* code,
|
||||
uword pc_marker,
|
||||
uword* stack_buffer);
|
||||
RelaxedAtomic<uword>* stack_buffer);
|
||||
|
||||
ZoneGrowableArray<uword> pcs_;
|
||||
int64_t timestamp_;
|
||||
|
||||
+11
-8
@@ -377,7 +377,7 @@ void Timeline::Cleanup() {
|
||||
}
|
||||
}
|
||||
|
||||
void Timeline::ReclaimCachedBlocksFromThreads() {
|
||||
void Timeline::ReclaimCachedBlocksFromThreads(OSThreadIterator* it) {
|
||||
RecorderSynchronizationLockScope ls;
|
||||
TimelineEventRecorder* recorder = Timeline::recorder();
|
||||
if (recorder == nullptr || ls.IsUninitialized()) {
|
||||
@@ -385,9 +385,8 @@ void Timeline::ReclaimCachedBlocksFromThreads() {
|
||||
}
|
||||
ASSERT(recorder != nullptr);
|
||||
// Iterate over threads.
|
||||
OSThreadIterator it;
|
||||
while (it.HasNext()) {
|
||||
OSThread* thread = it.Next();
|
||||
while (it->HasNext()) {
|
||||
OSThread* thread = it->Next();
|
||||
MutexLocker ml(thread->timeline_block_lock());
|
||||
// Grab block and clear it.
|
||||
TimelineEventBlock* block = thread->TimelineBlockLocked();
|
||||
@@ -443,8 +442,9 @@ void Timeline::Clear() {
|
||||
ASSERT(recorder != nullptr);
|
||||
// Acquire the recorder's lock to prevent the reclaimed blocks from being
|
||||
// handed out again until they have been cleared.
|
||||
OSThreadIterator it;
|
||||
MutexLocker ml(&recorder->lock_);
|
||||
ReclaimCachedBlocksFromThreads();
|
||||
ReclaimCachedBlocksFromThreads(&it);
|
||||
recorder->ClearLocked();
|
||||
}
|
||||
|
||||
@@ -1546,8 +1546,9 @@ void TimelineEventRecorder::WriteTo(const char* directory) {
|
||||
|
||||
// Acquire the recorder's lock to prevent the reclaimed blocks from being
|
||||
// handed out again until the trace has been serialized.
|
||||
OSThreadIterator it;
|
||||
MutexLocker ml(&lock_);
|
||||
Timeline::ReclaimCachedBlocksFromThreads();
|
||||
Timeline::ReclaimCachedBlocksFromThreads(&it);
|
||||
|
||||
intptr_t pid = OS::ProcessId();
|
||||
char* filename =
|
||||
@@ -1732,8 +1733,9 @@ void TimelineEventFixedBufferRecorder::ForEachNonEmptyBlock(
|
||||
std::function<void(const TimelineEventBlock&)>&& handle_block) {
|
||||
// Acquire the recorder's lock to prevent the reclaimed blocks from being
|
||||
// handed out again until the trace has been serialized.
|
||||
OSThreadIterator it;
|
||||
MutexLocker ml(&lock_);
|
||||
Timeline::ReclaimCachedBlocksFromThreads();
|
||||
Timeline::ReclaimCachedBlocksFromThreads(&it);
|
||||
ResetTimeTracking();
|
||||
intptr_t block_offset = FindOldestBlockIndexLocked();
|
||||
if (block_offset == -1) {
|
||||
@@ -2590,8 +2592,9 @@ void TimelineEventEndlessRecorder::ForEachNonEmptyBlock(
|
||||
std::function<void(const TimelineEventBlock&)>&& handle_block) {
|
||||
// Acquire the recorder's lock to prevent the reclaimed blocks from being
|
||||
// handed out again until the trace has been serialized.
|
||||
OSThreadIterator it;
|
||||
MutexLocker ml(&lock_);
|
||||
Timeline::ReclaimCachedBlocksFromThreads();
|
||||
Timeline::ReclaimCachedBlocksFromThreads(&it);
|
||||
ResetTimeTracking();
|
||||
for (TimelineEventBlock* current = head_; current != nullptr;
|
||||
current = current->next()) {
|
||||
|
||||
@@ -281,7 +281,7 @@ class Timeline : public AllStatic {
|
||||
}
|
||||
|
||||
// Reclaim all |TimelineEventBlocks|s that are cached by threads.
|
||||
static void ReclaimCachedBlocksFromThreads();
|
||||
static void ReclaimCachedBlocksFromThreads(OSThreadIterator* it);
|
||||
|
||||
static void Clear();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user