dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Revert "Reland "[VM/Timeline] Add test that makes TSAN check the track metadata map""

Browse Source 
This reverts commit d9f1aefe9e.

Reason for revert: broke vm-kernel-msvc-windows

Original change's description:
> Reland "[VM/Timeline] Add test that makes TSAN check the track metadata map"
>
> This is a reland of commit 8a39d781ce
>
> Fixed by changing accidental usages of `kInvalidThreadId` to
> `kInvalidThreadJoinId`.
>
> TEST=Windows tryjobs
>
> Original change's description:
> > [VM/Timeline] Add test that makes TSAN check the track metadata map
> >
> > TEST=CI
> >
> > Change-Id: I9e4fa4bbc08e77a1365e84f2d048384d7f0b1350
> > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/281720
> > Reviewed-by: Ben Konyi <bkonyi@google.com>
> > Commit-Queue: Derek Xu <derekx@google.com>
>
> Fixes https://github.com/dart-lang/sdk/issues/51328
> Change-Id: I9219ed9bee212ecb50e3d0a0ed639d3ac8676238
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/281920
> Reviewed-by: Ben Konyi <bkonyi@google.com>
> Commit-Queue: Derek Xu <derekx@google.com>

TBR=bkonyi@google.com,derekx@google.com,dart-scoped@luci-project-accounts.iam.gserviceaccount.com

