[Toybox] ps and top (and Android)

[enh]

On Fri, Apr 29, 2016 at 6:06 PM, enh <enh at google.com> wrote:
> On Fri, Apr 29, 2016 at 5:45 PM, Rob Landley <rob at landley.net> wrote:
>> On 04/29/2016 04:06 PM, enh wrote:
>>> sorry, hadn't had time to look until now. notes:
>>>
>>> * the tid column seems very wrong. in particular, i see a lot of
>>> duplicate tids. ah, the pid/tid columns don't auto-expand and are
>>> truncating...
>>>
>>> GNU: 147139 130820 ?        00:00:00 default_ThreadM
>>> toybox: 14713 13082 ?        00:00:00 default_ThreadM
>>
>> Hmmm, this sounds like a problem for PIDs too.
>
> (yeah, sorry --- you can see that in my example above but i forgot to
> actually call it out.)
>
>> Let's see...
>>
>> $ ps -o pid:3,user:3,cmd:3
>> PID USER CMD
>> 18748 la+ bash
>> 19147 la+ ps -o pid:3,user:3,cmd:3
>>
>> Ok,  I need to mark some columns as "expand this regardless" and others
>> to indicate when they get truncated, and yet it truncates the cmdline at
>> the right edge of the screen without indicating truncation...
>>
>> Hmmm... It's not quite obvious to me what the correct behavior is.
>> (Numeric fields expand, command lines are silently truncated,
>> non-command string fields do the + thing...? Maybe?)
>>
>>> * your -T is (like the desktop) showing the process name for each
>>> thread. Android shows the thread name, which is probably what you want
>>> if you're asking to see the threads in the first place.
>>
>> There's a thread name? Ah, the stat[2] field gets rewritten.
>
> in the pthread implementation we read/write /proc/self/task/%d/comm.
> toolbox's ps shows the text between ()s in the stat file. since you
> have to read that anyway, that probably makes sense.
>
>> How about if I treat PID != TID the same as kernel threads? [stat2name]
>> in square brackets?
>
> i reserve the right to come back later with strong negative feedback
> from users, but that sounds good to me. it might actually be useful to
> have a visible "not the main thread" distinction that doesn't require
> skipping back to the pid/tid columns.

i see you committed an implementation already. looks good to me so
far. as i predicted, i quite like the at-a-glance "this is a thread"
signal.

>>> * you say 0 for BIT when you don't know, rather than leaving it blank
>>> (like Android) or using '-' (like some other ps fields). i don't think
>>> this matters, but i've attached a patch anyway.
>>
>> Applied.
>>
>>> * there's a whitespace mixup in the help text for -O. patch attached.
>>
>> Applied.
>>
>>> * i need to send you the Android-specific cruft for scheduling policy.
>>
>> Woot.
>
> you say that because you haven't seen it yet :-)
>
> attached. i kept the #ifs in lib/portability.* because there was no
> obvious way to just say "this field isn't useful outside of Android".
>
>>> other than that, i went through my old notes and i think everything
>>> else is now done. i'll get AOSP synced this afternoon (must be friday
>>> again!) and send you a patch for the scheduling policy names.
>>
>> I'll try to figure out what to do about column expansion. (Mental note:
>> how does this impact top not wrapping lines and thus scrolling the
>> screen...)
>>
>> Rob
>
>
>
> --
> Elliott Hughes - http://who/enh - http://jessies.org/~enh/
> Android native code/tools questions? Mail me/drop by/add me as a reviewer.



-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.