Assign thread name to in_proc renderer and gpu thread.

Assign thread name to in_proc renderer and gpu thread.

This would be very helpful for analyzing traces.txt of ANR logs.

BUG=570819
Committed: https://crrev.com/5380add90830ef6a550563c87736484c1b2bfd17
Cr-Commit-Position: refs/heads/master@{#365953}

[hush (inactive), 2015-12-12 00:09:31]

hush@chromium.org changed reviewers:
+ boliu@chromium.org

[hush (inactive), 2015-12-12 00:09:32]

Hi Bo,
PTAL. Single process renderer will stick around, as long as there are L and M
devices. So I guess this is still relevant.

https://codereview.chromium.org/1518213002/diff/1/content/renderer/in_process...
File content/renderer/in_process_renderer_thread.cc (right):

https://codereview.chromium.org/1518213002/diff/1/content/renderer/in_process...
content/renderer/in_process_renderer_thread.cc:35:
RenderThreadImpl::Create(params_);
By the way, I'm sure AttachCurrentThread is called somewhere in line 34 or 35,
because the in_proc renderer thread (Android webview's blink thread) gets
assigned a Thread-??? by JVM, instead appearing as detached thread in
anr/traces.txt

[boliu, 2015-12-12 00:11:45]

lgtm but I don't own this code?

https://codereview.chromium.org/1518213002/diff/1/content/renderer/in_process...
File content/renderer/in_process_renderer_thread.cc (right):

https://codereview.chromium.org/1518213002/diff/1/content/renderer/in_process...
content/renderer/in_process_renderer_thread.cc:35:
RenderThreadImpl::Create(params_);
On 2015/12/12 00:09:32, hush wrote:
> By the way, I'm sure AttachCurrentThread is called somewhere in line 34 or 35,
> because the in_proc renderer thread (Android webview's blink thread) gets
> assigned a Thread-??? by JVM, instead appearing as detached thread in
> anr/traces.txt

At least video uses JNI on blink main thread. Also don't really matter either
way

[hush (inactive), 2015-12-12 00:21:37]

hush@chromium.org changed reviewers:
+ creis@chromium.org

[hush (inactive), 2015-12-12 00:21:37]

Hello Charlie,
Can you take a look?

[Charlie Reis, 2015-12-12 07:19:54]

creis@chromium.org changed reviewers:
+ jam@chromium.org
- creis@chromium.org

[Charlie Reis, 2015-12-12 07:19:54]

Sorry, I'm not a good reviewer for this.  Maybe jam@?

[jam, 2015-12-14 15:29:31]

Description was changed from

==========
Assign thread name to in_proc renderer thread.

This would be very helpful for analyzing traces.txt of ANR logs.

BUG=
==========

to

==========
Assign thread name to in_proc renderer thread.

This would be very helpful for analyzing traces.txt of ANR logs.

BUG=
==========

[jam, 2015-12-14 15:29:31]

jam@chromium.org changed reviewers:
+ sievers@chromium.org
- jam@chromium.org

[jam, 2015-12-14 15:29:39]

jam@chromium.org changed reviewers:
+ jam@chromium.org

[jam, 2015-12-14 15:29:39]

switching to sievers

[no sievers, 2015-12-14 19:34:38]

Sounds like for traces.txt, Android doesn't resolve thread names we set with
prctl(PR_SET_NAME).
Why don't we actually consistently do it for all browser threads then from
PlatformThread::SetName (platform_thread_android.cc)?

[hush (inactive), 2015-12-14 19:44:06]

On 2015/12/14 19:34:38, sievers wrote:
> Sounds like for traces.txt, Android doesn't resolve thread names we set with
> prctl(PR_SET_NAME).
> Why don't we actually consistently do it for all browser threads then from
> PlatformThread::SetName (platform_thread_android.cc)?

All browser threads already do it in
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/br...

[hush (inactive), 2015-12-14 19:47:16]

On 2015/12/14 19:44:06, hush wrote:
> On 2015/12/14 19:34:38, sievers wrote:
> > Sounds like for traces.txt, Android doesn't resolve thread names we set with
> > prctl(PR_SET_NAME).
> > Why don't we actually consistently do it for all browser threads then from
> > PlatformThread::SetName (platform_thread_android.cc)?
> 
> All browser threads already do it in
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/br...

According to my experience, if the native thread is never attached to Java, then
the name set in PlatformThread::SetName will work, but truncated to certain
number of characters. GPU thread is such an example.
If the native thread is attached to Java at one point, then we need to make sure
it is attached with a meaningful name, otherwise JVM will assign Thread-??? to
the name.

[hush (inactive), 2015-12-14 19:48:48]

On 2015/12/14 19:47:16, hush wrote:
> On 2015/12/14 19:44:06, hush wrote:
> > On 2015/12/14 19:34:38, sievers wrote:
> > > Sounds like for traces.txt, Android doesn't resolve thread names we set
with
> > > prctl(PR_SET_NAME).
> > > Why don't we actually consistently do it for all browser threads then from
> > > PlatformThread::SetName (platform_thread_android.cc)?
> > 
> > All browser threads already do it in
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/br...
> 
> According to my experience, if the native thread is never attached to Java,
then
> the name set in PlatformThread::SetName will work, but truncated to certain
> number of characters. GPU thread is such an example.
> If the native thread is attached to Java at one point, then we need to make
sure
> it is attached with a meaningful name, otherwise JVM will assign Thread-??? to
> the name.

For example, the gpu thread will be truncated like this
"Chrome_InProcGp" prio=5 (not attached)
  | sysTid=32595 nice=-6 cgrp=default
  | state=S schedstat=( 565336011 76494623 819 ) utm=27 stm=29 core=0 HZ=100

[no sievers, 2015-12-14 20:10:02]

On 2015/12/14 19:48:48, hush wrote:
> On 2015/12/14 19:47:16, hush wrote:
> > On 2015/12/14 19:44:06, hush wrote:
> > > On 2015/12/14 19:34:38, sievers wrote:
> > > > Sounds like for traces.txt, Android doesn't resolve thread names we set
> with
> > > > prctl(PR_SET_NAME).
> > > > Why don't we actually consistently do it for all browser threads then
from
> > > > PlatformThread::SetName (platform_thread_android.cc)?
> > > 
> > > All browser threads already do it in
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/br...
> > 
> > According to my experience, if the native thread is never attached to Java,
> then
> > the name set in PlatformThread::SetName will work, but truncated to certain
> > number of characters. GPU thread is such an example.
> > If the native thread is attached to Java at one point, then we need to make
> sure
> > it is attached with a meaningful name, otherwise JVM will assign Thread-???
to
> > the name.
> 
> For example, the gpu thread will be truncated like this
> "Chrome_InProcGp" prio=5 (not attached)
>   | sysTid=32595 nice=-6 cgrp=default
>   | state=S schedstat=( 565336011 76494623 819 ) utm=27 stm=29 core=0 HZ=100

The GPU thread will make JNI calls when you play video.

What about the in-process IO threads for GPU and renderer?

[hush (inactive), 2015-12-17 20:05:42]

On 2015/12/14 20:10:02, sievers wrote:
> On 2015/12/14 19:48:48, hush wrote:
> > On 2015/12/14 19:47:16, hush wrote:
> > > On 2015/12/14 19:44:06, hush wrote:
> > > > On 2015/12/14 19:34:38, sievers wrote:
> > > > > Sounds like for traces.txt, Android doesn't resolve thread names we
set
> > with
> > > > > prctl(PR_SET_NAME).
> > > > > Why don't we actually consistently do it for all browser threads then
> from
> > > > > PlatformThread::SetName (platform_thread_android.cc)?
> > > > 
> > > > All browser threads already do it in
> > > >
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/br...
> > > 
> > > According to my experience, if the native thread is never attached to
Java,
> > then
> > > the name set in PlatformThread::SetName will work, but truncated to
certain
> > > number of characters. GPU thread is such an example.
> > > If the native thread is attached to Java at one point, then we need to
make
> > sure
> > > it is attached with a meaningful name, otherwise JVM will assign
Thread-???
> to
> > > the name.
> > 
> > For example, the gpu thread will be truncated like this
> > "Chrome_InProcGp" prio=5 (not attached)
> >   | sysTid=32595 nice=-6 cgrp=default
> >   | state=S schedstat=( 565336011 76494623 819 ) utm=27 stm=29 core=0 HZ=100
> 
> The GPU thread will make JNI calls when you play video.
> 
> What about the in-process IO threads for GPU and renderer?

(Sorry about the delay in reply)
By in-process threads for GPU and renderer, do you mean this?
https://code.google.com/p/chromium/codesearch#chromium/src/content/child/chil...

If so, they are not attached to java, so the names show up in traces.txt is the
one from PlatformThread::SetName, truncated to 15 characters.

[no sievers, 2015-12-17 20:18:18]

On 2015/12/17 20:05:42, hush wrote:
> On 2015/12/14 20:10:02, sievers wrote:
> > On 2015/12/14 19:48:48, hush wrote:
> > > On 2015/12/14 19:47:16, hush wrote:
> > > > On 2015/12/14 19:44:06, hush wrote:
> > > > > On 2015/12/14 19:34:38, sievers wrote:
> > > > > > Sounds like for traces.txt, Android doesn't resolve thread names we
> set
> > > with
> > > > > > prctl(PR_SET_NAME).
> > > > > > Why don't we actually consistently do it for all browser threads
then
> > from
> > > > > > PlatformThread::SetName (platform_thread_android.cc)?
> > > > > 
> > > > > All browser threads already do it in
> > > > >
> > > >
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/br...
> > > > 
> > > > According to my experience, if the native thread is never attached to
> Java,
> > > then
> > > > the name set in PlatformThread::SetName will work, but truncated to
> certain
> > > > number of characters. GPU thread is such an example.
> > > > If the native thread is attached to Java at one point, then we need to
> make
> > > sure
> > > > it is attached with a meaningful name, otherwise JVM will assign
> Thread-???
> > to
> > > > the name.
> > > 
> > > For example, the gpu thread will be truncated like this
> > > "Chrome_InProcGp" prio=5 (not attached)
> > >   | sysTid=32595 nice=-6 cgrp=default
> > >   | state=S schedstat=( 565336011 76494623 819 ) utm=27 stm=29 core=0
HZ=100
> > 
> > The GPU thread will make JNI calls when you play video.
> > 
> > What about the in-process IO threads for GPU and renderer?
> 
> (Sorry about the delay in reply)
> By in-process threads for GPU and renderer, do you mean this?
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/child/chil...
> 
> If so, they are not attached to java, so the names show up in traces.txt is
the
> one from PlatformThread::SetName, truncated to 15 characters.

Ok that's good to know. Mind adding it in InProcessGpuThread since that might
make JNI calls?

But LGTM.

[hush (inactive), 2015-12-17 22:34:08]

Description was changed from

==========
Assign thread name to in_proc renderer thread.

This would be very helpful for analyzing traces.txt of ANR logs.

BUG=
==========

to

==========
Assign thread name to in_proc renderer and gpu thread.

This would be very helpful for analyzing traces.txt of ANR logs.

BUG=570819
==========

[hush (inactive), 2015-12-17 22:37:48]

The CQ bit was checked by hush@chromium.org

[hush (inactive), 2015-12-17 22:37:48]

The patchset sent to the CQ was uploaded after l-g-t-m from boliu@chromium.org,
sievers@chromium.org
Link to the patchset: https://codereview.chromium.org/1518213002/#ps40001
(title: "Gpu thread too")

[commit-bot: I haz the power, 2015-12-17 22:40:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1518213002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1518213002/40001

[commit-bot: I haz the power, 2015-12-18 00:41:51]

Description was changed from

==========
Assign thread name to in_proc renderer and gpu thread.

This would be very helpful for analyzing traces.txt of ANR logs.

BUG=570819
==========

to

==========
Assign thread name to in_proc renderer and gpu thread.

This would be very helpful for analyzing traces.txt of ANR logs.

BUG=570819
==========

[commit-bot: I haz the power, 2015-12-18 00:41:52]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-12-18 00:42:42]

Description was changed from

==========
Assign thread name to in_proc renderer and gpu thread.

This would be very helpful for analyzing traces.txt of ANR logs.

BUG=570819
==========

to

==========
Assign thread name to in_proc renderer and gpu thread.

This would be very helpful for analyzing traces.txt of ANR logs.

BUG=570819

Committed: https://crrev.com/5380add90830ef6a550563c87736484c1b2bfd17
Cr-Commit-Position: refs/heads/master@{#365953}
==========

[commit-bot: I haz the power, 2015-12-18 00:42:42]

Patchset 3 (id:??) landed as
https://crrev.com/5380add90830ef6a550563c87736484c1b2bfd17
Cr-Commit-Position: refs/heads/master@{#365953}