Change-Id: Iea36a34a392a51b53699527a7af36b93f924057f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/282340
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Derek Xu <derekx@google.com>
This commit is contained in:
Derek Xu
2023-02-10 13:49:06 +00:00
committed by Commit Queue
parent e1a2b10a81
commit 41d6db8ce5
4 changed files with 13 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files
runtime/vm/os_thread.cc
+1 -3
View File
@@ -27,9 +27,7 @@ inline void UpdateTimelineTrackMetadata(const OSThread& thread) {
RecorderLockScope rl;
TimelineEventRecorder* recorder = Timeline::recorder();
if (recorder != nullptr && !rl.IsShuttingDown()) {
recorder->AddTrackMetadataBasedOnThread(
OS::ProcessId(), OSThread::ThreadIdToIntPtr(thread.trace_id()),
thread.name());
recorder->AddTrackMetadataBasedOnThread(thread);
}
}
#endif // defined(SUPPORT_TIMELINE)
runtime/vm/timeline.cc
+11 -11
View File
@@ -1072,10 +1072,8 @@ TimelineEventRecorder::TimelineEventRecorder()
// were initialized before this point.
OSThreadIterator it;
while (it.HasNext()) {
OSThread& thread = *it.Next();
AddTrackMetadataBasedOnThread(OS::ProcessId(),
OSThread::ThreadIdToIntPtr(thread.trace_id()),
thread.name());
OSThread* thread = it.Next();
AddTrackMetadataBasedOnThread(*thread);
}
}
@@ -1248,9 +1246,7 @@ TimelineEventBlock* TimelineEventRecorder::GetNewBlock() {
}
void TimelineEventRecorder::AddTrackMetadataBasedOnThread(
const intptr_t process_id,
const intptr_t trace_id,
const char* thread_name) {
const OSThread& thread) {
if (FLAG_timeline_recorder != nullptr &&
// There is no way to retrieve track metadata when a callback or systrace
// recorder is in use, so we don't need to update the map in these cases.
@@ -1258,19 +1254,23 @@ void TimelineEventRecorder::AddTrackMetadataBasedOnThread(
strcmp("systrace", FLAG_timeline_recorder) != 0) {
MutexLocker ml(&track_uuid_to_track_metadata_lock_);
void* key = reinterpret_cast<void*>(trace_id);
const intptr_t hash = Utils::WordHash(trace_id);
intptr_t pid = OS::ProcessId();
intptr_t tid = OSThread::ThreadIdToIntPtr(thread.trace_id());
const char* thread_name = thread.name();
void* key = reinterpret_cast<void*>(tid);
const intptr_t hash = Utils::WordHash(tid);
SimpleHashMap::Entry* entry =
track_uuid_to_track_metadata_.Lookup(key, hash, true);
if (entry->value == nullptr) {
entry->value = new TimelineTrackMetadata(
process_id, trace_id,
pid, tid,
Utils::CreateCStringUniquePtr(
Utils::StrDup(thread_name == nullptr ? "" : thread_name)));
} else {
TimelineTrackMetadata* value =
static_cast<TimelineTrackMetadata*>(entry->value);
ASSERT(process_id == value->pid());
ASSERT(pid == value->pid());
value->set_track_name(Utils::CreateCStringUniquePtr(
Utils::StrDup(thread_name == nullptr ? "" : thread_name)));
}
runtime/vm/timeline.h
+1 -3
View File
@@ -848,9 +848,7 @@ class TimelineEventRecorder : public MallocAllocated {
void FinishBlock(TimelineEventBlock* block);
// This function must be called at least once for each thread that corresponds
// to a track in the trace.
void AddTrackMetadataBasedOnThread(const intptr_t process_id,
const intptr_t trace_id,
const char* thread_name);
void AddTrackMetadataBasedOnThread(const OSThread& thread);
protected:
#ifndef PRODUCT
runtime/vm/timeline_test.cc
-51
View File
@@ -3,7 +3,6 @@
// BSD-style license that can be found in the LICENSE file.
#include <cstring>
#include <memory>
#include "platform/assert.h"
@@ -374,56 +373,6 @@ TEST_CASE(TimelineRingRecorderJSONOrder) {
EXPECT(alpha < beta);
}
TEST_CASE(TimelineTrackMetadataRace) {
struct ReportMetadataArguments {
TimelineEventRingRecorder* recorder;
ThreadJoinId join_id = OSThread::kInvalidThreadJoinId;
};
TimelineEventRingRecorder recorder;
const intptr_t fake_process_id = 1;
const intptr_t fake_trace_id = 1;
// Try concurrently reading from / writing to the metadata map. I don't think
// it's possible to assert anything about the outcome, because of scheduling
// uncertainty. This test is just used to ensure that TSAN checks the metadata
// map code.
JSONStream js;
TimelineEventFilter filter;
const ReportMetadataArguments report_metadata_1_arguments{&recorder};
const ReportMetadataArguments report_metadata_2_arguments{&recorder};
OSThread::Start(
"ReportMetadata1",
[](uword arguments_ptr) {
ReportMetadataArguments& arguments =
*reinterpret_cast<ReportMetadataArguments*>(arguments_ptr);
arguments.recorder->AddTrackMetadataBasedOnThread(
fake_process_id, fake_trace_id, "Thread 1");
arguments.join_id =
OSThread::GetCurrentThreadJoinId(OSThread::Current());
},
reinterpret_cast<uword>(&report_metadata_1_arguments));
OSThread::Start(
"ReportMetadata2",
[](uword arguments_ptr) {
ReportMetadataArguments& arguments =
*reinterpret_cast<ReportMetadataArguments*>(arguments_ptr);
arguments.recorder->AddTrackMetadataBasedOnThread(
fake_process_id, fake_trace_id, "Incorrect Name");
arguments.join_id =
OSThread::GetCurrentThreadJoinId(OSThread::Current());
},
reinterpret_cast<uword>(&report_metadata_2_arguments));
recorder.PrintJSON(&js, &filter);
while (
report_metadata_1_arguments.join_id == OSThread::kInvalidThreadJoinId ||
report_metadata_2_arguments.join_id == OSThread::kInvalidThreadJoinId) {
// Spin until the join IDs have been set.
}
OSThread::Join(report_metadata_1_arguments.join_id);
OSThread::Join(report_metadata_2_arguments.join_id);
}
#endif // !PRODUCT
#if defined(SUPPORT_TIMELINE)