Enable setting thread names on Linux

Enable setting thread names on Linux

BUG=107547
TEST=Manual

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

[Rick Byers, 2011-12-14 15:45:07]

Hey Evan,
I see you tried to turn on thread naming on Linux awhile back, but disabled it
because naming the main thread broke tools like killall.  I'd love to have
thread names for the non-main threads - it would simplify some debugging
scenarios.  Any reason we can't just skip the main thread as I've done here?

Thanks!
   Rick

http://codereview.chromium.org/8936015/diff/1/base/threading/platform_thread_...
File base/threading/platform_thread_posix.cc (right):

http://codereview.chromium.org/8936015/diff/1/base/threading/platform_thread_...
base/threading/platform_thread_posix.cc:192: if (dynamic_pthread_setname_np) {
What's the benefit of using this API at all?  Can't we just use prctl in all
cases (and then avoid the cost of the dynamic symbol lookup and extra complexity
of having two different code paths?).

[Rick Byers, 2011-12-14 18:16:54]

Hi Jim,
Evan is out on leave.  Can you take a look at this please?  

Also see my question about the use of 'pthread_setname_np' - I'm tempted to take
that part out and rely only on prctl (that was how I was going to implement this
before I found Evan's pre-existing disabled code).

Thanks,
   Rick

[jar (doing other things), 2011-12-14 18:33:35]

I'm not as familiar with the Linux subtleties... so I'm putting erg@ on the
review list to comment.

I think I track thread names in the profiler, and your change (and approach)
seems inconsequential to that... so getting Linux folks to approve this is the
only issue.

[Elliot Glaysher, 2011-12-14 18:48:11]

On 2011/12/14 18:33:35, jar wrote:
> I'm not as familiar with the Linux subtleties... so I'm putting erg@ on the
> review list to comment.
> 
> I think I track thread names in the profiler, and your change (and approach)
> seems inconsequential to that... so getting Linux folks to approve this is the
> only issue.

I don't know why Evan disabled this; I've never touched the low level threading
primitives on Linux.

[Rick Byers, 2011-12-14 19:15:44]

On 2011/12/14 18:48:11, Elliot Glaysher wrote:
> On 2011/12/14 18:33:35, jar wrote:
> > I'm not as familiar with the Linux subtleties... so I'm putting erg@ on the
> > review list to comment.
> > 
> > I think I track thread names in the profiler, and your change (and approach)
> > seems inconsequential to that... so getting Linux folks to approve this is
the
> > only issue.
> 
> I don't know why Evan disabled this; I've never touched the low level
threading
> primitives on Linux.

It looks like (from the comments / history) Evan disabled this because of the
impact of setting the main thread name (changes the process name - breaking
tools like killall).  I avoid that by setting the name only for threads other
than the main thread, so hopefully it shouldn't impact anyone else.  

V8 uses prctl already to set the thread name for the 'SignalSender' thread (see
http://codesearch.google.com/#OAMlx_jo-ck/src/v8/src/platform-linux.cc&exact_...),
so there hopefully shouldn't be any consequences to using it more often.  I'm
not sure about the glib API call Evan had in here though -  I could cut that
piece out to lower the risk of any impact further.  Preferences?

[Rick Byers, 2011-12-15 15:46:40]

On 2011/12/14 19:15:44, Rick Byers wrote:
> On 2011/12/14 18:48:11, Elliot Glaysher wrote:
> > On 2011/12/14 18:33:35, jar wrote:
> > > I'm not as familiar with the Linux subtleties... so I'm putting erg@ on
the
> > > review list to comment.
> > > 
> > > I think I track thread names in the profiler, and your change (and
approach)
> > > seems inconsequential to that... so getting Linux folks to approve this is
> the
> > > only issue.
> > 
> > I don't know why Evan disabled this; I've never touched the low level
> threading
> > primitives on Linux.
> 
> It looks like (from the comments / history) Evan disabled this because of the
> impact of setting the main thread name (changes the process name - breaking
> tools like killall).  I avoid that by setting the name only for threads other
> than the main thread, so hopefully it shouldn't impact anyone else.  
> 
> V8 uses prctl already to set the thread name for the 'SignalSender' thread
(see
>
http://codesearch.google.com/#OAMlx_jo-ck/src/v8/src/platform-linux.cc&exact_...),
> so there hopefully shouldn't be any consequences to using it more often.  I'm
> not sure about the glib API call Evan had in here though -  I could cut that
> piece out to lower the risk of any impact further.  Preferences?

Ping.  Jim, if there are no concerns (and no-one else we should check with), can
I get an LGTM from you?

Thanks!
   Rick

[jar (doing other things), 2011-12-15 17:48:52]

lgtm

[commit-bot: I haz the power, 2011-12-15 18:04:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rbyers@chromium.org/8936015/1

[commit-bot: I haz the power, 2011-12-15 19:40:06]

Change committed as 114681

[Evan Martin, 2012-01-03 17:34:28]

FWIW, I can't remember why I did the extra work for pthread_setname_np.  I think
maybe it was for compatibility with OS X?  (The setname_np is not
Linux-specific, but it is "non portable".)

[Rick Byers, 2012-01-03 17:47:00]

On 2012/01/03 17:34:28, Evan Martin wrote:
> FWIW, I can't remember why I did the extra work for pthread_setname_np.  I
think
> maybe it was for compatibility with OS X?  (The setname_np is not
> Linux-specific, but it is "non portable".)

Thanks for the context Evan.  So I assume that you're happy with where this
landed in your absence.  Let me know if there's anything you'd like me to change
- I'm happy to do it.