Detect child tasks born during a profiled tasks

Detect child tasks born during a profiled tasks

r=rtenneti
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=114336

[jar (doing other things), 2011-12-13 07:31:40]

Raman,

This version has the const static set to "false," and hence won't include almost
any of the code in our release build (after global optimization cleans up
resulting dead code).  

By landing this, I'll be able to do tiny (one-line) changes to turn this
profiling on and off (to see impact on perf bots), and also eroman will be able
to play with the resulting JSON.

All unit tests will pass with the flag in either true or false settings.

http://codereview.chromium.org/8894022/diff/9010/base/tracked_objects.cc
File base/tracked_objects.cc (left):

http://codereview.chromium.org/8894022/diff/9010/base/tracked_objects.cc#oldc...
base/tracked_objects.cc:290: if (!thread_data)
Thread local store no longers call us with NULLs.

[ramant (doing other things), 2011-12-13 19:14:45]

LGTM.

I think tracked_objects.cc:614 would be good to fix.

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.cc
File base/tracked_objects.cc (right):

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.cc#new...
base/tracked_objects.cc:300:
reinterpret_cast<ThreadData*>(thread_data)->OnThreadTerminationCleanup();
nit: could we add a DCHECK for thread_data? thanks.

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.cc#new...
base/tracked_objects.cc:614: if (kTrackParentChildLinks && kInitialStartupState
== FULLY_ACTIVE)
should this be if (!kTrackParentChildLinks ...)?

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.cc#new...
base/tracked_objects.cc:649: ThreadData* current_thread_data = Get();
nit: could we add DCHECK for current_thread_data? It looks like for unittests
Get() could return NULL. In such cases, they shouldn't call this method. It is
your call.

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.cc#new...
base/tracked_objects.cc:705: status_ = DORMANT_DURING_TESTS;  // Almost
UNINITIALIZED
nit: period at the end.

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.cc#new...
base/tracked_objects.cc:775: void DataCollector::Append(const ThreadData
&thread_data,
nit: ThreadData & ->: ThreadData&

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.h
File base/tracked_objects.h (right):

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.h#newc...
base/tracked_objects.h:350: DEACTIVATED,           // No longer recordin
profling.
nit: recordin -> recording

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.h#newc...
base/tracked_objects.h:617: // correnponds to potentially non local Births
(location and thread), and a
nit: correnponds -> corresponds

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.h#newc...
base/tracked_objects.h:648: // incarations).
nit: incarations -> incarnations

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.h#newc...
base/tracked_objects.h:701: // The complete list of parent-child relatioships
among tasks.
nit: relatioships -> relationships

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects_unitte...
File base/tracked_objects_unittest.cc (right):

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects_unitte...
base/tracked_objects_unittest.cc:154: 
nit: extra line.

[jar (doing other things), 2011-12-13 20:29:10]

Changes made per comments by rtenneti.

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.cc
File base/tracked_objects.cc (right):

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.cc#new...
base/tracked_objects.cc:300:
reinterpret_cast<ThreadData*>(thread_data)->OnThreadTerminationCleanup();
On 2011/12/13 19:14:45, ramant wrote:
> nit: could we add a DCHECK for thread_data? thanks.

Done.

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.cc#new...
base/tracked_objects.cc:614: if (kTrackParentChildLinks && kInitialStartupState
== FULLY_ACTIVE)
On 2011/12/13 19:14:45, ramant wrote:
> should this be if (!kTrackParentChildLinks ...)?

Done  .... and I renamed the enums to help make the code clearer, so I won't
make this mistake again.

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.cc#new...
base/tracked_objects.cc:649: ThreadData* current_thread_data = Get();
On 2011/12/13 19:14:45, ramant wrote:
> nit: could we add DCHECK for current_thread_data? It looks like for unittests
> Get() could return NULL. In such cases, they shouldn't call this method. It is
> your call.

Done.

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.cc#new...
base/tracked_objects.cc:705: status_ = DORMANT_DURING_TESTS;  // Almost
UNINITIALIZED
On 2011/12/13 19:14:45, ramant wrote:
> nit: period at the end.

Done.

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.cc#new...
base/tracked_objects.cc:775: void DataCollector::Append(const ThreadData
&thread_data,
On 2011/12/13 19:14:45, ramant wrote:
> nit: ThreadData & ->: ThreadData&

Done.

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.h
File base/tracked_objects.h (right):

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.h#newc...
base/tracked_objects.h:350: DEACTIVATED,           // No longer recordin
profling.
On 2011/12/13 19:14:45, ramant wrote:
> nit: recordin -> recording

Done.

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.h#newc...
base/tracked_objects.h:617: // correnponds to potentially non local Births
(location and thread), and a
On 2011/12/13 19:14:45, ramant wrote:
> nit: correnponds -> corresponds

Done.

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.h#newc...
base/tracked_objects.h:648: // incarations).
On 2011/12/13 19:14:45, ramant wrote:
> nit: incarations -> incarnations

Done.

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects.h#newc...
base/tracked_objects.h:701: // The complete list of parent-child relatioships
among tasks.
On 2011/12/13 19:14:45, ramant wrote:
> nit: relatioships -> relationships

Done.

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects_unitte...
File base/tracked_objects_unittest.cc (right):

http://codereview.chromium.org/8894022/diff/24001/base/tracked_objects_unitte...
base/tracked_objects_unittest.cc:154: 
On 2011/12/13 19:14:45, ramant wrote:
> nit: extra line.

Done.

[ramant (doing other things), 2011-12-13 20:58:58]

LGTM. thanks for making the changes.