1460973 - The "Java" thread has a bad registerTime

Status: RESOLVED FIXED

(Core :: Gecko Profiler, enhancement, P2)

Description

[Markus Stange [:mstange]]

The ThreadInfo object for the "Java" thread gets created here:
https://searchfox.org/mozilla-central/rev/3f17a234769d25fca5144ebb8abc8e1cb3c56c16/tools/profiler/core/platform.cpp#1767

The ThreadInfo's mRegisterTime gets initialized to the current time in the ThreadInfo constructor:
https://searchfox.org/mozilla-central/rev/3f17a234769d25fca5144ebb8abc8e1cb3c56c16/tools/profiler/core/ThreadInfo.h#25

For the "Java" thread, this means that the register time is set to the time at which the profile was dumped, so it's at the end of the collected range and will result in perf.html displaying the entire thread has "not having started yet".

We could initialize this field to CorePS::ProcessStartTime() instead, by adding another argument to the ThreadInfo constructor (possibly with a default value of TimeStamp::Now() so that existing callers don't need to be updated).

Comment 1

[Markus Stange [:mstange]]

Attachment: Bug 1460973 - Set the thread register time of the Java thread to the process creation time.

Review commit: https://reviewboard.mozilla.org/r/246652/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/246652/

Comment 2

[Greg Tatum [:gregtatum]]

Comment on attachment 8980481 [details]
Bug 1460973 - Set the thread register time of the Java thread to the process creation time.

https://reviewboard.mozilla.org/r/246652/#review252990

Ah, nice simple fix. Thanks.

Comment 3

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 56d849a4a4189dbc36df7a6b11989db3a02c314c -d eba75ea19102: rebasing 465259:56d849a4a418 "Bug 1460973 - Set the thread register time of the Java thread to the process creation time. r=gregtatum" (tip)
merging tools/profiler/core/platform.cpp
warning: conflicts while merging tools/profiler/core/platform.cpp! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 4

[Markus Stange [:mstange]]

Comment on attachment 8980481 [details]
Bug 1460973 - Set the thread register time of the Java thread to the process creation time.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246652/diff/1-2/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/56f0565728de
Set the thread register time of the Java thread to the process creation time. r=gregtatum

Comment 6

[Greg Tatum [:gregtatum]]

Comment on attachment 8980481 [details]
Bug 1460973 - Set the thread register time of the Java thread to the process creation time.

https://reviewboard.mozilla.org/r/246652/#review253076

It looks like this breaks loading existing saved devtools profiles. I think this probably needs some kind of profile upgrader to handle the new category format. The rest of the code all looks fine, and it works otherwise.

STR:
 * Load current Nightly
 * Record a profile
 * Save it to a JSON file
 * Open up this new build
 * Load in the old profile

The call tree is broken.

Comment 7

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/56f0565728de