Added unique function names (IOTHreadRun, DBTHreadRun, etc)

Added unique function names (IOThreadRun, DBThreadRun, etc)
which make it possible to tell the thread name in crash dumps
from the call stack. Added a volatile automatic variable to
avoid global optimizers optimizing the function.

This change would help us to triage ThreadWatcher crashes
for IO thread. It would also help in debugging crashes.

R=sky, jar
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=147949

[ramant (doing other things), 2012-07-21 20:56:38]

Hi sky and jar,
  Implemented changes similar to what eroman had implemented in ThreadWatcher to
identify the thread. Added volatile (thanks jar for the suggestion) to be sure
that global optimizer doesn't collapse these functions.

  This changes adds an extra function call, but will help us in auto triage of
crashes and also help us in debugging crash dumps.

thanks,
raman

[jar (doing other things), 2012-07-21 23:19:36]

lgtm

[commit-bot: I haz the power, 2012-07-22 00:34:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rtenneti@chromium.org/10796079/7003

[commit-bot: I haz the power, 2012-07-22 00:34:41]

Presubmit check for 10796079-7003 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change has an associated bug, add BUG=[bug number].

If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    content/browser

[sky, 2012-07-23 15:49:17]

I added Eric to the review list.

https://chromiumcodereview.appspot.com/10796079/diff/7003/content/browser/bro...
File content/browser/browser_thread_impl.cc (right):

https://chromiumcodereview.appspot.com/10796079/diff/7003/content/browser/bro...
content/browser/browser_thread_impl.cc:101: #if defined(COMPILER_MSVC)
Use MSVC_PUSH_DISABLE/POP_DISABLE to avoid the ifdefs (in
base/compiler_specific.h).

https://chromiumcodereview.appspot.com/10796079/diff/7003/content/browser/bro...
content/browser/browser_thread_impl.cc:107: volatile int line_number = __LINE__;
Why do you need the line numbers on all these?

[ramant (doing other things), 2012-07-23 17:33:04]

Hi Eric,
  I had used what you had done in ThreadWatcher.cc (removed the COMPILER_MSVC
ifdef). Would appreciate your comments.

thanks
raman

http://codereview.chromium.org/10796079/diff/7003/content/browser/browser_thr...
File content/browser/browser_thread_impl.cc (right):

http://codereview.chromium.org/10796079/diff/7003/content/browser/browser_thr...
content/browser/browser_thread_impl.cc:101: #if defined(COMPILER_MSVC)
On 2012/07/23 15:49:17, sky wrote:
> Use MSVC_PUSH_DISABLE/POP_DISABLE to avoid the ifdefs (in
> base/compiler_specific.h).

Done.

http://codereview.chromium.org/10796079/diff/7003/content/browser/browser_thr...
content/browser/browser_thread_impl.cc:107: volatile int line_number = __LINE__;
On 2012/07/23 15:49:17, sky wrote:
> Why do you need the line numbers on all these?

Compilers could optimize away all *ThreadRun functions away. line_number
provides a way to trick the optimizer (global?) not to fold this code (or keep
the function name in the callstack). jar mentioned secondary optimizations as
the other reason.

[jar (doing other things), 2012-07-23 17:37:41]

The line numbers ensure that the secondary (link-time) optimizer won't "fold the
identical functions" into a single call.  I *think* that even if you turn off
optimizations, such folding may be fair game... not sure.

In any case, the cost of the code addition seems very minimal (absolute bytes of
code) and insignificant in terms of perf (only run once).

On 2012/07/23 15:49:17, sky wrote:
> I added Eric to the review list.
> 
>
https://chromiumcodereview.appspot.com/10796079/diff/7003/content/browser/bro...
> File content/browser/browser_thread_impl.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10796079/diff/7003/content/browser/bro...
> content/browser/browser_thread_impl.cc:101: #if defined(COMPILER_MSVC)
> Use MSVC_PUSH_DISABLE/POP_DISABLE to avoid the ifdefs (in
> base/compiler_specific.h).
> 
>
https://chromiumcodereview.appspot.com/10796079/diff/7003/content/browser/bro...
> content/browser/browser_thread_impl.cc:107: volatile int line_number =
__LINE__;
> Why do you need the line numbers on all these?

[eroman, 2012-07-23 17:59:02]

For VC++ the MSVC_DISABLE_OPTIMIZE() is sufficient to thwart all optimizations.
The use of __LINE__ is unnecessary for those builds.

However the gcc compilers don't have an equivalent way to disable optimizations
locally in a file. To prevent inlining on those builds, I believe it is
necessary to (1) mark the functions as NOINLINE to prevent them getting inlined
directly into the callsites, and (2) giving each function a unique body so that
ICF doesn't roll all of the functions into a single one (for instance by adding
an expression using __LINE__). See also the discussion on this thread:
https://chromiumcodereview.appspot.com/10700080

LGTM as-is. There may need to be some refinements for mac/linux, but this should
work nicely for Windows and start letting us play with the data.

[sky, 2012-07-23 18:19:32]

LGTM

[commit-bot: I haz the power, 2012-07-23 18:21:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rtenneti@chromium.org/10796079/3002

[commit-bot: I haz the power, 2012-07-23 19:58:03]

Try job failure for 10796079-3002 (retry) on mac_rel for step "browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...