Commit 1e28e27d authored by jar@chromium.org's avatar jar@chromium.org

Revert 107921 - Pile of nits for tracked object enablement

[Being extra conservative an leaking tracking data got
the base_unittest heap_checkers too excited. I need 
a supresion to match what we already have in the 
browser runs to calm them... so I'll revert for now.]

Be extra carful about handling races in access to status_.
This will avoid generating a delta between a null time
and a real time, when status is changing in/around
the run of a task.  This won't help with the benign
race for checking status_, but it may help with unit
test tsan complaints.

Leak data aggressively, rather than cleaning up, to
prevent any chance of a data access race between
tracked object testing (which need a near-virgin
global state, and hence must start by cleaning it up), and
other tests, which may have lingering threaded actions, 
that still access some previously created task tracking
data.

Provide more options for flags to enable/disable
tracking.  These options might become useful
if we changed the default to not do tracking.

Allow for HTML generation even if the tracking has
changed to being disabled.  This is especially useful
for looking at the tracked instances that were
monitored after turning tracking on by default, but
before the command line deactiated tracking.

r=rtenneti
bug=102327
Review URL: http://codereview.chromium.org/8414050

TBR=jar@chromium.org
Review URL: http://codereview.chromium.org/8414051

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@107922 0039d316-1c4b-4281-b951-d872f2087c98
parent b2b7ee42
...@@ -220,7 +220,7 @@ void ThreadData::OnThreadTerminationCleanup() const { ...@@ -220,7 +220,7 @@ void ThreadData::OnThreadTerminationCleanup() const {
// static // static
void ThreadData::WriteHTML(const std::string& query, std::string* output) { void ThreadData::WriteHTML(const std::string& query, std::string* output) {
if (status_ == UNINITIALIZED) if (!ThreadData::tracking_status())
return; // Not yet initialized. return; // Not yet initialized.
DataCollector collected_data; // Gather data. DataCollector collected_data; // Gather data.
...@@ -406,18 +406,8 @@ void ThreadData::TallyRunOnNamedThreadIfTracking( ...@@ -406,18 +406,8 @@ void ThreadData::TallyRunOnNamedThreadIfTracking(
? tracked_objects::TrackedTime(completed_task.time_posted) ? tracked_objects::TrackedTime(completed_task.time_posted)
: tracked_objects::TrackedTime(completed_task.delayed_run_time); : tracked_objects::TrackedTime(completed_task.delayed_run_time);
// Watch out for a race where status_ is changing, and hence one or both Duration queue_duration = start_of_run - effective_post_time;
// of start_of_run or end_of_run is zero. IN that case, we didn't bother to Duration run_duration = end_of_run - start_of_run;
// get a time value since we "weren't tracking" and we were trying to be
// efficient by not calling for a genuine time value. For simplicity, we'll
// use a default zero duration when we can't calculate a true value.
Duration queue_duration;
Duration run_duration;
if (!start_of_run.is_null()) {
queue_duration = start_of_run - effective_post_time;
if (!end_of_run.is_null())
run_duration = end_of_run - start_of_run;
}
current_thread_data->TallyADeath(*birth, queue_duration, run_duration); current_thread_data->TallyADeath(*birth, queue_duration, run_duration);
} }
...@@ -449,13 +439,8 @@ void ThreadData::TallyRunOnWorkerThreadIfTracking( ...@@ -449,13 +439,8 @@ void ThreadData::TallyRunOnWorkerThreadIfTracking(
if (!current_thread_data) if (!current_thread_data)
return; return;
Duration queue_duration; Duration queue_duration = start_of_run - time_posted;
Duration run_duration; Duration run_duration = end_of_run - start_of_run;
if (!start_of_run.is_null()) {
queue_duration = start_of_run - time_posted;
if (!end_of_run.is_null())
run_duration = end_of_run - start_of_run;
}
current_thread_data->TallyADeath(*birth, queue_duration, run_duration); current_thread_data->TallyADeath(*birth, queue_duration, run_duration);
} }
...@@ -539,9 +524,9 @@ bool ThreadData::tracking_status() { ...@@ -539,9 +524,9 @@ bool ThreadData::tracking_status() {
// static // static
TrackedTime ThreadData::Now() { TrackedTime ThreadData::Now() {
if (kTrackAllTaskObjects && tracking_status()) if (!kTrackAllTaskObjects || status_ != ACTIVE)
return TrackedTime::Now(); return TrackedTime(); // Super fast when disabled, or not compiled.
return TrackedTime(); // Super fast when disabled, or not compiled. return TrackedTime::Now();
} }
// static // static
...@@ -561,19 +546,6 @@ void ThreadData::ShutdownSingleThreadedCleanup() { ...@@ -561,19 +546,6 @@ void ThreadData::ShutdownSingleThreadedCleanup() {
unregistered_thread_data_pool_ = NULL; unregistered_thread_data_pool_ = NULL;
} }
// Put most global static back in pristine shape.
thread_number_counter_ = 0;
tls_index_.Set(NULL);
status_ = UNINITIALIZED;
// To avoid any chance of racing in unit tests, which is the only place we
// call this function, we will leak all the data structures we recovered.
// These structures could plausibly be used by other threads in earlier tests
// that are still running.
return;
// If we wanted to cleanup (on a single thread), here is what we would do.
if (final_pool) { if (final_pool) {
// The thread_data_list contains *all* the instances, and we'll use it to // The thread_data_list contains *all* the instances, and we'll use it to
// delete them. This pool has pointers to some instances, and we just // delete them. This pool has pointers to some instances, and we just
...@@ -595,6 +567,10 @@ void ThreadData::ShutdownSingleThreadedCleanup() { ...@@ -595,6 +567,10 @@ void ThreadData::ShutdownSingleThreadedCleanup() {
next_thread_data->death_map_.clear(); next_thread_data->death_map_.clear();
delete next_thread_data; // Includes all Death Records. delete next_thread_data; // Includes all Death Records.
} }
// Put most global static back in pristine shape.
thread_number_counter_ = 0;
tls_index_.Set(NULL);
status_ = UNINITIALIZED;
} }
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
......
...@@ -1218,11 +1218,10 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { ...@@ -1218,11 +1218,10 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
} }
if (parsed_command_line().HasSwitch(switches::kEnableTracking)) { if (parsed_command_line().HasSwitch(switches::kEnableTracking)) {
// User wants to override default tracking status.
std::string flag = std::string flag =
parsed_command_line().GetSwitchValueASCII(switches::kEnableTracking); parsed_command_line().GetSwitchValueASCII(switches::kEnableTracking);
bool enabled = flag.compare("0") != 0; if (flag.compare("0") == 0)
tracked_objects::ThreadData::InitializeAndSetTrackingStatus(enabled); tracked_objects::ThreadData::InitializeAndSetTrackingStatus(false);
} }
// This forces the TabCloseableStateWatcher to be created and, on chromeos, // This forces the TabCloseableStateWatcher to be created and, on chromeos,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment